Re: [PATCH v3] ksmbd: fix use-after-free and NULL deref in smb_grant_oplock()

[ChenXiaoSong]

Looks good to me so far. Others can continue the review.

Thanks,
ChenXiaoSong <chenxiaosong@xxxxxxxxxx>

On 3/17/26 14:52, Werner Kasselman wrote:
> smb_grant_oplock() has two issues in the oplock publication sequence:
>
> 1) opinfo is linked into ci->m_op_list (via opinfo_add) before
>     add_lease_global_list() is called.  If add_lease_global_list()
>     fails (kmalloc returns NULL), the error path frees the opinfo
>     via __free_opinfo() while it is still linked in ci->m_op_list.
>     Concurrent m_op_list readers (opinfo_get_list, or direct iteration
>     in smb_break_all_levII_oplock) dereference the freed node.
>
> 2) opinfo->o_fp is assigned after add_lease_global_list() publishes
>     the opinfo on the global lease list.  A concurrent
>     find_same_lease_key() can walk the lease list and dereference
>     opinfo->o_fp->f_ci while o_fp is still NULL.
>
> Fix by restructuring the publication sequence to eliminate post-publish
> failure:
>
> - Set opinfo->o_fp before any list publication (fixes NULL deref).
> - Preallocate lease_table via alloc_lease_table() before opinfo_add()
>    so add_lease_global_list() becomes infallible after publication.
> - Keep the original m_op_list publication order (opinfo_add before
>    lease list) so concurrent opens via same_client_has_lease() and
>    opinfo_get_list() still see the in-flight grant.
> - Use opinfo_put() instead of __free_opinfo() on err_out so that
>    the RCU-deferred free path is used.
>
> This also requires splitting add_lease_global_list() to take a
> preallocated lease_table and changing its return type from int to void,
> since it can no longer fail.
>
> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
> Fixes: 1dfd062caa16 ("ksmbd: fix use-after-free by using call_rcu() for oplock_info")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Werner Kasselman <werner@xxxxxxxxxxx>