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

[Steve French]

I see that Sashiko had AI review comments on the patch:

https://sashiko.dev/#/patchset/20260317094653.2236624-1-werner%40verivus.com

On Tue, Mar 17, 2026 at 6:10 AM ChenXiaoSong
<chenxiaosong@xxxxxxxxxxxxxxxx> wrote:
>
> Looks good. Feel free to add:
> Reviewed-by: ChenXiaoSong <chenxiaosong@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
> > Suggested-by: ChenXiaoSong<chenxiaosong@xxxxxxxxxx>
> > Signed-off-by: Werner Kasselman<werner@xxxxxxxxxxx>
> > ---
> >   fs/smb/server/smb2pdu.c | 27 ++++++++++++++++++---------
> >   1 file changed, 18 insertions(+), 9 deletions(-)
>


-- 
Thanks,

Steve