On Mon, Nov 25, 2024 at 4:53 PM Yunseong Kim <yskelg@xxxxxxxxx> wrote: > > A race condition exists between SMB request handling in > `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the > workqueue handler `handle_ksmbd_work()`. This leads to a UAF. > - KASAN: slab-use-after-free Read in handle_ksmbd_work > - KASAN: slab-use-after-free in rtlock_slowlock_locked > > This race condition arises as follows: > - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: > `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` > - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using > `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls > `ksmbd_conn_free()`, which frees `conn`. > - However, after `handle_ksmbd_work()` decrements `conn->r_count`, > it may still access `conn->r_count_q` in the following line: > `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` > This results in a UAF, as `conn` has already been freed. > > The discovery of this UAF can be referenced in the following PR for > syzkaller's support for SMB requests. > Link: https://github.com/google/syzkaller/pull/5524 > > Fixes: ee426bfb9d09 ("ksmbd: add refcnt to ksmbd_conn struct") > Cc: linux-cifs@xxxxxxxxxxxxxxx > Cc: stable@xxxxxxxxxxxxxxx # v6.6.55+, v6.10.14+, v6.11.3+ > Cc: syzkaller@xxxxxxxxxxxxxxxx > Signed-off-by: Yunseong Kim <yskelg@xxxxxxxxx> Looks good to me:) Applied it to #ksmbd-for-next-next now. Thanks!
