multipath-tcp / mptcp_net-next

Development version of the Upstream MultiPath TCP Linux kernel 🐧
https://mptcp.dev
Other
292 stars 42 forks source link

syzkaller: WARNING in `mptcp_token_destroy` #442

Closed cpaasch closed 1 year ago

cpaasch commented 1 year ago

syzkaller-id: 5ac39dd915154ec01f83fad890fa594c89a5e207

HEAD: 6a1b099dc979

Crash:

lo: entered allmulticast mode
------------[ cut here ]------------
WARNING: CPU: 0 PID: 19731 at net/mptcp/token.c:388 mptcp_token_destroy+0x123/0x130
Modules linked in:
CPU: 0 PID: 19731 Comm: syz-executor.3 Not tainted 6.6.0-rc2-g6a1b099dc979 #51
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014
RIP: 0010:mptcp_token_destroy+0x123/0x130 net/mptcp/token.c:388
Code: 4c 27 40 4c 89 f7 e8 cc 79 07 00 41 c7 85 d0 08 00 00 00 00 00 00 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc e8 cd 2b c3 fe <0f> 0b eb d5 66 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90
RSP: 0018:ffffc90000e77b10 EFLAGS: 00010293
RAX: ffffffff82605993 RBX: 0000000000000000 RCX: ffff8880091fb300
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: 0000000000300000 R08: ffffffff826058eb R09: 0000000000000000
R10: ffffea00007c8a00 R11: ffff8880091fb300 R12: 0000000000000000
R13: ffff88803b3daf00 R14: ffff888005e80000 R15: ffff888005e80000
FS:  0000000000000000(0000) GS:ffff88807dc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000002000c000 CR3: 0000000052145001 CR4: 0000000000370ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 mptcp_destroy_common+0x1ad/0x1d0 net/mptcp/protocol.c:3314
 mptcp_destroy+0x1e/0x40 net/mptcp/protocol.c:3325
 __mptcp_destroy_sock+0x68/0x1a0 net/mptcp/protocol.c:2953
 __mptcp_close+0x3ca/0x500 net/mptcp/protocol.c:3057
 mptcp_close+0x28/0x100 net/mptcp/protocol.c:3072
 inet_release+0x88/0xa0 net/ipv4/af_inet.c:433
 sock_close+0x51/0x110 net/socket.c:659
 __fput+0x1f2/0x4e0 fs/file_table.c:384
 task_work_run+0x104/0x150 kernel/task_work.c:179
 exit_task_work include/linux/task_work.h:38 [inline]
 do_exit+0x4a0/0xf50 kernel/exit.c:874
 do_group_exit+0xaa/0xe0 kernel/exit.c:1024
 get_signal+0xde0/0xf50 kernel/signal.c:2892
 arch_do_signal_or_restart+0x33/0x410 arch/x86/kernel/signal.c:309
 exit_to_user_mode_loop+0x6a/0xe0 kernel/entry/common.c:168
 exit_to_user_mode_prepare+0x93/0xe0 kernel/entry/common.c:204
 syscall_exit_to_user_mode+0x4c/0x1e0 kernel/entry/common.c:285
 do_syscall_64+0x53/0xa0 arch/x86/entry/common.c:86
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8
RIP: 0033:0x7f400309f6a9
Code: Unable to access opcode bytes at 0x7f400309f67f.
RSP: 002b:00007f40023ccd78 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: fffffffffffffe00 RBX: 00000000006bbf88 RCX: 00007f400309f6a9
RDX: 0000000000000000 RSI: 0000000000000080 RDI: 00000000006bbf88
RBP: 00000000006bbf80 R08: 00000000007df998 R09: 00000000007df998
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006bbf8c
R13: fffffffffffffea8 R14: 000000000000000b R15: 000000000001fe40
 </TASK>
irq event stamp: 6148
hardirqs last  enabled at (6160): [<ffffffff8115899a>] console_unlock+0xfa/0x1c0 kernel/printk/printk.c:347
hardirqs last disabled at (6171): [<ffffffff8115897f>] console_unlock+0xdf/0x1c0 kernel/printk/printk.c:345
softirqs last  enabled at (5438): [<ffffffff825f101f>] spin_unlock_bh include/linux/spinlock.h:396 [inline]
softirqs last  enabled at (5438): [<ffffffff825f101f>] mptcp_destroy_common+0x18f/0x1d0 net/mptcp/protocol.c:3307
softirqs last disabled at (5440): [<ffffffff826058cb>] spin_lock_bh include/linux/spinlock.h:356 [inline]
softirqs last disabled at (5440): [<ffffffff826058cb>] mptcp_token_destroy+0x5b/0x130 net/mptcp/token.c:386
---[ end trace 0000000000000000 ]---

Kconfig: Kconfig_k7_clean.txt

Reproducer:

# {Threaded:false Repeat:false RepeatTimes:0 Procs:1 Slowdown:1 Sandbox: SandboxArg:0 Leak:false NetInjection:false NetDevices:false NetReset:false Cgroups:false BinfmtMisc:false CloseFDs:false KCSAN:false DevlinkPCI:false NicVF:false USB:false VhciInjection:false Wifi:false IEEE802154:false Sysctl:false Swap:false UseTmpDir:false HandleSegv:false Repro:false Trace:false LegacyOptions:{Collide:false Fault:false FaultCall:0 FaultNth:0}}
r0 = socket$inet6_mptcp(0xa, 0x1, 0x106)
bind$inet6(r0, &(0x7f0000000180)={0xa, 0x4e23, 0x0, @loopback}, 0x1c)
listen(r0, 0x0)
r1 = socket$inet6_mptcp(0xa, 0x1, 0x106)
connect$inet6(r1, &(0x7f00000000c0)={0xa, 0x4e23, 0x0, @loopback}, 0x63)
r2 = accept(r0, 0x0, 0x0)
setsockopt$sock_int(r2, 0x1, 0x12, &(0x7f0000000280)=0x20000, 0x4)

C-reproducer available.

pabeni commented 1 year ago

@cpaasch: could you please additionally share the c-repro (possibly as attachment) or test the tentative fix in the following comment?

pabeni commented 1 year ago

should be fixed with:

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 5fa417a6f4c5..c34a6f41e2eb 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1490,9 +1490,17 @@ int mptcp_set_rcvlowat(struct sock *sk, int val)
                return 0;

        space = __tcp_space_from_win(mptcp_sk(sk)->scaling_ratio, val);
-       if (space > sk->sk_rcvbuf) {
-               WRITE_ONCE(sk->sk_rcvbuf, space);
+       if (space <= sk->sk_rcvbuf)
+               return 0;
+
+       WRITE_ONCE(sk->sk_rcvbuf, space);
+       mptcp_for_each_subflow(msk, subflow) {
+               struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+               bool slow;
+
+               slow = lock_sock_fast(ssk);
                tcp_sk(sk)->window_clamp = val;
+               unlock_sock_fast(ssk, slow);
        }
        return 0;

even simpler:

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 5fa417a6f4c5..e41010a4de7f 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1490,9 +1490,7 @@ int mptcp_set_rcvlowat(struct sock *sk, int val)
                return 0;

        space = __tcp_space_from_win(mptcp_sk(sk)->scaling_ratio, val);
-       if (space > sk->sk_rcvbuf) {
+       if (space > sk->sk_rcvbuf)
                WRITE_ONCE(sk->sk_rcvbuf, space);
-               tcp_sk(sk)->window_clamp = val;
-       }
        return 0;
 }
cpaasch commented 1 year ago

Yes, this works @pabeni

matttbe commented 1 year ago

Fixed by Paolo's patches:

New patches for t/upstream:

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20231003T165758

Cheers, Matt