Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

syzkaller: divide error in mptcp_set_rcvlowat #443

Closed
cpaasch opened this issue Sep 26, 2023 · 5 comments
Closed

syzkaller: divide error in mptcp_set_rcvlowat #443

cpaasch opened this issue Sep 26, 2023 · 5 comments
Labels
bug reproducer Has a simple program to reproduce the bug syzkaller

Comments

@cpaasch
Copy link
Member

cpaasch commented Sep 26, 2023

syzkaller-id: 9facae10350754b5068f72085c4c765aacc5391f

HEAD: 6a1b099

Crash:

divide error: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 17708 Comm: syz-executor.2 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_set_rcvlowat+0x118/0x200 include/net/tcp.h:1447
Code: 0f b6 8e e8 0a 00 00 49 63 c7 48 c1 e0 08 48 89 c2 48 c1 ea 20 74 11 31 d2 48 f7 f1 48 89 c3 eb 0d e8 1c 3a c2 fe eb 34 31 d2 <f7> f1 89 c3 41 8b ae 38 02 00 00 89 ef 89 de e8 c4 3b c2 fe 39 dd
RSP: 0018:ffffc9000195fd80 EFLAGS: 00010246
RAX: 0000000030000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000300000 R08: ffffffff82614b18 R09: ffffffff81d29965
R10: 000000000000003b R11: ffffffff82614a30 R12: ffff888016a6a340
R13: 0000000000000000 R14: ffff888016a6a340 R15: 0000000000300000
FS:  00007fa8d2944640(0000) GS:ffff88807dc80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000006bd988 CR3: 000000001c05b001 CR4: 0000000000370ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
lo: entered allmulticast mode
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 sk_setsockopt+0x13bf/0x1af0 net/core/sock.c:1289
IPVS: You probably need to specify IP address on multicast interface.
 mptcp_setsockopt+0x14c5/0x2570 net/mptcp/sockopt.c:354
 __sys_setsockopt+0x16b/0x1d0 net/socket.c:2308
 __do_sys_setsockopt net/socket.c:2319 [inline]
 __se_sys_setsockopt net/socket.c:2316 [inline]
 __x64_sys_setsockopt+0x23/0x30 net/socket.c:2316
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x47/0xa0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8
RIP: 0033:0x7fa8d36166a9
Code: 5c c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 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 4f 37 0d 00 f7 d8 64 89 01 48
RSP: 002b:00007fa8d2943cd8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 00000000006bbf80 RCX: 00007fa8d36166a9
RDX: 0000000000000012 RSI: 0000000000000001 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000004 R09: 0000000000000000
R10: 0000000020000100 R11: 0000000000000246 R12: 00000000006bbf8c
R13: fffffffffffffea8 R14: 00000000006bbf80 R15: 000000000001fe40
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:mptcp_set_rcvlowat+0x118/0x200 include/net/tcp.h:1447
Code: 0f b6 8e e8 0a 00 00 49 63 c7 48 c1 e0 08 48 89 c2 48 c1 ea 20 74 11 31 d2 48 f7 f1 48 89 c3 eb 0d e8 1c 3a c2 fe eb 34 31 d2 <f7> f1 89 c3 41 8b ae 38 02 00 00 89 ef 89 de e8 c4 3b c2 fe 39 dd
RSP: 0018:ffffc9000195fd80 EFLAGS: 00010246
RAX: 0000000030000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000300000 R08: ffffffff82614b18 R09: ffffffff81d29965
R10: 000000000000003b R11: ffffffff82614a30 R12: ffff888016a6a340
R13: 0000000000000000 R14: ffff888016a6a340 R15: 0000000000300000
FS:  00007fa8d2944640(0000) GS:ffff88807dc80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000006bd988 CR3: 000000001c05b001 CR4: 0000000000370ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
IPVS: sync thread started: state = MASTER, mcast_ifn = lo, syncid = 0, id = 0
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
   0:	0f b6 8e e8 0a 00 00 	movzbl 0xae8(%rsi),%ecx
   7:	49 63 c7             	movslq %r15d,%rax
   a:	48 c1 e0 08          	shl    $0x8,%rax
   e:	48 89 c2             	mov    %rax,%rdx
  11:	48 c1 ea 20          	shr    $0x20,%rdx
  15:	74 11                	je     0x28
  17:	31 d2                	xor    %edx,%edx
  19:	48 f7 f1             	div    %rcx
  1c:	48 89 c3             	mov    %rax,%rbx
  1f:	eb 0d                	jmp    0x2e
  21:	e8 1c 3a c2 fe       	callq  0xfec23a42
  26:	eb 34                	jmp    0x5c
  28:	31 d2                	xor    %edx,%edx
* 2a:	f7 f1                	div    %ecx <-- trapping instruction
  2c:	89 c3                	mov    %eax,%ebx
  2e:	41 8b ae 38 02 00 00 	mov    0x238(%r14),%ebp
  35:	89 ef                	mov    %ebp,%edi
  37:	89 de                	mov    %ebx,%esi
  39:	e8 c4 3b c2 fe       	callq  0xfec23c02
  3e:	39 dd                	cmp    %ebx,%ebp

Kconfig:
Kconfig_k5_lockdep.txt

Reproducer:

# {Threaded:true Repeat:true RepeatTimes:0 Procs:8 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}}
unshare(0x40000400)
socket$igmp6(0xa, 0x3, 0x2)
socket$igmp6(0xa, 0x3, 0x2)
socket$inet6_mptcp(0xa, 0x1, 0x106)
socket$inet(0x2, 0x1, 0x0) (async)
r0 = socket$inet(0x2, 0x1, 0x0)
setsockopt$sock_int(r0, 0x1, 0x12, &(0x7f0000000140)=0x636bd5ca, 0x4)

C-repro available.
@cpaasch cpaasch added bug syzkaller reproducer Has a simple program to reproduce the bug labels Sep 26, 2023
@cpaasch
Copy link
Member Author

cpaasch commented Sep 26, 2023

Bisected to af1c4b3

@pabeni
Copy link

pabeni commented Sep 26, 2023

should be fixed with:

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 6f9e116598ed..3ef6368e26f6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2758,6 +2758,8 @@ static void __mptcp_init_sock(struct sock *sk)
        msk->rmem_fwd_alloc = 0;
        WRITE_ONCE(msk->rmem_released, 0);
        msk->timer_ival = TCP_RTO_MIN;
+       msk->scaling_ratio = (1200 << TCP_RMEM_TO_WIN_SCALE) /
+                            SKB_TRUESIZE(4096);
 
        WRITE_ONCE(msk->first, NULL);
        inet_csk(sk)->icsk_sync_mss = mptcp_sync_mss;

@pabeni
Copy link

pabeni commented Sep 26, 2023

@cpaasch: could you please share the c-repro or test the above?

@cpaasch
Copy link
Member Author

cpaasch commented Sep 27, 2023

Yes, this works as well @pabeni !

cpaasch added a commit that referenced this issue Sep 27, 2023
@cpaasch
mptcp: Paulo's fix for #443
331b78e 
Signed-off-by: Christoph Paasch <[email protected]>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 28, 2023
@intel-lab-lkp
Squash-to: "mptcp: give rcvlowat some love"
8bb7278 
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.
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Oct 2, 2023
@intel-lab-lkp
Squash-to: "mptcp: give rcvlowat some love"
c090904 
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)
cpaasch added a commit that referenced this issue Oct 3, 2023
@cpaasch
mptcp: Paulo's fix for #443
b032f8f 
Signed-off-by: Christoph Paasch <[email protected]>
matttbe pushed a commit that referenced this issue Oct 3, 2023
@matttbe
Squash-to: "mptcp: give rcvlowat some love"
4b8e173 
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
Copy link
Member

matttbe commented Oct 3, 2023

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

@matttbe matttbe closed this as completed Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug reproducer Has a simple program to reproduce the bug syzkaller
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@cpaasch @matttbe @pabeni