From 76fa42c004eb95a983bed8fd0e6e0e8428c751a5 Mon Sep 17 00:00:00 2001 From: Yunseong Kim Date: Wed, 5 Aug 2026 02:46:56 +0200 Subject: [PATCH] smb: smbdirect: avoid recursive listen.lock during cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit __smbdirect_socket_schedule_cleanup() and smbdirect_socket_cleanup_work() take sc->listen.lock and walk the listener's pending list, recursing into smbdirect_socket_schedule_cleanup() for each child, and every child takes its own listen.lock inside that walk. This cannot deadlock for real: the nesting is strictly listener into child, a child never listens, so the outer and the inner lock are always different instances. lockdep only tracks lock classes, though; it sees the same class acquired twice and reports a possible recursive locking deadlock. This change therefore works around a lockdep limitation rather than fixing a real hang, but the report is still worth avoiding: lockdep disables itself after the first splat and then hides real locking bugs for the rest of the run. Only a socket that was a listener owns a populated listen.ready/pending list; a child has empty lists and nothing to do in these blocks. Guard both of them with sc->listen.backlog != -1, the "was a listener" marker that smbdirect_socket_destroy() already uses: listen.backlog leaves its initial -1 exactly once, when smbdirect_socket_listen() succeeds. The alternative !sc->accept.listener test reads as "not a listener" while meaning the opposite, and it is also true for an accepted child, whose accept.listener has been cleared on hand-over. With the guard the walk only runs for a listener and never nests a child's listen.lock under it; a pending child stays on its listener's list for the free path (smbdirect_socket_destroy) to reap. [ 741.705044] WARNING: possible recursive locking detected [ 741.705403] 7.1.0-next-20260623+ #75 Not tainted [ 741.705695] -------------------------------------------- [ 741.706022] ksmbd.control/18502 is trying to acquire lock: [ 741.706379] ffff888108d612f8 (&sc->listen.lock){....}-{3:3}, at: __smbdirect_socket_schedule_cleanup+0x719/0xd70 [ 741.707008] [ 741.707008] but task is already holding lock: [ 741.707396] ffff8881087642f8 (&sc->listen.lock){....}-{3:3}, at: __smbdirect_socket_schedule_cleanup+0x719/0xd70 [ 741.708025] [ 741.708025] other info that might help us debug this: [ 741.708448] Possible unsafe locking scenario: [ 741.708448] [ 741.708845] CPU0 [ 741.709016] ---- [ 741.709186] lock(&sc->listen.lock); [ 741.709453] lock(&sc->listen.lock); [ 741.709705] [ 741.709705] *** DEADLOCK *** [ 741.709705] [ 741.710095] May be due to missing lock nesting notation [ 741.710095] [ 741.710663] 6 locks held by ksmbd.control/18502: [ 741.710975] #0: ffff888109e51420 (sb_writers#7){.+.+}-{0:0}, at: vfs_write+0x1e7/0xc70 [ 741.711561] #1: ffff888126ec3880 (&of->mutex){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x1be/0x4d0 [ 741.712147] #2: ffff888102af17b0 (kn->active#45){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x205/0x4d0 [ 741.712803] #3: ffffffff85ad1e00 (ctrl_lock){+.+.}-{4:4}, at: kill_server_store+0x1e0/0x2b0 [ 741.713381] #4: ffffffff85ad41a0 (init_lock){+.+.}-{4:4}, at: ksmbd_conn_transport_destroy+0x5b/0x3c0 [ 741.713995] #5: ffff8881087642f8 (&sc->listen.lock){....}-{3:3}, at: __smbdirect_socket_schedule_cleanup+0x719/0xd70 [ 741.714736] [ 741.714736] stack backtrace: [ 741.715038] CPU: 4 UID: 0 PID: 18502 Comm: ksmbd.control Not tainted 7.1.0-next-20260623+ #75 PREEMPT(lazy) [ 741.715043] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014 [ 741.715046] Call Trace: [ 741.715049] [ 741.715052] dump_stack_lvl+0x77/0xa0 [ 741.715058] print_deadlock_bug+0x279/0x290 [ 741.715065] __lock_acquire+0x272a/0x2e30 [ 741.715070] ? stack_trace_save+0xae/0x100 [ 741.715075] ? smb_direct_logging_vaprintf+0x1a0/0x230 [ 741.715079] ? __pfx_smb_direct_logging_vaprintf+0x10/0x10 [ 741.715082] ? __timer_delete+0x58/0x320 [ 741.715087] lock_acquire+0xd3/0x270 [ 741.715091] ? __smbdirect_socket_schedule_cleanup+0x719/0xd70 [ 741.715095] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f [ 741.715099] _raw_spin_lock_irqsave+0x42/0x60 [ 741.715105] ? __smbdirect_socket_schedule_cleanup+0x719/0xd70 Note the two addresses above: ffff888108d612f8 is the child's lock, ffff8881087642f8 the listener's, always distinct objects. Found with ksmbdzzer [2], a KSMBD fuzzer that drives libFuzzer with a kcov-dataflow [1] coverage vector: it folds each instrumented comparison/argument's runtime operand value together with its PC (the default arm mixes them as pc⊕val) so that a new operand value at a known site counts as new coverage. [1] https://lwn.net/Articles/1077606/ [2] https://github.com/yskzalloc/kcov-dataflow Fixes: dc691b91ad16 ("smb: smbdirect: introduce smbdirect_socket_{listen,accept}()") Signed-off-by: Yunseong Kim Reviewed-by: Stefan Metzmacher Signed-off-by: Namjae Jeon --- fs/smb/smbdirect/socket.c | 42 +++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/fs/smb/smbdirect/socket.c b/fs/smb/smbdirect/socket.c index 39cca7219c4d..8dec47a6603c 100644 --- a/fs/smb/smbdirect/socket.c +++ b/fs/smb/smbdirect/socket.c @@ -305,12 +305,26 @@ void __smbdirect_socket_schedule_cleanup(struct smbdirect_socket *sc, * disconnect all pending and ready sockets * * First we move ready sockets to pending again. + * + * Only a socket that was a listener (listen.backlog != -1) owns a + * populated listen.ready/pending list. Guarding on that also keeps + * lockdep quiet: without it, the listener holds sc->listen.lock while + * the loop recurses into each child psc, which takes psc->listen.lock. + * Those are always different instances of the same lock class -- a + * child never listens, so the nesting is strictly listener -> child + * and cannot really deadlock -- but lockdep only sees the class and + * reports "possible recursive locking". A child has empty listen + * lists and nothing to do here, so skipping it loses nothing, and a + * pending child stays on its listener's list for the free path + * (smbdirect_socket_destroy) to reap. */ - spin_lock_irqsave(&sc->listen.lock, flags); - list_splice_init(&sc->listen.ready, &sc->listen.pending); - list_for_each_entry_safe(psc, tsc, &sc->listen.pending, accept.list) - smbdirect_socket_schedule_cleanup(psc, sc->first_error); - spin_unlock_irqrestore(&sc->listen.lock, flags); + if (sc->listen.backlog != -1) { /* was a listener */ + spin_lock_irqsave(&sc->listen.lock, flags); + list_splice_init(&sc->listen.ready, &sc->listen.pending); + list_for_each_entry_safe(psc, tsc, &sc->listen.pending, accept.list) + smbdirect_socket_schedule_cleanup(psc, sc->first_error); + spin_unlock_irqrestore(&sc->listen.lock, flags); + } switch (sc->status) { case SMBDIRECT_SOCKET_RESOLVE_ADDR_FAILED: @@ -405,12 +419,20 @@ static void smbdirect_socket_cleanup_work(struct work_struct *work) * disconnect all pending and ready sockets * * First we move ready sockets to pending again. + * + * Guarded on listen.backlog != -1 for the same reason as in + * __smbdirect_socket_schedule_cleanup(): only a listener owns a + * populated listen list, and skipping the block for a child avoids + * nesting psc->listen.lock under a listener's listen.lock (different + * instances of one class -- harmless, but lockdep cannot tell). */ - spin_lock_irqsave(&sc->listen.lock, flags); - list_splice_init(&sc->listen.ready, &sc->listen.pending); - list_for_each_entry_safe(psc, tsc, &sc->listen.pending, accept.list) - smbdirect_socket_schedule_cleanup(psc, sc->first_error); - spin_unlock_irqrestore(&sc->listen.lock, flags); + if (sc->listen.backlog != -1) { /* was a listener */ + spin_lock_irqsave(&sc->listen.lock, flags); + list_splice_init(&sc->listen.ready, &sc->listen.pending); + list_for_each_entry_safe(psc, tsc, &sc->listen.pending, accept.list) + smbdirect_socket_schedule_cleanup(psc, sc->first_error); + spin_unlock_irqrestore(&sc->listen.lock, flags); + } switch (sc->status) { case SMBDIRECT_SOCKET_NEGOTIATE_NEEDED: