[PATCH] tcp: another fix of uncloning packets before mangling them

Subsystems: networking [general], networking [tcp], the rest

STALE2906d REVIEWED: 2 (0M)

2 review trailers.

2 messages, 2 authors, 2018-08-28 · open the first message on its own page

[PATCH] tcp: another fix of uncloning packets before mangling them

From: Wen Yang <hidden>
Date: 2018-08-28 07:32:57

The following warning was caught:

[937151.638394] Call Trace:
[937151.638401]  [<ffffffff8163f2f6>] dump_stack+0x19/0x1b
[937151.638405]  [<ffffffff8107dd70>] warn_slowpath_common+0x70/0xb0
[937151.638407]  [<ffffffff8107deba>] warn_slowpath_null+0x1a/0x20
[937151.638410]  [<ffffffff8158bb7b>] tcp_set_skb_tso_segs+0xeb/0x100
[937151.638412]  [<ffffffff8158bbc7>] tcp_init_tso_segs+0x37/0x50
[937151.638414]  [<ffffffff8158d7b9>] tcp_write_xmit+0x1d9/0xce0
[937151.638417]  [<ffffffff8158e53e>] __tcp_push_pending_frames+0x2e/0xc0
[937151.638419]  [<ffffffff8157cf3c>] tcp_push+0xec/0x120
[937151.638421]  [<ffffffff81580728>] tcp_sendmsg+0xc8/0xc20
[937151.638424]  [<ffffffff815aae24>] inet_sendmsg+0x64/0xb0
[937151.638428]  [<ffffffff810b9565>] ? check_preempt_curr+0x75/0xa0
[937151.638434]  [<ffffffff81519917>] sock_aio_write+0x157/0x180
[937151.638437]  [<ffffffff811e267d>] do_sync_write+0x8d/0xd0
[937151.638440]  [<ffffffff811e2f95>] vfs_write+0x1b5/0x1e0
[937151.638442]  [<ffffffff811e393f>] SyS_write+0x7f/0xe0
[937151.638445]  [<ffffffff816513fd>] system_call_fastpath+0x16/0x1b

According commit c52e2421f736 ("tcp: must unclone packets before
mangling them"), TCP stack should make sure it owns skbs before
mangling them.
And there is another place where skb_unclone() is needed. This patch
fix that.

Signed-off-by: Wen Yang <redacted>
Tested-by: Liu Bo <redacted>
Reviewed-by: Jiang Biao <redacted>
---
 net/ipv4/tcp_output.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 597dbd7..fbe8140 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1793,6 +1793,9 @@ static int tcp_init_tso_segs(struct sk_buff *skb, unsigned int mss_now)
 	int tso_segs = tcp_skb_pcount(skb);
 
 	if (!tso_segs || (tso_segs > 1 && tcp_skb_mss(skb) != mss_now)) {
+		if (skb_unclone(skb, GFP_ATOMIC))
+			return -ENOMEM;
+
 		tcp_set_skb_tso_segs(skb, mss_now);
 		tso_segs = tcp_skb_pcount(skb);
 	}
@@ -2045,6 +2048,7 @@ static int tcp_mtu_probe(struct sock *sk)
 	int copy, len;
 	int mss_now;
 	int interval;
+	int err;
 
 	/* Not currently probing/verifying,
 	 * not in recovery,
@@ -2151,7 +2155,9 @@ static int tcp_mtu_probe(struct sock *sk)
 		if (len >= probe_size)
 			break;
 	}
-	tcp_init_tso_segs(nskb, nskb->len);
+	err = tcp_init_tso_segs(nskb, nskb->len);
+	if (unlikely(err < 0))
+		return err;
 
 	/* We're ready to send.  If this fails, the probe will
 	 * be resegmented into mss-sized pieces by tcp_write_xmit().
@@ -2309,6 +2315,8 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
 
 		tso_segs = tcp_init_tso_segs(skb, mss_now);
 		BUG_ON(!tso_segs);
+		if (unlikely(tso_segs < 0))
+			break;
 
 		if (unlikely(tp->repair) && tp->repair_queue == TCP_SEND_QUEUE) {
 			/* "skb_mstamp" is used as a start point for the retransmit timer */
-- 
1.8.3.1

Re: [PATCH] tcp: another fix of uncloning packets before mangling them

From: Eric Dumazet <edumazet@google.com>
Date: 2018-08-28 07:36:52

On Tue, Aug 28, 2018 at 12:32 AM Wen Yang [off-list ref] wrote:
quoted hunk
The following warning was caught:

[937151.638394] Call Trace:
[937151.638401]  [<ffffffff8163f2f6>] dump_stack+0x19/0x1b
[937151.638405]  [<ffffffff8107dd70>] warn_slowpath_common+0x70/0xb0
[937151.638407]  [<ffffffff8107deba>] warn_slowpath_null+0x1a/0x20
[937151.638410]  [<ffffffff8158bb7b>] tcp_set_skb_tso_segs+0xeb/0x100
[937151.638412]  [<ffffffff8158bbc7>] tcp_init_tso_segs+0x37/0x50
[937151.638414]  [<ffffffff8158d7b9>] tcp_write_xmit+0x1d9/0xce0
[937151.638417]  [<ffffffff8158e53e>] __tcp_push_pending_frames+0x2e/0xc0
[937151.638419]  [<ffffffff8157cf3c>] tcp_push+0xec/0x120
[937151.638421]  [<ffffffff81580728>] tcp_sendmsg+0xc8/0xc20
[937151.638424]  [<ffffffff815aae24>] inet_sendmsg+0x64/0xb0
[937151.638428]  [<ffffffff810b9565>] ? check_preempt_curr+0x75/0xa0
[937151.638434]  [<ffffffff81519917>] sock_aio_write+0x157/0x180
[937151.638437]  [<ffffffff811e267d>] do_sync_write+0x8d/0xd0
[937151.638440]  [<ffffffff811e2f95>] vfs_write+0x1b5/0x1e0
[937151.638442]  [<ffffffff811e393f>] SyS_write+0x7f/0xe0
[937151.638445]  [<ffffffff816513fd>] system_call_fastpath+0x16/0x1b

According commit c52e2421f736 ("tcp: must unclone packets before
mangling them"), TCP stack should make sure it owns skbs before
mangling them.
And there is another place where skb_unclone() is needed. This patch
fix that.

Signed-off-by: Wen Yang <redacted>
Tested-by: Liu Bo <redacted>
Reviewed-by: Jiang Biao <redacted>
---
 net/ipv4/tcp_output.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 597dbd7..fbe8140 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1793,6 +1793,9 @@ static int tcp_init_tso_segs(struct sk_buff *skb, unsigned int mss_now)
        int tso_segs = tcp_skb_pcount(skb);
Certainly not needed.

TCP stack owns its packets, as long they were never sent (not yet in
retransmit queue)

You probably are using an old kernel, missing some backport...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help