syzkaller: WARNING in `mptcp_token_destroy` #442

cpaasch · 2023-09-26T17:25:36Z

syzkaller-id: 5ac39dd915154ec01f83fad890fa594c89a5e207

HEAD: 6a1b099

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.

The text was updated successfully, but these errors were encountered:

pabeni · 2023-09-26T20:36:06Z

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

pabeni · 2023-09-26T20:53:20Z

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 · 2023-09-27T03:11:00Z

Yes, this works @pabeni

Signed-off-by: Christoph Paasch <[email protected]>

Christoph reported a couple of serious splat caused by the mentioned patch. mptcp_set_rcvlowat() can use msk->scaling_ratio, before such field is initialized, causing a divide by zero: we need to init it in the sock constructor. Additionally the same function bogusly cast an msk to a tcp_sock, causing memory corruption. The reproducer likely clears the sk refcount for the next msk allocated into the same slab. The intent was to properly propagate the rcvbuf changes to the subflows. Let's do that explicitly. Signed-off-by: Paolo Abeni <[email protected]> -- Closes: multipath-tcp/mptcp_net-next#442 Closes: multipath-tcp/mptcp_net-next#443 since the above issues are introduced by the squash-to patch, I think we can't have the tag in the final patch.

Christoph reported a couple of serious splat caused by the mentioned patch. mptcp_set_rcvlowat() can use msk->scaling_ratio, before such field is initialized, causing a divide by zero: we need to init it in the sock constructor. Additionally the same function bogusly cast an msk to a tcp_sock, causing memory corruption. The reproducer likely clears the sk refcount for the next msk allocated into the same slab. The intent was to properly propagate the rcvbuf changes to the subflows. Let's do that explicitly. Signed-off-by: Paolo Abeni <[email protected]> -- Closes: multipath-tcp/mptcp_net-next#442 Closes: multipath-tcp/mptcp_net-next#443 since the above issues are introduced by the squash-to patch, I think we can't have the tag in the final patch. v1 -> v2: - use scaling_ratio define (Mat)

Signed-off-by: Christoph Paasch <[email protected]>

Christoph reported a couple of serious splat caused by the mentioned patch. mptcp_set_rcvlowat() can use msk->scaling_ratio, before such field is initialized, causing a divide by zero: we need to init it in the sock constructor. Additionally the same function bogusly cast an msk to a tcp_sock, causing memory corruption. The reproducer likely clears the sk refcount for the next msk allocated into the same slab. The intent was to properly propagate the rcvbuf changes to the subflows. Let's do that explicitly. Signed-off-by: Paolo Abeni <[email protected]> -- Closes: #442 Closes: #443 since the above issues are introduced by the squash-to patch, I think we can't have the tag in the final patch. v1 -> v2: - use scaling_ratio define (Mat) Link: https://lore.kernel.org/r/21dd84bd44a8d0839564bed351e7d77a16b68474.1696237983.git.pabeni@redhat.com Signed-off-by: Matthieu Baerts <[email protected]>

matttbe · 2023-10-03T17:01:46Z

Fixed by Paolo's patches:

New patches for t/upstream:

3a6e7c6: tcp: define initial scaling factor value as a macro
4b8e173: "squashed" patch 2/2 in "mptcp: give rcvlowat some love"
Results: 3cbb192..ad8e597 (export)

Tests are now in progress:

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

Cheers,
Matt

cpaasch added bug syzkaller reproducer Has a simple program to reproduce the bug labels Sep 26, 2023

cpaasch added a commit that referenced this issue Sep 27, 2023

mptcp: Paulo's fix for #442

b9e611b

Signed-off-by: Christoph Paasch <[email protected]>

cpaasch added a commit that referenced this issue Oct 3, 2023

mptcp: Paulo's fix for #442

44e6d8b

Signed-off-by: Christoph Paasch <[email protected]>

matttbe closed this as completed Oct 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syzkaller: WARNING in `mptcp_token_destroy` #442

syzkaller: WARNING in `mptcp_token_destroy` #442

cpaasch commented Sep 26, 2023 •

edited

Loading

pabeni commented Sep 26, 2023 •

edited

Loading

pabeni commented Sep 26, 2023

cpaasch commented Sep 27, 2023

matttbe commented Oct 3, 2023

syzkaller: WARNING in mptcp_token_destroy #442

syzkaller: WARNING in mptcp_token_destroy #442

Comments

cpaasch commented Sep 26, 2023 • edited Loading

pabeni commented Sep 26, 2023 • edited Loading

pabeni commented Sep 26, 2023

cpaasch commented Sep 27, 2023

matttbe commented Oct 3, 2023

syzkaller: WARNING in `mptcp_token_destroy` #442

syzkaller: WARNING in `mptcp_token_destroy` #442

cpaasch commented Sep 26, 2023 •

edited

Loading

pabeni commented Sep 26, 2023 •

edited

Loading