[PATCH net-next] tcp_ipv6: set the drop_reason in the right place

[PATCH net-next] tcp_ipv6: set the drop_reason in the right place

[Jakub Kicinski]

Looks like the IPv6 version of the patch under Fixes was
a copy/paste of the IPv4 but hit the wrong spot.
It is tcp_v6_rcv() which uses drop_reason as a boolean, and
needs to be protected against reason == 0 before calling free.
tcp_v6_do_rcv() has a pretty straightforward flow.

Fixes: f8319dfd1b3b ("net: tcp: reset 'drop_reason' to NOT_SPCIFIED in tcp_v{4,6}_rcv()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: yoshfuji@linux-ipv6.org
CC: dsahern@kernel.org
CC: flyingpeng@tencent.com
CC: imagedong@tencent.com
CC: benbjiang@tencent.com
---
 net/ipv6/tcp_ipv6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 28e47ca1e26d..f37dd4aa91c6 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1509,7 +1509,6 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 discard:
 	if (opt_skb)
 		__kfree_skb(opt_skb);
-	SKB_DR_OR(reason, NOT_SPECIFIED);
 	kfree_skb_reason(skb, reason);
 	return 0;
 csum_err:
@@ -1763,6 +1762,7 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 	}
 
 discard_it:
+	SKB_DR_OR(drop_reason, NOT_SPECIFIED);
 	kfree_skb_reason(skb, drop_reason);
 	return 0;
 
-- 
2.34.3

Re: [PATCH net-next] tcp_ipv6: set the drop_reason in the right place

[Matthieu Baerts]

Hi Jakub,

On 20/05/2022 04:13, Jakub Kicinski wrote:
> Looks like the IPv6 version of the patch under Fixes was
> a copy/paste of the IPv4 but hit the wrong spot.
> It is tcp_v6_rcv() which uses drop_reason as a boolean, and
> needs to be protected against reason == 0 before calling free.
> tcp_v6_do_rcv() has a pretty straightforward flow.

Thank you for the patch!

It looks like this fixes an issue our MPTCP CI detected recently:

https://github.com/multipath-tcp/mptcp_net-next/issues/277


Just in case someone else had this issue on their side, here is the call
trace we had:


[ 1256.803388] WARNING: CPU: 1 PID: 0 at net/core/skbuff.c:775
kfree_skb_reason (net/core/skbuff.c:775 (discriminator 1))
[ 1256.804398] Modules linked in: nft_tproxy nf_tproxy_ipv6
nf_tproxy_ipv4 nft_socket nf_socket_ipv4 nf_socket_ipv6 nf_tables sch_netem
[ 1256.805815] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.18.0-rc6 #201
[ 1256.806590] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.13.0-1ubuntu1.1 04/01/2014
[ 1256.807680] RIP: 0010:kfree_skb_reason (net/core/skbuff.c:775
(discriminator 1))
[ 1256.808245] Code: 99 77 75 b2 0f 1f 44 00 00 eb ab 48 8d bf dc 00 00
00 b8 ff ff ff ff f0 0f c1 85 dc 00 00 00 83 f8 01 74 83 85 c0 7e 06 5d
c3 <0f> 0b eb 81 be 03 00 00 00 5d e9 11 98 ac ff c3 0f 1f 44 00 00 48
[ 1256.810639] RSP: 0018:ffffa46c000c8d10 EFLAGS: 00010286
[ 1256.811382] RAX: 00000000ffffffff RBX: ffff9a6903226a00 RCX:
ffff9a6902f96c00
[ 1256.812262] RDX: 00000000000002c0 RSI: 0000000000000000 RDI:
ffff9a69054d36e8
[ 1256.812712] RBP: ffff9a69054d36e8 R08: ffff9a69032b2580 R09:
00000000851b0660
[ 1256.813153] R10: 0000000000000000 R11: ffff9a69032b2500 R12:
ffff9a69032b2500
[ 1256.813594] R13: ffff9a6902f96cec R14: ffff9a6902f96d14 R15:
0000000000000001
[ 1256.814082] FS:  0000000000000000(0000) GS:ffff9a697dd00000(0000)
knlGS:0000000000000000
[ 1256.814607] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1256.814975] CR2: 000056011bdca008 CR3: 0000000002a78000 CR4:
0000000000350ee0
[ 1256.815419] Call Trace:
[ 1256.815611]  <IRQ>
[ 1256.815761] tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1767)
[ 1256.816014] ? ip6_route_input (include/linux/skbuff.h:1305)
[ 1256.816315] ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:438)
[ 1256.816646] ip6_input_finish (include/linux/rcupdate.h:726)
[ 1256.816921] ip6_input (include/linux/netfilter.h:307)
[ 1256.817153] ipv6_rcv (net/ipv6/ip6_input.c:309)
[ 1256.817379] ? internal_add_timer (kernel/time/timer.c:612)
[ 1256.817685] __netif_receive_skb_one_core (net/core/dev.c:5484)
[ 1256.818044] process_backlog (include/linux/rcupdate.h:726)
[ 1256.818317] __napi_poll (net/core/dev.c:6489)
[ 1256.818568] net_rx_action (net/core/dev.c:6558)
[ 1256.818828] __do_softirq (arch/x86/include/asm/jump_label.h:27)
[ 1256.819049] irq_exit_rcu (kernel/softirq.c:432)
[ 1256.819284] sysvec_apic_timer_interrupt
(arch/x86/kernel/apic/apic.c:1097 (discriminator 14))
[ 1256.819586]  </IRQ>
[ 1256.819721]  <TASK>
[ 1256.819855] asm_sysvec_apic_timer_interrupt
(arch/x86/include/asm/idtentry.h:645)


With your patch, I no longer hit the WARN locally even after ~50
executions of "mptcp_connect.sh -m mmap" test.

Tested-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH net-next] tcp_ipv6: set the drop_reason in the right place

[Jakub Kicinski]

On Fri, 20 May 2022 18:22:22 +0200 Matthieu Baerts wrote:
> On 20/05/2022 04:13, Jakub Kicinski wrote:
> > Looks like the IPv6 version of the patch under Fixes was
> > a copy/paste of the IPv4 but hit the wrong spot.
> > It is tcp_v6_rcv() which uses drop_reason as a boolean, and
> > needs to be protected against reason == 0 before calling free.
> > tcp_v6_do_rcv() has a pretty straightforward flow.  
> 
> Thank you for the patch!
> 
> It looks like this fixes an issue our MPTCP CI detected recently:
> 
> https://github.com/multipath-tcp/mptcp_net-next/issues/277
> 
> 
> Just in case someone else had this issue on their side, here is the call
> trace we had:

Ack, I got the same trace. Let me add the trace to the commit msg and
fast-path merging this fix so more people don't hit the stack trace.

Re: [PATCH net-next] tcp_ipv6: set the drop_reason in the right place

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 19 May 2022 19:13:47 -0700 you wrote:
> Looks like the IPv6 version of the patch under Fixes was
> a copy/paste of the IPv4 but hit the wrong spot.
> It is tcp_v6_rcv() which uses drop_reason as a boolean, and
> needs to be protected against reason == 0 before calling free.
> tcp_v6_do_rcv() has a pretty straightforward flow.
> 
> Fixes: f8319dfd1b3b ("net: tcp: reset 'drop_reason' to NOT_SPCIFIED in tcp_v{4,6}_rcv()")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> [...]

Here is the summary with links:
  - [net-next] tcp_ipv6: set the drop_reason in the right place
    https://git.kernel.org/netdev/net-next/c/dc7769244e03

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html