Date:   Mon, 6 Jul 2020 12:25:25 -0700 (PDT)
From:   Mat Martineau <mathew.j.martineau@...ux.intel.com>
To:     Davide Caratti <dcaratti@...hat.com>
cc:     Matthieu Baerts <matthieu.baerts@...sares.net>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        mptcp@...ts.01.org
Subject: Re: [PATCH net-next] mptcp: fix race in subflow_data_ready()

On Mon, 6 Jul 2020, Davide Caratti wrote:

> syzkaller was able to make the kernel reach subflow_data_ready() for a
> server subflow that was closed before subflow_finish_connect() completed.
> In these cases we can avoid using the path for regular/fallback MPTCP
> data, and just wake the main socket, to avoid the following warning:
>
> WARNING: CPU: 0 PID: 9370 at net/mptcp/subflow.c:885
> subflow_data_ready+0x1e6/0x290 net/mptcp/subflow.c:885
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 0 PID: 9370 Comm: syz-executor.0 Not tainted 5.7.0 #106
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> Call Trace:
>  <IRQ>
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0xb7/0xfe lib/dump_stack.c:118
>  panic+0x29e/0x692 kernel/panic.c:221
>  __warn.cold+0x2f/0x3d kernel/panic.c:582
>  report_bug+0x28b/0x2f0 lib/bug.c:195
>  fixup_bug arch/x86/kernel/traps.c:105 [inline]
>  fixup_bug arch/x86/kernel/traps.c:100 [inline]
>  do_error_trap+0x10f/0x180 arch/x86/kernel/traps.c:197
>  do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:216
>  invalid_op+0x1e/0x30 arch/x86/entry/entry_64.S:1027
> RIP: 0010:subflow_data_ready+0x1e6/0x290 net/mptcp/subflow.c:885
> Code: 04 02 84 c0 74 06 0f 8e 91 00 00 00 41 0f b6 5e 48 31 ff 83 e3 18
> 89 de e8 37 ec 3d fe 84 db 0f 85 65 ff ff ff e8 fa ea 3d fe <0f> 0b e9
> 59 ff ff ff e8 ee ea 3d fe 48 89 ee 4c 89 ef e8 f3 77 ff
> RSP: 0018:ffff88811b2099b0 EFLAGS: 00010206
> RAX: ffff888111197000 RBX: 0000000000000000 RCX: ffffffff82fbc609
> RDX: 0000000000000100 RSI: ffffffff82fbc616 RDI: 0000000000000001
> RBP: ffff8881111bc800 R08: ffff888111197000 R09: ffffed10222a82af
> R10: ffff888111541577 R11: ffffed10222a82ae R12: 1ffff11023641336
> R13: ffff888111541000 R14: ffff88810fd4ca00 R15: ffff888111541570
>  tcp_child_process+0x754/0x920 net/ipv4/tcp_minisocks.c:841
>  tcp_v4_do_rcv+0x749/0x8b0 net/ipv4/tcp_ipv4.c:1642
>  tcp_v4_rcv+0x2666/0x2e60 net/ipv4/tcp_ipv4.c:1999
>  ip_protocol_deliver_rcu+0x29/0x1f0 net/ipv4/ip_input.c:204
>  ip_local_deliver_finish net/ipv4/ip_input.c:231 [inline]
>  NF_HOOK include/linux/netfilter.h:421 [inline]
>  ip_local_deliver+0x2da/0x390 net/ipv4/ip_input.c:252
>  dst_input include/net/dst.h:441 [inline]
>  ip_rcv_finish net/ipv4/ip_input.c:428 [inline]
>  ip_rcv_finish net/ipv4/ip_input.c:414 [inline]
>  NF_HOOK include/linux/netfilter.h:421 [inline]
>  ip_rcv+0xef/0x140 net/ipv4/ip_input.c:539
>  __netif_receive_skb_one_core+0x197/0x1e0 net/core/dev.c:5268
>  __netif_receive_skb+0x27/0x1c0 net/core/dev.c:5382
>  process_backlog+0x1e5/0x6d0 net/core/dev.c:6226
>  napi_poll net/core/dev.c:6671 [inline]
>  net_rx_action+0x3e3/0xd70 net/core/dev.c:6739
>  __do_softirq+0x18c/0x634 kernel/softirq.c:292
>  do_softirq_own_stack+0x2a/0x40 arch/x86/entry/entry_64.S:1082
>  </IRQ>
>  do_softirq.part.0+0x26/0x30 kernel/softirq.c:337
>  do_softirq arch/x86/include/asm/preempt.h:26 [inline]
>  __local_bh_enable_ip+0x46/0x50 kernel/softirq.c:189
>  local_bh_enable include/linux/bottom_half.h:32 [inline]
>  rcu_read_unlock_bh include/linux/rcupdate.h:723 [inline]
>  ip_finish_output2+0x78a/0x19c0 net/ipv4/ip_output.c:229
>  __ip_finish_output+0x471/0x720 net/ipv4/ip_output.c:306
>  dst_output include/net/dst.h:435 [inline]
>  ip_local_out+0x181/0x1e0 net/ipv4/ip_output.c:125
>  __ip_queue_xmit+0x7a1/0x14e0 net/ipv4/ip_output.c:530
>  __tcp_transmit_skb+0x19dc/0x35e0 net/ipv4/tcp_output.c:1238
>  __tcp_send_ack.part.0+0x3c2/0x5b0 net/ipv4/tcp_output.c:3785
>  __tcp_send_ack net/ipv4/tcp_output.c:3791 [inline]
>  tcp_send_ack+0x7d/0xa0 net/ipv4/tcp_output.c:3791
>  tcp_rcv_synsent_state_process net/ipv4/tcp_input.c:6040 [inline]
>  tcp_rcv_state_process+0x36a4/0x49c2 net/ipv4/tcp_input.c:6209
>  tcp_v4_do_rcv+0x343/0x8b0 net/ipv4/tcp_ipv4.c:1651
>  sk_backlog_rcv include/net/sock.h:996 [inline]
>  __release_sock+0x1ad/0x310 net/core/sock.c:2548
>  release_sock+0x54/0x1a0 net/core/sock.c:3064
>  inet_wait_for_connect net/ipv4/af_inet.c:594 [inline]
>  __inet_stream_connect+0x57e/0xd50 net/ipv4/af_inet.c:686
>  inet_stream_connect+0x53/0xa0 net/ipv4/af_inet.c:725
>  mptcp_stream_connect+0x171/0x5f0 net/mptcp/protocol.c:1920
>  __sys_connect_file net/socket.c:1854 [inline]
>  __sys_connect+0x267/0x2f0 net/socket.c:1871
>  __do_sys_connect net/socket.c:1882 [inline]
>  __se_sys_connect net/socket.c:1879 [inline]
>  __x64_sys_connect+0x6f/0xb0 net/socket.c:1879
>  do_syscall_64+0xb7/0x3d0 arch/x86/entry/common.c:295
>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7fb577d06469
> Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89
> f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01
> f0 ff ff 73 01 c3 48 8b 0d ff 49 2b 00 f7 d8 64 89 01 48
> RSP: 002b:00007fb5783d5dd8 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
> RAX: ffffffffffffffda RBX: 000000000068bfa0 RCX: 00007fb577d06469
> RDX: 000000000000004d RSI: 0000000020000040 RDI: 0000000000000003
> RBP: 00000000ffffffff R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: 000000000041427c R14: 00007fb5783d65c0 R15: 0000000000000003
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/39
> Reported-by: Christoph Paasch <cpaasch@...le.com>
> Fixes: e1ff9e82e2ea ("net: mptcp: improve fallback to TCP")
> Suggested-by: Paolo Abeni <pabeni@...hat.com>
> Signed-off-by: Davide Caratti <dcaratti@...hat.com>
> ---
> net/mptcp/subflow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index e1e19c76e267..9f7f3772c13c 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -873,7 +873,7 @@ static void subflow_data_ready(struct sock *sk)
> 	struct mptcp_sock *msk;
>
> 	msk = mptcp_sk(parent);
> -	if (inet_sk_state_load(sk) == TCP_LISTEN) {
> +	if ((1 << inet_sk_state_load(sk)) & (TCPF_LISTEN | TCPF_CLOSE)) {
> 		set_bit(MPTCP_DATA_READY, &msk->flags);
> 		parent->sk_data_ready(parent);
> 		return;
> -- 
> 2.26.2

Reviewed-by: Mat Martineau <mathew.j.martineau@...ux.intel.com>

--
Mat Martineau
Intel

Powered by blists - more mailing lists