• Paolo Abeni's avatar
    mptcp: use the workqueue to destroy unaccepted sockets · b6985b9b
    Paolo Abeni authored
    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: 58b09919 ("mptcp: create msk early")
    Cc: stable@vger.kernel.org
    Reported-and-tested-by: default avatarChristoph Paasch <cpaasch@apple.com>
    Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/347Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
    Reviewed-by: default avatarMatthieu Baerts <matthieu.baerts@tessares.net>
    Signed-off-by: default avatarMatthieu Baerts <matthieu.baerts@tessares.net>
    Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    b6985b9b
protocol.c 99.1 KB