Re: [PATCH] smb/server: fix use-after-free in ksmbd_conn_transport_destroy()

> Yes, they are different issues. We still need to continue analyzing the
> BUG_ON issue.

Agreed. I dug into it further on ksmbd-for-next-next and I think there are two
sites that need the same treatment. Data below, and a patch that survives the
reproducer at the end -- though please read the caveat before taking it.

All runs: branch smb3/ksmbd-for-next-next at e7317f3e41, fs/smb/ built against
the 7.2.0-rc7 it is based on, freshly booted guest each time, kernel otherwise
clean (Tainted: [O]=OOT_MODULE only), same lockfuzz2.py.


Site 1: __ksmbd_close_fd()
-------------------------

  branch as-is:  fault after ~1.6 s, 142 lock requests

  kernel BUG at fs/locks.c:312!
  Call trace:
   locks_release_private+0x214/0x2c8 (P)
   locks_free_lock+0x20/0x40
   __ksmbd_close_fd+0x4d8/0xd28          [ksmbd]
   ksmbd_destroy_file_table+0x5c/0xf4    [ksmbd]
   ksmbd_session_destroy+0xbc/0x38c      [ksmbd]
   ksmbd_sessions_deregister+0x450/0x540 [ksmbd]
   ksmbd_server_terminate_conn+0x20/0x40 [ksmbd]

The comment above that loop is the thing that misleads:

/* because the reference count of fp is 0, it is guaranteed that
* there are not accesses to fp->lock_list.
*/

fp's refcount reaching zero rules out further *ksmbd* references. It says
nothing about the VFS blocked-request graph, which fs/locks.c owns and which can
still have another client's pending request chained onto this file_lock.


Site 2: the cross-request unlock path in smb2_lock()
----------------------------------------------------

With only the vfs_cache.c hunk applied, the teardown path is genuinely closed --
the trace no longer goes through __ksmbd_close_fd() -- but the same BUG_ON
reappears:

  branch + vfs_cache.c hunk:  fault after ~118 s, 247 lock requests

  Call trace:
   locks_release_private+0x214/0x2c8 (P)
   locks_free_lock+0x20/0x40
   smb2_lock+0x264c/0x4404               [ksmbd]
   handle_ksmbd_work+0x4dc/0x17dc        [ksmbd]

smb2_lock+0x264c is fs/smb/server/smb2pdu.c:9366 on this branch -- the
locks_free_lock(cmp_lock->fl) in the cross-request unlock path, whose only guard
is

static inline bool lock_defer_pending(struct file_lock *fl)
{
/* check pending lock waiters */
return waitqueue_active(&fl->c.flc_wait);
}

That asks "am I waiting?", not "is anyone waiting on me?".
locks_release_private() asserts five separate conditions and this guard covers
one of them (flc_wait); the one that actually trips is flc_blocked_requests.

c120970353 reworked when granted locks get published, which is what fixed the
use-after-free, but it does not change this guard, so this site is still
reachable.


Both hunks together
-------------------

  branch + both hunks:  11185 lock requests over 181 s, no fault,
                        kernel still Tainted: [O] only

For scale, the unpatched branch faults after 142 requests, so that is roughly
79x the request volume with nothing. I also watched locks_sent climb the whole
way (9592 -> 11185 in the last 25 s) rather than plateauing, because a stalled
harness reporting zero faults is exactly how I fooled myself earlier in this
thread.

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -645,6 +645,14 @@ static void __ksmbd_close_fd(struct
ksmbd_file_table *ft, struct ksmbd_file *fp)
  }

  list_del(&smb_lock->flist);
+ /*
+ * Waiters may still be chained onto this lock through the VFS
+ * blocked-request graph. fp's refcount says nothing about that
+ * graph, so detach and wake them before destroying the lock;
+ * otherwise locks_release_private() trips
+ * BUG_ON(!list_empty(&flc->flc_blocked_requests)).
+ */
+ ksmbd_vfs_posix_lock_unblock(smb_lock->fl);
  locks_free_lock(smb_lock->fl);
  kfree(smb_lock);
  }

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -9362,6 +9362,13 @@ int smb2_lock(struct ksmbd_work *work)
  up_read(&conn_list_lock);

  ksmbd_conn_put(conn);
+ /*
+ * Other clients may still be chained onto this
+ * lock in the VFS blocked-request graph;
+ * lock_defer_pending() only covers flc_wait.
+ * Detach and wake them before destroying it.
+ */
+ ksmbd_vfs_posix_lock_unblock(cmp_lock->fl);
  locks_free_lock(cmp_lock->fl);
  kfree(cmp_lock);
  goto out_check_cl;

2026년 8월 13일 (목) 오후 9:38, ChenXiaoSong <[email protected]>님이 작성:
>
> Fixes: b38f99c1217a ("ksmbd: add procfs interface for runtime monitoring
> and statistics")
>
> 在 2026/8/13 18:40, ChenXiaoSong 写道:
> > From: ChenXiaoSong <[email protected]>
> >
> > Reproducer (Link[1]):
> >
> >    1. Build kernel with CONFIG_KASAN=y
> >    2. server: systemctl start ksmbd
> >    3. client: mount -t cifs //localhost/export /mnt
> >    4. client: umount /mnt
> >    5. server: modprobe -r ksmbd
> >
> > The error message is as follows:
> >
> >    ==================================================================
> >    BUG: KASAN: slab-use-after-free in proc_remove+0x3e/0x80
> >    Read of size 8 at addr ffff88810654e098 by task modprobe/785
> >    ...
> >    Call Trace:
> >     <TASK>
> >     __dump_stack+0x19/0x30
> >     dump_stack_lvl+0x49/0x60
> >     print_address_description+0x7b/0x200
> >     print_report+0x5b/0x70
> >     kasan_report+0xed/0x130
> >     __asan_report_load8_noabort+0x18/0x20
> >     proc_remove+0x3e/0x80
> >     ksmbd_conn_transport_destroy+0x2b/0x320 [ksmbd]
> >     cleanup_module+0x33/0xe00 [ksmbd]
> >     __se_sys_delete_module+0x276/0x400
> >     __x64_sys_delete_module+0x5f/0x70
> >     x64_sys_call+0x2675/0x3030
> >     do_syscall_64+0xf0/0x3b0
> >     entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >    RIP: 0033:0x7f5b56d2b02b
> >    ...
> >     </TASK>
> >
> >    Allocated by task 159:
> >     kasan_save_track+0x2f/0x70
> >     kasan_save_alloc_info+0x40/0x50
> >     __kasan_slab_alloc+0x52/0x70
> >     kmem_cache_alloc_noprof+0x168/0x3e0
> >     __proc_create+0x20b/0x710
> >     proc_create_single_data+0x78/0x150
> >     ksmbd_proc_create+0x24/0x30 [ksmbd]
> >     ksmbd_conn_transport_init+0x4f/0x80 [ksmbd]
> >     server_ctrl_handle_work+0x64/0x2c0 [ksmbd]
> >     process_scheduled_works+0x788/0xec0
> >     worker_thread+0x894/0xc10
> >     kthread+0x2e5/0x3c0
> >     ret_from_fork+0x168/0x4f0
> >     ret_from_fork_asm+0x1a/0x30
> >
> >    Freed by task 785:
> >     kasan_save_track+0x2f/0x70
> >     kasan_save_free_info+0x4a/0x60
> >     __kasan_slab_free+0x47/0x70
> >     kmem_cache_free+0x122/0x410
> >     pde_put+0xfd/0x160
> >     remove_proc_subtree+0x365/0x540
> >     proc_remove+0x6a/0x80
> >     ksmbd_proc_cleanup+0x1f/0x60 [ksmbd]
> >     cleanup_module+0x18/0xe00 [ksmbd]
> >     __se_sys_delete_module+0x276/0x400
> >     __x64_sys_delete_module+0x5f/0x70
> >     x64_sys_call+0x2675/0x3030
> >     do_syscall_64+0xf0/0x3b0
> >     entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >    ==================================================================
> >
> > Reported-by: Kyenghwan Hwang <[email protected]>
> > Link[1]: https://lore.kernel.org/linux-cifs/[email protected]/
> > Signed-off-by: ChenXiaoSong <[email protected]>
> > ---
> >   fs/smb/server/server.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
> > index 0ccd123ba418..d619d1f22601 100644
> > --- a/fs/smb/server/server.c
> > +++ b/fs/smb/server/server.c
> > @@ -596,11 +596,16 @@ static int ksmbd_server_shutdown(void)
> >   {
> >       WRITE_ONCE(server_conf.state, SERVER_STATE_SHUTTING_DOWN);
> >
> > -     ksmbd_proc_cleanup();
> >       class_unregister(&ksmbd_control_class);
> >       ksmbd_workqueue_destroy();
> >       ksmbd_ipc_release();
> >       ksmbd_conn_transport_destroy();
> > +     /*
> > +      * ksmbd_conn_transport_destroy() calls delete_proc_clients() and destroys
> > +      * sessions. ksmbd_session_destroy() removes each session's proc entry.
> > +      * Keep the procfs tree alive until these entries have been removed.
> > +      */
> > +     ksmbd_proc_cleanup();
> >       ksmbd_crypto_destroy();
> >       ksmbd_free_global_file_table();
> >       destroy_lease_table(NULL);
>
> --
> ChenXiaoSong <[email protected]>
> Chinese Homepage: https://chenxiaosong.com
> English Homepage: https://chenxiaosong.com/en
>