Re: [PATCH v2] ksmbd: fix memory leaks and NULL deref in smb2_lock()

[ChenXiaoSong <chenxiaosong@chenxiaosong.com>]

Looks good. Feel free to add:
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>

On 3/17/26 17:46, Werner Kasselman wrote:
> smb2_lock() has three error handling issues after list_del() detaches
> smb_lock from lock_list at no_check_cl:
> 
> 1) If vfs_lock_file() returns an unexpected error in the non-UNLOCK
>     path, goto out leaks smb_lock and its flock because the out:
>     handler only iterates lock_list and rollback_list, neither of
>     which contains the detached smb_lock.
> 
> 2) If vfs_lock_file() returns -ENOENT in the UNLOCK path, goto out
>     leaks smb_lock and flock for the same reason.  The error code
>     returned to the dispatcher is also stale.
> 
> 3) In the rollback path, smb_flock_init() can return NULL on
>     allocation failure.  The result is dereferenced unconditionally,
>     causing a kernel NULL pointer dereference.  Add a NULL check to
>     prevent the crash and clean up the bookkeeping; the VFS lock
>     itself cannot be rolled back without the allocation and will be
>     released at file or connection teardown.
> 
> Fix cases 1 and 2 by hoisting the locks_free_lock()/kfree() to before
> the if(!rc) check in the UNLOCK branch so all exit paths share one
> free site, and by freeing smb_lock and flock before goto out in the
> non-UNLOCK branch.  Propagate the correct error code in both cases.
> Fix case 3 by wrapping the VFS unlock in an if(rlock) guard and adding
> a NULL check for locks_free_lock(rlock) in the shared cleanup.
> 
> Found via call-graph analysis using sqry.
> 
> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
> Cc:stable@vger.kernel.org
> Suggested-by: ChenXiaoSong<chenxiaosong@kylinos.cn>
> Signed-off-by: Werner Kasselman<werner@verivus.com>
> ---
>   fs/smb/server/smb2pdu.c | 27 ++++++++++++++++++---------
>   1 file changed, 18 insertions(+), 9 deletions(-)