• Lin Ma's avatar
    net/handshake: remove fput() that causes use-after-free · 361b6889
    Lin Ma authored
    A reference underflow is found in TLS handshake subsystem that causes a
    direct use-after-free. Part of the crash log is like below:
    
    [    2.022114] ------------[ cut here ]------------
    [    2.022193] refcount_t: underflow; use-after-free.
    [    2.022288] WARNING: CPU: 0 PID: 60 at lib/refcount.c:28 refcount_warn_saturate+0xbe/0x110
    [    2.022432] Modules linked in:
    [    2.022848] RIP: 0010:refcount_warn_saturate+0xbe/0x110
    [    2.023231] RSP: 0018:ffffc900001bfe18 EFLAGS: 00000286
    [    2.023325] RAX: 0000000000000000 RBX: 0000000000000007 RCX: 00000000ffffdfff
    [    2.023438] RDX: 0000000000000000 RSI: 00000000ffffffea RDI: 0000000000000001
    [    2.023555] RBP: ffff888004c20098 R08: ffffffff82b392c8 R09: 00000000ffffdfff
    [    2.023693] R10: ffffffff82a592e0 R11: ffffffff82b092e0 R12: ffff888004c200d8
    [    2.023813] R13: 0000000000000000 R14: ffff888004c20000 R15: ffffc90000013ca8
    [    2.023930] FS:  0000000000000000(0000) GS:ffff88807dc00000(0000) knlGS:0000000000000000
    [    2.024062] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [    2.024161] CR2: ffff888003601000 CR3: 0000000002a2e000 CR4: 00000000000006f0
    [    2.024275] Call Trace:
    [    2.024322]  <TASK>
    [    2.024367]  ? __warn+0x7f/0x130
    [    2.024430]  ? refcount_warn_saturate+0xbe/0x110
    [    2.024513]  ? report_bug+0x199/0x1b0
    [    2.024585]  ? handle_bug+0x3c/0x70
    [    2.024676]  ? exc_invalid_op+0x18/0x70
    [    2.024750]  ? asm_exc_invalid_op+0x1a/0x20
    [    2.024830]  ? refcount_warn_saturate+0xbe/0x110
    [    2.024916]  ? refcount_warn_saturate+0xbe/0x110
    [    2.024998]  __tcp_close+0x2f4/0x3d0
    [    2.025065]  ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10
    [    2.025168]  tcp_close+0x1f/0x70
    [    2.025231]  inet_release+0x33/0x60
    [    2.025297]  sock_release+0x1f/0x80
    [    2.025361]  handshake_req_cancel_test2+0x100/0x2d0
    [    2.025457]  kunit_try_run_case+0x4c/0xa0
    [    2.025532]  kunit_generic_run_threadfn_adapter+0x15/0x20
    [    2.025644]  kthread+0xe1/0x110
    [    2.025708]  ? __pfx_kthread+0x10/0x10
    [    2.025780]  ret_from_fork+0x2c/0x50
    
    One can enable CONFIG_NET_HANDSHAKE_KUNIT_TEST config to reproduce above
    crash.
    
    The root cause of this bug is that the commit 1ce77c99
    ("net/handshake: Unpin sock->file if a handshake is cancelled") adds one
    additional fput() function. That patch claims that the fput() is used to
    enable sock->file to be freed even when user space never calls DONE.
    
    However, it seems that the intended DONE routine will never give an
    additional fput() of ths sock->file. The existing two of them are just
    used to balance the reference added in sockfd_lookup().
    
    This patch revert the mentioned commit to avoid the use-after-free. The
    patched kernel could successfully pass the KUNIT test and boot to shell.
    
    [    0.733613]     # Subtest: Handshake API tests
    [    0.734029]     1..11
    [    0.734255]         KTAP version 1
    [    0.734542]         # Subtest: req_alloc API fuzzing
    [    0.736104]         ok 1 handshake_req_alloc NULL proto
    [    0.736114]         ok 2 handshake_req_alloc CLASS_NONE
    [    0.736559]         ok 3 handshake_req_alloc CLASS_MAX
    [    0.737020]         ok 4 handshake_req_alloc no callbacks
    [    0.737488]         ok 5 handshake_req_alloc no done callback
    [    0.737988]         ok 6 handshake_req_alloc excessive privsize
    [    0.738529]         ok 7 handshake_req_alloc all good
    [    0.739036]     # req_alloc API fuzzing: pass:7 fail:0 skip:0 total:7
    [    0.739444]     ok 1 req_alloc API fuzzing
    [    0.740065]     ok 2 req_submit NULL req arg
    [    0.740436]     ok 3 req_submit NULL sock arg
    [    0.740834]     ok 4 req_submit NULL sock->file
    [    0.741236]     ok 5 req_lookup works
    [    0.741621]     ok 6 req_submit max pending
    [    0.741974]     ok 7 req_submit multiple
    [    0.742382]     ok 8 req_cancel before accept
    [    0.742764]     ok 9 req_cancel after accept
    [    0.743151]     ok 10 req_cancel after done
    [    0.743510]     ok 11 req_destroy works
    [    0.743882] # Handshake API tests: pass:11 fail:0 skip:0 total:11
    [    0.744205] # Totals: pass:17 fail:0 skip:0 total:17
    Acked-by: default avatarChuck Lever <chuck.lever@oracle.com>
    Fixes: 1ce77c99 ("net/handshake: Unpin sock->file if a handshake is cancelled")
    Signed-off-by: default avatarLin Ma <linma@zju.edu.cn>
    Link: https://lore.kernel.org/r/20230613083204.633896-1-linma@zju.edu.cn
    Link: https://lore.kernel.org/r/20230614015249.987448-1-linma@zju.edu.cnSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    361b6889
handshake.h 2.26 KB