tcp_tx_timestamp() can select skbs from the rtx queue when all the copied data has been sent in tcp_sendmsg_locked(). These skbs may be already cloned, sharing the same shinfo, and handed off into the lower tx layers. tcp_tx_timestamp() sets tx_flags before setting skb tskey, racing with any reader of both. It is possible to observe a valid tx_flag, but an uninitialized tskey, which produces a large underflow after subtracting the socket tskey. Initialize skb tskey just before cloning while its shinfo is still private. In tcp_tx_timestamp(), if it selects a cloned skb from the rtx queue from the current tcp_sendmsg_locked() call (i.e. tx_flags and txstamp_ack are 0) and the pre-init skb tskey matches, then tx_flags can be set safely. If the skb is not shared, coming from the write queue, then both tskey and tx_flags are set, as before. Fixes: 838eb9687691 ("tcp: tcp_tx_timestamp() must look at the rtx queue") Assisted-by: LLM Signed-off-by: David Wei --- net/ipv4/tcp.c | 21 ++++++++++++++++----- net/ipv4/tcp_output.c | 20 +++++++++++++------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 562752352afe..0d6bbb41d5f8 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -481,20 +481,31 @@ void tcp_init_sock(struct sock *sk) static void tcp_tx_timestamp(struct sock *sk, struct sockcm_cookie *sockc) { struct sk_buff *skb = tcp_write_queue_tail(sk); + struct skb_shared_info *shinfo; u32 tsflags = sockc->tsflags; + struct tcp_skb_cb *tcb; + u32 tskey; if (unlikely(!skb)) skb = skb_rb_last(&sk->tcp_rtx_queue); + if (unlikely(!skb)) + return; - if (tsflags && skb) { - struct skb_shared_info *shinfo = skb_shinfo(skb); - struct tcp_skb_cb *tcb = TCP_SKB_CB(skb); + shinfo = skb_shinfo(skb); + tcb = TCP_SKB_CB(skb); + tskey = tcb->seq + skb->len - 1; + if (skb_cloned(skb) && + !shinfo->tx_flags && !tcb->txstamp_ack) { + if (shinfo->tskey != tskey) + return; + } else if (tsflags & SOF_TIMESTAMPING_TX_RECORD_MASK) { + shinfo->tskey = tskey; + } + if (tsflags) { sock_tx_timestamp(sk, sockc, &shinfo->tx_flags); if (tsflags & SOF_TIMESTAMPING_TX_ACK) tcb->txstamp_ack |= TSTAMP_ACK_SK; - if (tsflags & SOF_TIMESTAMPING_TX_RECORD_MASK) - shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1; } if (cgroup_bpf_enabled(CGROUP_SOCK_OPS) && diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 00417a429222..ee3efcbbdf3a 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -1522,6 +1522,12 @@ static void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb) } #endif +static bool tcp_has_tx_tstamp(const struct sk_buff *skb) +{ + return TCP_SKB_CB(skb)->txstamp_ack || + (skb_shinfo(skb)->tx_flags & SKBTX_ANY_TSTAMP); +} + /* This routine actually transmits TCP packets queued in by * tcp_do_sendmsg(). This is used by both the initial * transmission and possible later retransmissions. @@ -1559,8 +1565,14 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, tcp_skb_tsorted_save(oskb) { if (unlikely(skb_cloned(oskb))) skb = pskb_copy(oskb, gfp_mask); - else + else { + if (oskb->len && + !(TCP_SKB_CB(oskb)->tcp_flags & TCPHDR_SYN) && + !tcp_has_tx_tstamp(oskb)) + skb_shinfo(oskb)->tskey = + TCP_SKB_CB(oskb)->seq + oskb->len - 1; skb = skb_clone(oskb, gfp_mask); + } } tcp_skb_tsorted_restore(oskb); if (unlikely(!skb)) @@ -1795,12 +1807,6 @@ static void tcp_adjust_pcount(struct sock *sk, const struct sk_buff *skb, int de tcp_verify_left_out(tp); } -static bool tcp_has_tx_tstamp(const struct sk_buff *skb) -{ - return TCP_SKB_CB(skb)->txstamp_ack || - (skb_shinfo(skb)->tx_flags & SKBTX_ANY_TSTAMP); -} - static void tcp_fragment_tstamp(struct sk_buff *skb, struct sk_buff *skb2) { struct skb_shared_info *shinfo = skb_shinfo(skb); -- 2.53.0-Meta