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.

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

syzkaller: KASAN: use-after-free Read in __token_bucket_busy #347

Closed
cpaasch opened this issue Feb 1, 2023 · 8 comments
Closed

syzkaller: KASAN: use-after-free Read in __token_bucket_busy #347

cpaasch opened this issue Feb 1, 2023 · 8 comments
Labels
bug reproducer Has a simple program to reproduce the bug syzkaller
Projects

Comments

@cpaasch
Copy link
Member

cpaasch commented Feb 1, 2023

HEAD: ab24eb4 ("DO-NOT-MERGE: mptcp: enabled by default") + 5 commits:

9af4eaa31c1f ("Revert "inet6: Remove inet6_destroy_sock() in sk->sk_prot->destroy()."")
121590fdd8aa ("Revert "dccp: Call inet6_destroy_sock() via sk->sk_destruct()."")
2e2384e1c087 ("Revert "sctp: Call inet6_destroy_sock() via sk->sk_destruct()."")
0c0512519c1c ("Revert "inet6: Remove inet6_destroy_sock()."")
b6c5bd1b7c5b ("Revert "inet6: Clean up failure path in do_ipv6_setsockopt()."")

==================================================================
BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x6e/0x91
 print_report+0x16a/0x46f
 kasan_report+0xad/0x130
 __token_bucket_busy+0x253/0x260
 mptcp_token_new_connect+0x13d/0x490
 mptcp_connect+0x4ed/0x860
 __inet_stream_connect+0x80e/0xd90
 tcp_sendmsg_fastopen+0x3ce/0x710
 mptcp_sendmsg+0xff1/0x1a20
 inet_sendmsg+0x11d/0x140
 __sys_sendto+0x405/0x490
 __x64_sys_sendto+0xdc/0x1b0
 do_syscall_64+0x3b/0x90
 entry_SYSCALL_64_after_hwframe+0x72/0xdc
RIP: 0033:0x7fca6d2d5e79
Code: 08 44 89 e0 5b 41 5c c3 66 0f 1f 84 00 00 00 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 6f df 0e 00 f7 d8 64 89 01 48
RSP: 002b:00007ffd1c225fc8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007fca6d2d5e79
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000004
RBP: 0000000000000000 R08: 0000000020000000 R09: 0000000000000010
R10: 000000002000c000 R11: 0000000000000246 R12: 0000000000000862
R13: 431bde82d7b634db R14: 00007fca6d40aaa0 R15: 0000000000405dd0
 </TASK>

Allocated by task 2726:
 kasan_save_stack+0x1e/0x40
 kasan_set_track+0x21/0x30
 __kasan_kmalloc+0x7e/0x90
 __kmalloc+0x56/0x130
 sk_prot_alloc.constprop.0+0x127/0x210
 sk_alloc+0x2d/0x480
 inet_create+0x2ae/0xca0
 __sock_create+0x1ec/0x440
 __sys_socket+0x133/0x250
 __x64_sys_socket+0x6e/0xb0
 do_syscall_64+0x3b/0x90
 entry_SYSCALL_64_after_hwframe+0x72/0xdc

Freed by task 0:
 kasan_save_stack+0x1e/0x40
 kasan_set_track+0x21/0x30
 kasan_save_free_info+0x2a/0x50
 ____kasan_slab_free+0x146/0x1c0
 __kmem_cache_free+0x138/0x270
 __sk_destruct+0x4a4/0x680
 rcu_core+0x5a3/0x1880
 __do_softirq+0x1a6/0x5af

Last potentially related work creation:
 kasan_save_stack+0x1e/0x40
 __kasan_record_aux_stack+0x9f/0xb0
 __call_rcu_common.constprop.0+0x6a/0xa00
 sk_destruct+0x8e/0xe0
 __sk_free+0xed/0x3d0
 sk_free+0x78/0xa0
 mptcp_close+0x127/0x150
 inet_release+0xe9/0x1f0
 __sock_release+0xd2/0x280
 sock_close+0x15/0x20
 __fput+0x252/0xa20
 task_work_run+0x169/0x250
 exit_to_user_mode_prepare+0x113/0x120
 syscall_exit_to_user_mode+0x1d/0x40
 do_syscall_64+0x48/0x90
 entry_SYSCALL_64_after_hwframe+0x72/0xdc

Second to last potentially related work creation:
 kasan_save_stack+0x1e/0x40
 __kasan_record_aux_stack+0x9f/0xb0
 __call_rcu_common.constprop.0+0x6a/0xa00
 sk_destruct+0x8e/0xe0
 __sk_free+0xed/0x3d0
 sk_free+0x78/0xa0
 subflow_ulp_release+0x1fa/0x260
 tcp_cleanup_ulp+0x7a/0x130
 tcp_v4_destroy_sock+0x88/0x5f0
 inet_csk_destroy_sock+0x199/0x320
 inet_csk_reqsk_queue_add+0x1f5/0x250
 tcp_get_cookie_sock+0x2f5/0x860
 cookie_v4_check+0x151c/0x1fe0
 tcp_v4_do_rcv+0x743/0x9d0
 tcp_v4_rcv+0x2c8a/0x2d60
 ip_protocol_deliver_rcu+0x2b/0x250
 ip_local_deliver+0x383/0x4b0
 ip_rcv+0x145/0x190
 __netif_receive_skb_one_core+0x19b/0x1f0
 __netif_receive_skb+0x1f/0x1c0
 process_backlog+0x1b2/0x510
 __napi_poll+0xb5/0x540
 net_rx_action+0x897/0xb90
 __do_softirq+0x1a6/0x5af

The buggy address belongs to the object at ffff88810698d000
 which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 1456 bytes inside of
 2048-byte region [ffff88810698d000, ffff88810698d800)

The buggy address belongs to the physical page:
page:00000000f72bb4ce refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x106988
head:00000000f72bb4ce order:3 compound_mapcount:0 subpages_mapcount:0 compound_pincount:0
flags: 0x200000000010200(slab|head|node=0|zone=2)
raw: 0200000000010200 ffff888100042000 dead000000000100 dead000000000122
raw: 0000000000000000 0000000000080008 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff88810698d480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88810698d500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88810698d580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                     ^
 ffff88810698d600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88810698d680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

Reproducer:

# {Threaded:false Repeat:true RepeatTimes:0 Procs:1 Slowdown:1 Sandbox:none SandboxArg:0 Leak:false NetInjection:false NetDevices:true NetReset:true Cgroups:true BinfmtMisc:false CloseFDs:true KCSAN:false DevlinkPCI:false NicVF:false USB:false VhciInjection:false Wifi:false IEEE802154:false Sysctl:false UseTmpDir:true HandleSegv:false Repro:false Trace:false LegacyOptions:{Collide:false Fault:false FaultCall:0 FaultNth:0}}
r0 = socket$inet_mptcp(0x2, 0x1, 0x106)
bind$inet(r0, &(0x7f0000002200)={0x2, 0x4e20, @local}, 0x10)
listen(r0, 0x0)
r1 = socket$inet_mptcp(0x2, 0x1, 0x106)
sendto$inet(r1, 0x0, 0x0, 0x2000c000, &(0x7f0000000000)={0x2, 0x4e20, @local}, 0x10)

Kernel-Config-file:
CONFIG_MPTCP_NETNEXT.txt

C-reproducer:
repro.c.txt

@matttbe matttbe added this to Needs triage in MPTCP Bugs via automation Feb 1, 2023
@cpaasch
Copy link
Member Author

cpaasch commented Feb 7, 2023

==================================================================
BUG: KASAN: use-after-free in __token_bucket_busy (token.c:74 (inlined by) token.c:82)
Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl (dump_stack.c:107 (discriminator 1))
print_report (report.c:307 (inlined by) report.c:417)
kasan_report (report.c:184 (inlined by) report.c:519)
__token_bucket_busy (token.c:74 (inlined by) token.c:82)
mptcp_token_new_connect (token.c:165)
mptcp_connect (protocol.c:3619 (discriminator 1))
__inet_stream_connect (af_inet.c:669)
tcp_sendmsg_fastopen (tcp.c:1206)
mptcp_sendmsg (protocol.c:1740 (inlined by) protocol.c:1778)
inet_sendmsg (af_inet.c:832 (discriminator 5))
__sys_sendto (socket.c:722 (inlined by) socket.c:745 (inlined by) socket.c:2142)
__x64_sys_sendto (socket.c:2150)
do_syscall_64 (common.c:50 (inlined by) common.c:80)
entry_SYSCALL_64_after_hwframe (entry_64.S:120)
RIP: 0033:0x7fca6d2d5e79
Code: 08 44 89 e0 5b 41 5c c3 66 0f 1f 84 00 00 00 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 6f df 0e 00 f7 d8 64 89 01 48
All code
========
   0:	08 44 89 e0          	or     %al,-0x20(%rcx,%rcx,4)
   4:	5b                   	pop    %rbx
   5:	41 5c                	pop    %r12
   7:	c3                   	retq
   8:	66 0f 1f 84 00 00 00 	nopw   0x0(%rax,%rax,1)
   f:	00 00
  11:	48 89 f8             	mov    %rdi,%rax
  14:	48 89 f7             	mov    %rsi,%rdi
  17:	48 89 d6             	mov    %rdx,%rsi
  1a:	48 89 ca             	mov    %rcx,%rdx
  1d:	4d 89 c2             	mov    %r8,%r10
  20:	4d 89 c8             	mov    %r9,%r8
  23:	4c 8b 4c 24 08       	mov    0x8(%rsp),%r9
  28:	0f 05                	syscall
  2a:*	48 3d 01 f0 ff ff    	cmp    $0xfffffffffffff001,%rax		<-- trapping instruction
  30:	73 01                	jae    0x33
  32:	c3                   	retq
  33:	48 8b 0d 6f df 0e 00 	mov    0xedf6f(%rip),%rcx        # 0xedfa9
  3a:	f7 d8                	neg    %eax
  3c:	64 89 01             	mov    %eax,%fs:(%rcx)
  3f:	48                   	rex.W

Code starting with the faulting instruction
===========================================
   0:	48 3d 01 f0 ff ff    	cmp    $0xfffffffffffff001,%rax
   6:	73 01                	jae    0x9
   8:	c3                   	retq
   9:	48 8b 0d 6f df 0e 00 	mov    0xedf6f(%rip),%rcx        # 0xedf7f
  10:	f7 d8                	neg    %eax
  12:	64 89 01             	mov    %eax,%fs:(%rcx)
  15:	48                   	rex.W
RSP: 002b:00007ffd1c225fc8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007fca6d2d5e79
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000004
RBP: 0000000000000000 R08: 0000000020000000 R09: 0000000000000010
R10: 000000002000c000 R11: 0000000000000246 R12: 0000000000000862
R13: 431bde82d7b634db R14: 00007fca6d40aaa0 R15: 0000000000405dd0
</TASK>

Allocated by task 2726:
kasan_save_stack (common.c:46)
kasan_set_track (common.c:52)
__kasan_kmalloc (common.c:381)
__kmalloc (kasan.h:211 (inlined by) slab_common.c:968 (inlined by) slab_common.c:981)
sk_prot_alloc.constprop.0 (slab.h:584 (inlined by) sock.c:2038)
sk_alloc (sock.c:2091)
inet_create (af_inet.c:322 (inlined by) af_inet.c:245)
__sock_create (socket.c:1541)
__sys_socket (socket.c:1629 (inlined by) socket.c:1613 (inlined by) socket.c:1661)
__x64_sys_socket (socket.c:1672)
do_syscall_64 (common.c:50 (inlined by) common.c:80)
entry_SYSCALL_64_after_hwframe (entry_64.S:120)

Freed by task 0:
kasan_save_stack (common.c:46)
kasan_set_track (common.c:52)
kasan_save_free_info (generic.c:520)
____kasan_slab_free (common.c:238 (inlined by) common.c:200)
__kmem_cache_free (slub.c:1807 (inlined by) slub.c:3787 (inlined by) slub.c:3800)
__sk_destruct (sock.c:2074 (inlined by) sock.c:2166)
rcu_core (preempt.h:27 (inlined by) tree.c:2253 (inlined by) tree.c:2506)
__do_softirq (jump_label.h:27 (inlined by) jump_label.h:207 (inlined by) irq.h:142 (inlined by) softirq.c:572)

Last potentially related work creation:
kasan_save_stack (common.c:46)
__kasan_record_aux_stack (generic.c:488)
__call_rcu_common.constprop.0 (irqflags.h:29 (inlined by) irqflags.h:70 (inlined by) irqflags.h:106 (inlined by) tree.c:2756)
sk_destruct (sock.c:2182)
__sk_free (sock.c:2193)
sk_free (sock.c:2204)
mptcp_close (protocol.c:3051)
inet_release (af_inet.c:432)
__sock_release (socket.c:652)
sock_close (socket.c:1392)
__fput (file_table.c:321)
task_work_run (task_work.c:180 (discriminator 1))
exit_to_user_mode_prepare (resume_user_mode.h:49 (inlined by) common.c:171 (inlined by) common.c:203)
syscall_exit_to_user_mode (jump_label.h:55 (inlined by) nospec-branch.h:558 (inlined by) entry-common.h:94 (inlined by) common.c:135 (inlined by) common.c:298)
do_syscall_64 (common.c:87)
entry_SYSCALL_64_after_hwframe (entry_64.S:120)

Second to last potentially related work creation:
kasan_save_stack (common.c:46)
__kasan_record_aux_stack (generic.c:488)
__call_rcu_common.constprop.0 (irqflags.h:29 (inlined by) irqflags.h:70 (inlined by) irqflags.h:106 (inlined by) tree.c:2756)
sk_destruct (sock.c:2182)
__sk_free (sock.c:2193)
sk_free (sock.c:2204)
subflow_ulp_release (sock.h:1991 (inlined by) subflow.c:1952)
tcp_cleanup_ulp (tcp_ulp.c:125)
tcp_v4_destroy_sock (tcp_ipv4.c:2301)
inet_csk_destroy_sock (inet_connection_sock.c:1198 (discriminator 9))
inet_csk_reqsk_queue_add (inet_connection_sock.c:1307)
tcp_get_cookie_sock (syncookies.c:219)
cookie_v4_check (syncookies.c:448)
tcp_v4_do_rcv (tcp_ipv4.c:1670 (inlined by) tcp_ipv4.c:1730)
tcp_v4_rcv (tcp_ipv4.c:2132)
ip_protocol_deliver_rcu (ip_input.c:205 (discriminator 1))
ip_local_deliver (rcupdate.h:796 (inlined by) ip_input.c:234 (inlined by) netfilter.h:411 (inlined by) ip_input.c:254)
ip_rcv (dst.h:454 (inlined by) ip_input.c:449 (inlined by) netfilter.h:411 (inlined by) ip_input.c:569)
__netif_receive_skb_one_core (dev.c:5482 (discriminator 4))
__netif_receive_skb (dev.c:5596)
process_backlog (rcupdate.h:796 (inlined by) dev.c:5925)
__napi_poll (dev.c:6485)
net_rx_action (dev.c:6554 (inlined by) dev.c:6662)
__do_softirq (jump_label.h:27 (inlined by) jump_label.h:207 (inlined by) irq.h:142 (inlined by) softirq.c:572)

The buggy address belongs to the object at ffff88810698d000
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 1456 bytes inside of
2048-byte region [ffff88810698d000, ffff88810698d800)

The buggy address belongs to the physical page:
page:00000000f72bb4ce refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x106988
head:00000000f72bb4ce order:3 compound_mapcount:0 subpages_mapcount:0 compound_pincount:0
flags: 0x200000000010200(slab|head|node=0|zone=2)
raw: 0200000000010200 ffff888100042000 dead000000000100 dead000000000122
raw: 0000000000000000 0000000000080008 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff88810698d480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88810698d500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88810698d580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88810698d600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88810698d680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

@pabeni
Copy link

pabeni commented Feb 9, 2023

while traversing the hash bucket, __token_bucket_busy() is touching an already freed - by inet_child_forget() ->
inet_csk_destroy_sock() -> subflow_ulp_release() -> sock_put() - msk. AFAICS the relevant token manipulation functions correctly implement the RCU rules.

The issue happens when the msk is still unaccepted and thus has a single reference (from the mpc subflow). Note that the inet_child_forget() call path does not allow to call/trigger the MPTCP_WORK_CLOSE_SUBFLOW machinery: the msk is deleted without any possibility for the mptcp protocol impl. to run the proper clean-up (in this case, delete the token).

A tentative solution would be retaining a refcount == 2 at passive msk creation time, so that mpc subflow deletion does not destroy the msk socket - and hooking MPTCP_WORK_CLOSE_SUBFLOW into the inet_child_forget call path, so that the the full msk cleanup including last reference drop could be triggered in the relevant scenario - the closed subflow is the mpc one and the msk socket is still unaccepted. See the posted patches

@cpaasch
Copy link
Member Author

cpaasch commented Feb 9, 2023

FYI - can't repro right now. Let's see if I trigger new one with a different reproducer.

@cpaasch
Copy link
Member Author

cpaasch commented Feb 9, 2023

I was able to repro on /export-branch.

But, can't repro with fixes for #346.

Thus, can probably be closed as dup of #346

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 9, 2023
Christoph reported a UaF at token lookup time:

BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 torvalds#6
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x6e/0x91
 print_report+0x16a/0x46f
 kasan_report+0xad/0x130
 __token_bucket_busy+0x253/0x260
 mptcp_token_new_connect+0x13d/0x490
 mptcp_connect+0x4ed/0x860
 __inet_stream_connect+0x80e/0xd90
 tcp_sendmsg_fastopen+0x3ce/0x710
 mptcp_sendmsg+0xff1/0x1a20
 inet_sendmsg+0x11d/0x140
 __sys_sendto+0x405/0x490
 __x64_sys_sendto+0xdc/0x1b0
 do_syscall_64+0x3b/0x90
 entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
@matttbe matttbe added the reproducer Has a simple program to reproduce the bug label Feb 16, 2023
matttbe pushed a commit that referenced this issue Feb 17, 2023
Christoph reported a UaF at token lookup time:

    BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
    Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

    CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x6e/0x91
     print_report+0x16a/0x46f
     kasan_report+0xad/0x130
     __token_bucket_busy+0x253/0x260
     mptcp_token_new_connect+0x13d/0x490
     mptcp_connect+0x4ed/0x860
     __inet_stream_connect+0x80e/0xd90
     tcp_sendmsg_fastopen+0x3ce/0x710
     mptcp_sendmsg+0xff1/0x1a20
     inet_sendmsg+0x11d/0x140
     __sys_sendto+0x405/0x490
     __x64_sys_sendto+0xdc/0x1b0
     do_syscall_64+0x3b/0x90
     entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Fixes: 58b0991 ("mptcp: create msk early")
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
MPTCP Bugs automation moved this from Needs triage to Closed Feb 17, 2023
matttbe pushed a commit that referenced this issue Feb 17, 2023
Christoph reported a UaF at token lookup time:

    BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
    Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

    CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x6e/0x91
     print_report+0x16a/0x46f
     kasan_report+0xad/0x130
     __token_bucket_busy+0x253/0x260
     mptcp_token_new_connect+0x13d/0x490
     mptcp_connect+0x4ed/0x860
     __inet_stream_connect+0x80e/0xd90
     tcp_sendmsg_fastopen+0x3ce/0x710
     mptcp_sendmsg+0xff1/0x1a20
     inet_sendmsg+0x11d/0x140
     __sys_sendto+0x405/0x490
     __x64_sys_sendto+0xdc/0x1b0
     do_syscall_64+0x3b/0x90
     entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Fixes: 58b0991 ("mptcp: create msk early")
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
jenkins-tessares pushed a commit that referenced this issue Feb 18, 2023
Christoph reported a UaF at token lookup time:

    BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
    Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

    CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x6e/0x91
     print_report+0x16a/0x46f
     kasan_report+0xad/0x130
     __token_bucket_busy+0x253/0x260
     mptcp_token_new_connect+0x13d/0x490
     mptcp_connect+0x4ed/0x860
     __inet_stream_connect+0x80e/0xd90
     tcp_sendmsg_fastopen+0x3ce/0x710
     mptcp_sendmsg+0xff1/0x1a20
     inet_sendmsg+0x11d/0x140
     __sys_sendto+0x405/0x490
     __x64_sys_sendto+0xdc/0x1b0
     do_syscall_64+0x3b/0x90
     entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Fixes: 58b0991 ("mptcp: create msk early")
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
jenkins-tessares pushed a commit that referenced this issue Feb 18, 2023
Christoph reported a UaF at token lookup time:

    BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
    Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

    CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x6e/0x91
     print_report+0x16a/0x46f
     kasan_report+0xad/0x130
     __token_bucket_busy+0x253/0x260
     mptcp_token_new_connect+0x13d/0x490
     mptcp_connect+0x4ed/0x860
     __inet_stream_connect+0x80e/0xd90
     tcp_sendmsg_fastopen+0x3ce/0x710
     mptcp_sendmsg+0xff1/0x1a20
     inet_sendmsg+0x11d/0x140
     __sys_sendto+0x405/0x490
     __x64_sys_sendto+0xdc/0x1b0
     do_syscall_64+0x3b/0x90
     entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Fixes: 58b0991 ("mptcp: create msk early")
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
matttbe pushed a commit that referenced this issue Feb 20, 2023
Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
matttbe pushed a commit that referenced this issue Feb 20, 2023
Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
matttbe pushed a commit that referenced this issue Feb 20, 2023
Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
matttbe pushed a commit that referenced this issue Feb 20, 2023
Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
jenkins-tessares pushed a commit that referenced this issue Feb 21, 2023
Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: #347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
@cpaasch
Copy link
Member Author

cpaasch commented Feb 21, 2023

With the syzkaller reproducer, I hit a soft lockup...

[   64.674605] watchdog: BUG: soft lockup - CPU#1 stuck for 23s! [syz-executor:3338]
[   64.676438] Modules linked in:
[   64.677168] CPU: 1 PID: 3338 Comm: syz-executor Not tainted 6.2.0-rc8+ #32
[   64.678763] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
[   64.681215] RIP: 0010:mptcp_token_exists+0xd1/0x160
[   64.682338] Code: 35 a0 5b fe 48 89 d8 48 c1 e8 03 42 80 3c 20 00 0f 85 80 00 00 00 48 8b 1b 41 89 df 31 ff 41 83 e7 01 44 89 fe e8 5f 98 5b fe <45> 85 ff 74 8d e8 05 a0 5b fe 48 d1 eb 41 89 ef 44 23 3d 00 3b 12
[   64.686234] RSP: 0018:ffff88811b7094b8 EFLAGS: 00000246
[   64.687355] RAX: 0000000000000000 RBX: 0000000000000ed7 RCX: ffffffff82e0ef41
[   64.688940] RDX: ffff888107841c00 RSI: 0000000000000100 RDI: 0000000000000005
[   64.690639] RBP: 000000008e935705 R08: 0000000000000005 R09: 0000000000000000
[   64.692231] R10: 0000000000000001 R11: 0000000000000000 R12: dffffc0000000000
[   64.693795] R13: ffffed10201a1511 R14: ffff888100d0a878 R15: 0000000000000001
[   64.695455] FS:  00007fde38e36800(0000) GS:ffff88811b700000(0000) knlGS:0000000000000000
[   64.697431] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   64.698852] CR2: 000000000047c550 CR3: 0000000107964000 CR4: 00000000000006e0
[   64.700502] Call Trace:
[   64.701073]  <IRQ>
[   64.701564]  subflow_check_req+0x9d1/0xe70
[   64.702526]  ? __pfx_subflow_check_req+0x10/0x10
[   64.703597]  ? unwind_get_return_address+0x55/0xa0
[   64.704698]  ? __pfx_stack_trace_consume_entry+0x10/0x10
[   64.705929]  ? ip_route_output_flow+0x225/0x2c0
[   64.706974]  ? __pfx_ip_route_output_flow+0x10/0x10
[   64.708086]  ? kmem_cache_alloc+0x177/0x310
[   64.709030]  ? inet_csk_route_req+0x6f4/0x9b0
[   64.710009]  subflow_v4_route_req+0x1f1/0x350
[   64.710991]  tcp_conn_request+0xb29/0x2d40

@matttbe
Copy link
Member

matttbe commented Feb 21, 2023

@cpaasch thank you for that!

On top of net or net-next?

The calltrace looks different from the original one, we will probably need decode_stacktrace.sh :)

I suggest to re-open this ticket.

@matttbe matttbe reopened this Feb 21, 2023
MPTCP Bugs automation moved this from Closed to Needs triage Feb 21, 2023
@cpaasch
Copy link
Member Author

cpaasch commented Feb 22, 2023

I can repro on both, net and netnext.

MPTCP Bugs automation moved this from Needs triage to Closed Feb 22, 2023
@matttbe
Copy link
Member

matttbe commented Feb 22, 2023

Because there is a commit closing this ticket in our export branch, we need to open a new one: #365

ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 22, 2023
commit b6985b9 upstream.

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 22, 2023
commit b6985b9 upstream.

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 25, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 26, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 27, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 27, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 27, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-fork that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 torvalds#6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 28, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 30, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Mar 30, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
gregkh pushed a commit to gregkh/linux that referenced this issue Mar 30, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-fork that referenced this issue Mar 30, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 torvalds#6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
xrh0905 pushed a commit to xrh0905/linux that referenced this issue Apr 14, 2023
[ Upstream commit b6985b9 ]

  Backports notes: one simple conflict in net/mptcp/protocol.c with:

    commit a5ef058 ("net: introduce and use custom sockopt socket flag")

  Where the two commits add a new line for different actions in the same
  context in mptcp_stream_accept().

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 raspberrypi#6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
sileshn pushed a commit to sileshn/ubuntu-kernel-lunar that referenced this issue Apr 19, 2023
BugLink: https://bugs.launchpad.net/bugs/2016876

commit b6985b9b82954caa53f862d6059d06c0526254f0 upstream.

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
mj22226 pushed a commit to mj22226/linux that referenced this issue Apr 21, 2023
Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 torvalds#6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
sileshn pushed a commit to sileshn/ubuntu-kernel-lunar that referenced this issue May 21, 2023
BugLink: https://bugs.launchpad.net/bugs/2016876

commit b6985b9b82954caa53f862d6059d06c0526254f0 upstream.

Christoph reported a UaF at token lookup time after having
refactored the passive socket initialization part:

  BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260
  Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198

  CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6c00c8f1e448ed99a45c66340dd5 #6
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x6e/0x91
   print_report+0x16a/0x46f
   kasan_report+0xad/0x130
   __token_bucket_busy+0x253/0x260
   mptcp_token_new_connect+0x13d/0x490
   mptcp_connect+0x4ed/0x860
   __inet_stream_connect+0x80e/0xd90
   tcp_sendmsg_fastopen+0x3ce/0x710
   mptcp_sendmsg+0xff1/0x1a20
   inet_sendmsg+0x11d/0x140
   __sys_sendto+0x405/0x490
   __x64_sys_sendto+0xdc/0x1b0
   do_syscall_64+0x3b/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

We need to properly clean-up all the paired MPTCP-level
resources and be sure to release the msk last, even when
the unaccepted subflow is destroyed by the TCP internals
via inet_child_forget().

We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra,
explicitly checking that for the critical scenario: the
closed subflow is the MPC one, the msk is not accepted and
eventually going through full cleanup.

With such change, __mptcp_destroy_sock() is always called
on msk sockets, even on accepted ones. We don't need anymore
to transiently drop one sk reference at msk clone time.

Please note this commit depends on the parent one:

  mptcp: refactor passive socket initialization

Fixes: 58b0991 ("mptcp: create msk early")
Cc: stable@vger.kernel.org
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#347
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
jenkins-tessares pushed a commit that referenced this issue Aug 26, 2023
Enable CPU v4 instruction tests for arm64. Below are the test results from
BPF test_progs selftests:

  # ./test_progs -t ldsx_insn,verifier_sdiv,verifier_movsx,verifier_ldsx,verifier_gotol,verifier_bswap
  #115/1   ldsx_insn/map_val and probed_memory:OK
  #115/2   ldsx_insn/ctx_member_sign_ext:OK
  #115/3   ldsx_insn/ctx_member_narrow_sign_ext:OK
  #115     ldsx_insn:OK
  #302/1   verifier_bswap/BSWAP, 16:OK
  #302/2   verifier_bswap/BSWAP, 16 @unpriv:OK
  #302/3   verifier_bswap/BSWAP, 32:OK
  #302/4   verifier_bswap/BSWAP, 32 @unpriv:OK
  #302/5   verifier_bswap/BSWAP, 64:OK
  #302/6   verifier_bswap/BSWAP, 64 @unpriv:OK
  #302     verifier_bswap:OK
  #316/1   verifier_gotol/gotol, small_imm:OK
  #316/2   verifier_gotol/gotol, small_imm @unpriv:OK
  #316     verifier_gotol:OK
  #324/1   verifier_ldsx/LDSX, S8:OK
  #324/2   verifier_ldsx/LDSX, S8 @unpriv:OK
  #324/3   verifier_ldsx/LDSX, S16:OK
  #324/4   verifier_ldsx/LDSX, S16 @unpriv:OK
  #324/5   verifier_ldsx/LDSX, S32:OK
  #324/6   verifier_ldsx/LDSX, S32 @unpriv:OK
  #324/7   verifier_ldsx/LDSX, S8 range checking, privileged:OK
  #324/8   verifier_ldsx/LDSX, S16 range checking:OK
  #324/9   verifier_ldsx/LDSX, S16 range checking @unpriv:OK
  #324/10  verifier_ldsx/LDSX, S32 range checking:OK
  #324/11  verifier_ldsx/LDSX, S32 range checking @unpriv:OK
  #324     verifier_ldsx:OK
  #335/1   verifier_movsx/MOV32SX, S8:OK
  #335/2   verifier_movsx/MOV32SX, S8 @unpriv:OK
  #335/3   verifier_movsx/MOV32SX, S16:OK
  #335/4   verifier_movsx/MOV32SX, S16 @unpriv:OK
  #335/5   verifier_movsx/MOV64SX, S8:OK
  #335/6   verifier_movsx/MOV64SX, S8 @unpriv:OK
  #335/7   verifier_movsx/MOV64SX, S16:OK
  #335/8   verifier_movsx/MOV64SX, S16 @unpriv:OK
  #335/9   verifier_movsx/MOV64SX, S32:OK
  #335/10  verifier_movsx/MOV64SX, S32 @unpriv:OK
  #335/11  verifier_movsx/MOV32SX, S8, range_check:OK
  #335/12  verifier_movsx/MOV32SX, S8, range_check @unpriv:OK
  #335/13  verifier_movsx/MOV32SX, S16, range_check:OK
  #335/14  verifier_movsx/MOV32SX, S16, range_check @unpriv:OK
  #335/15  verifier_movsx/MOV32SX, S16, range_check 2:OK
  #335/16  verifier_movsx/MOV32SX, S16, range_check 2 @unpriv:OK
  #335/17  verifier_movsx/MOV64SX, S8, range_check:OK
  #335/18  verifier_movsx/MOV64SX, S8, range_check @unpriv:OK
  #335/19  verifier_movsx/MOV64SX, S16, range_check:OK
  #335/20  verifier_movsx/MOV64SX, S16, range_check @unpriv:OK
  #335/21  verifier_movsx/MOV64SX, S32, range_check:OK
  #335/22  verifier_movsx/MOV64SX, S32, range_check @unpriv:OK
  #335/23  verifier_movsx/MOV64SX, S16, R10 Sign Extension:OK
  #335/24  verifier_movsx/MOV64SX, S16, R10 Sign Extension @unpriv:OK
  #335     verifier_movsx:OK
  #347/1   verifier_sdiv/SDIV32, non-zero imm divisor, check 1:OK
  #347/2   verifier_sdiv/SDIV32, non-zero imm divisor, check 1 @unpriv:OK
  #347/3   verifier_sdiv/SDIV32, non-zero imm divisor, check 2:OK
  #347/4   verifier_sdiv/SDIV32, non-zero imm divisor, check 2 @unpriv:OK
  #347/5   verifier_sdiv/SDIV32, non-zero imm divisor, check 3:OK
  #347/6   verifier_sdiv/SDIV32, non-zero imm divisor, check 3 @unpriv:OK
  #347/7   verifier_sdiv/SDIV32, non-zero imm divisor, check 4:OK
  #347/8   verifier_sdiv/SDIV32, non-zero imm divisor, check 4 @unpriv:OK
  #347/9   verifier_sdiv/SDIV32, non-zero imm divisor, check 5:OK
  #347/10  verifier_sdiv/SDIV32, non-zero imm divisor, check 5 @unpriv:OK
  #347/11  verifier_sdiv/SDIV32, non-zero imm divisor, check 6:OK
  #347/12  verifier_sdiv/SDIV32, non-zero imm divisor, check 6 @unpriv:OK
  #347/13  verifier_sdiv/SDIV32, non-zero imm divisor, check 7:OK
  #347/14  verifier_sdiv/SDIV32, non-zero imm divisor, check 7 @unpriv:OK
  #347/15  verifier_sdiv/SDIV32, non-zero imm divisor, check 8:OK
  #347/16  verifier_sdiv/SDIV32, non-zero imm divisor, check 8 @unpriv:OK
  #347/17  verifier_sdiv/SDIV32, non-zero reg divisor, check 1:OK
  #347/18  verifier_sdiv/SDIV32, non-zero reg divisor, check 1 @unpriv:OK
  #347/19  verifier_sdiv/SDIV32, non-zero reg divisor, check 2:OK
  #347/20  verifier_sdiv/SDIV32, non-zero reg divisor, check 2 @unpriv:OK
  #347/21  verifier_sdiv/SDIV32, non-zero reg divisor, check 3:OK
  #347/22  verifier_sdiv/SDIV32, non-zero reg divisor, check 3 @unpriv:OK
  #347/23  verifier_sdiv/SDIV32, non-zero reg divisor, check 4:OK
  #347/24  verifier_sdiv/SDIV32, non-zero reg divisor, check 4 @unpriv:OK
  #347/25  verifier_sdiv/SDIV32, non-zero reg divisor, check 5:OK
  #347/26  verifier_sdiv/SDIV32, non-zero reg divisor, check 5 @unpriv:OK
  #347/27  verifier_sdiv/SDIV32, non-zero reg divisor, check 6:OK
  #347/28  verifier_sdiv/SDIV32, non-zero reg divisor, check 6 @unpriv:OK
  #347/29  verifier_sdiv/SDIV32, non-zero reg divisor, check 7:OK
  #347/30  verifier_sdiv/SDIV32, non-zero reg divisor, check 7 @unpriv:OK
  #347/31  verifier_sdiv/SDIV32, non-zero reg divisor, check 8:OK
  #347/32  verifier_sdiv/SDIV32, non-zero reg divisor, check 8 @unpriv:OK
  #347/33  verifier_sdiv/SDIV64, non-zero imm divisor, check 1:OK
  #347/34  verifier_sdiv/SDIV64, non-zero imm divisor, check 1 @unpriv:OK
  #347/35  verifier_sdiv/SDIV64, non-zero imm divisor, check 2:OK
  #347/36  verifier_sdiv/SDIV64, non-zero imm divisor, check 2 @unpriv:OK
  #347/37  verifier_sdiv/SDIV64, non-zero imm divisor, check 3:OK
  #347/38  verifier_sdiv/SDIV64, non-zero imm divisor, check 3 @unpriv:OK
  #347/39  verifier_sdiv/SDIV64, non-zero imm divisor, check 4:OK
  #347/40  verifier_sdiv/SDIV64, non-zero imm divisor, check 4 @unpriv:OK
  #347/41  verifier_sdiv/SDIV64, non-zero imm divisor, check 5:OK
  #347/42  verifier_sdiv/SDIV64, non-zero imm divisor, check 5 @unpriv:OK
  #347/43  verifier_sdiv/SDIV64, non-zero imm divisor, check 6:OK
  #347/44  verifier_sdiv/SDIV64, non-zero imm divisor, check 6 @unpriv:OK
  #347/45  verifier_sdiv/SDIV64, non-zero reg divisor, check 1:OK
  #347/46  verifier_sdiv/SDIV64, non-zero reg divisor, check 1 @unpriv:OK
  #347/47  verifier_sdiv/SDIV64, non-zero reg divisor, check 2:OK
  #347/48  verifier_sdiv/SDIV64, non-zero reg divisor, check 2 @unpriv:OK
  #347/49  verifier_sdiv/SDIV64, non-zero reg divisor, check 3:OK
  #347/50  verifier_sdiv/SDIV64, non-zero reg divisor, check 3 @unpriv:OK
  #347/51  verifier_sdiv/SDIV64, non-zero reg divisor, check 4:OK
  #347/52  verifier_sdiv/SDIV64, non-zero reg divisor, check 4 @unpriv:OK
  #347/53  verifier_sdiv/SDIV64, non-zero reg divisor, check 5:OK
  #347/54  verifier_sdiv/SDIV64, non-zero reg divisor, check 5 @unpriv:OK
  #347/55  verifier_sdiv/SDIV64, non-zero reg divisor, check 6:OK
  #347/56  verifier_sdiv/SDIV64, non-zero reg divisor, check 6 @unpriv:OK
  #347/57  verifier_sdiv/SMOD32, non-zero imm divisor, check 1:OK
  #347/58  verifier_sdiv/SMOD32, non-zero imm divisor, check 1 @unpriv:OK
  #347/59  verifier_sdiv/SMOD32, non-zero imm divisor, check 2:OK
  #347/60  verifier_sdiv/SMOD32, non-zero imm divisor, check 2 @unpriv:OK
  #347/61  verifier_sdiv/SMOD32, non-zero imm divisor, check 3:OK
  #347/62  verifier_sdiv/SMOD32, non-zero imm divisor, check 3 @unpriv:OK
  #347/63  verifier_sdiv/SMOD32, non-zero imm divisor, check 4:OK
  #347/64  verifier_sdiv/SMOD32, non-zero imm divisor, check 4 @unpriv:OK
  #347/65  verifier_sdiv/SMOD32, non-zero imm divisor, check 5:OK
  #347/66  verifier_sdiv/SMOD32, non-zero imm divisor, check 5 @unpriv:OK
  #347/67  verifier_sdiv/SMOD32, non-zero imm divisor, check 6:OK
  #347/68  verifier_sdiv/SMOD32, non-zero imm divisor, check 6 @unpriv:OK
  #347/69  verifier_sdiv/SMOD32, non-zero reg divisor, check 1:OK
  #347/70  verifier_sdiv/SMOD32, non-zero reg divisor, check 1 @unpriv:OK
  #347/71  verifier_sdiv/SMOD32, non-zero reg divisor, check 2:OK
  #347/72  verifier_sdiv/SMOD32, non-zero reg divisor, check 2 @unpriv:OK
  #347/73  verifier_sdiv/SMOD32, non-zero reg divisor, check 3:OK
  #347/74  verifier_sdiv/SMOD32, non-zero reg divisor, check 3 @unpriv:OK
  #347/75  verifier_sdiv/SMOD32, non-zero reg divisor, check 4:OK
  #347/76  verifier_sdiv/SMOD32, non-zero reg divisor, check 4 @unpriv:OK
  #347/77  verifier_sdiv/SMOD32, non-zero reg divisor, check 5:OK
  #347/78  verifier_sdiv/SMOD32, non-zero reg divisor, check 5 @unpriv:OK
  #347/79  verifier_sdiv/SMOD32, non-zero reg divisor, check 6:OK
  #347/80  verifier_sdiv/SMOD32, non-zero reg divisor, check 6 @unpriv:OK
  #347/81  verifier_sdiv/SMOD64, non-zero imm divisor, check 1:OK
  #347/82  verifier_sdiv/SMOD64, non-zero imm divisor, check 1 @unpriv:OK
  #347/83  verifier_sdiv/SMOD64, non-zero imm divisor, check 2:OK
  #347/84  verifier_sdiv/SMOD64, non-zero imm divisor, check 2 @unpriv:OK
  #347/85  verifier_sdiv/SMOD64, non-zero imm divisor, check 3:OK
  #347/86  verifier_sdiv/SMOD64, non-zero imm divisor, check 3 @unpriv:OK
  #347/87  verifier_sdiv/SMOD64, non-zero imm divisor, check 4:OK
  #347/88  verifier_sdiv/SMOD64, non-zero imm divisor, check 4 @unpriv:OK
  #347/89  verifier_sdiv/SMOD64, non-zero imm divisor, check 5:OK
  #347/90  verifier_sdiv/SMOD64, non-zero imm divisor, check 5 @unpriv:OK
  #347/91  verifier_sdiv/SMOD64, non-zero imm divisor, check 6:OK
  #347/92  verifier_sdiv/SMOD64, non-zero imm divisor, check 6 @unpriv:OK
  #347/93  verifier_sdiv/SMOD64, non-zero imm divisor, check 7:OK
  #347/94  verifier_sdiv/SMOD64, non-zero imm divisor, check 7 @unpriv:OK
  #347/95  verifier_sdiv/SMOD64, non-zero imm divisor, check 8:OK
  #347/96  verifier_sdiv/SMOD64, non-zero imm divisor, check 8 @unpriv:OK
  #347/97  verifier_sdiv/SMOD64, non-zero reg divisor, check 1:OK
  #347/98  verifier_sdiv/SMOD64, non-zero reg divisor, check 1 @unpriv:OK
  #347/99  verifier_sdiv/SMOD64, non-zero reg divisor, check 2:OK
  #347/100 verifier_sdiv/SMOD64, non-zero reg divisor, check 2 @unpriv:OK
  #347/101 verifier_sdiv/SMOD64, non-zero reg divisor, check 3:OK
  #347/102 verifier_sdiv/SMOD64, non-zero reg divisor, check 3 @unpriv:OK
  #347/103 verifier_sdiv/SMOD64, non-zero reg divisor, check 4:OK
  #347/104 verifier_sdiv/SMOD64, non-zero reg divisor, check 4 @unpriv:OK
  #347/105 verifier_sdiv/SMOD64, non-zero reg divisor, check 5:OK
  #347/106 verifier_sdiv/SMOD64, non-zero reg divisor, check 5 @unpriv:OK
  #347/107 verifier_sdiv/SMOD64, non-zero reg divisor, check 6:OK
  #347/108 verifier_sdiv/SMOD64, non-zero reg divisor, check 6 @unpriv:OK
  #347/109 verifier_sdiv/SMOD64, non-zero reg divisor, check 7:OK
  #347/110 verifier_sdiv/SMOD64, non-zero reg divisor, check 7 @unpriv:OK
  #347/111 verifier_sdiv/SMOD64, non-zero reg divisor, check 8:OK
  #347/112 verifier_sdiv/SMOD64, non-zero reg divisor, check 8 @unpriv:OK
  #347/113 verifier_sdiv/SDIV32, zero divisor:OK
  #347/114 verifier_sdiv/SDIV32, zero divisor @unpriv:OK
  #347/115 verifier_sdiv/SDIV64, zero divisor:OK
  #347/116 verifier_sdiv/SDIV64, zero divisor @unpriv:OK
  #347/117 verifier_sdiv/SMOD32, zero divisor:OK
  #347/118 verifier_sdiv/SMOD32, zero divisor @unpriv:OK
  #347/119 verifier_sdiv/SMOD64, zero divisor:OK
  #347/120 verifier_sdiv/SMOD64, zero divisor @unpriv:OK
  #347     verifier_sdiv:OK
  Summary: 6/166 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Florent Revest <revest@chromium.org>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Acked-by: Florent Revest <revest@chromium.org>
Link: https://lore.kernel.org/bpf/20230815154158.717901-8-xukuohai@huaweicloud.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug reproducer Has a simple program to reproduce the bug syzkaller
Projects
No open projects
MPTCP Bugs
  
Closed
Development

No branches or pull requests

3 participants