[PATCH net] tcp: initialize skb tx timestamp key before cloning

[David Wei <dw@davidwei.uk>]

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 <dw@davidwei.uk>
---
 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