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

Werner Kasselman
2026-03-17T09:49:16Z

Thanks ChenXiaoSong,

Both suggestions incorporated in v2:

1. Hoisted locks_free_lock()/kfree() before the if(!rc) check in the UNLOCK branch, all exit paths now share one free site instead of duplicating it in the -ENOENT branch.
2. Replaced the duplicated rollback cleanup with an if(rlock) guard around the VFS unlock and a matching NULL check on locks_free_lock(rlock) in the shared cleanup tail. This eliminates the code duplication we had in v1 where the !rlock path copied the entire teardown sequence.

v2 sent.

Kind regards,
Werner

-----Original Message-----
From: ChenXiaoSong <chenxiaosong@chenxiaosong.com>
Sent: Tuesday, 17 March 2026 7:30 PM
To: Werner Kasselman <werner@verivus.ai>; Namjae Jeon <linkinjeon@kernel.org>; Steve French <smfrench@gmail.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>; Tom Talpey <tom@talpey.com>; linux-cifs@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
Subject: Re: [PATCH] ksmbd: fix memory leaks and NULL deref in smb2_lock()

And it might be better to change it as follows.

```
@@ -7685,13 +7686,17 @@ int smb2_lock(struct ksmbd_work *work)
struct file_lock *rlock = NULL;

rlock = smb_flock_init(filp);
- rlock->c.flc_type = F_UNLCK;
- rlock->fl_start = smb_lock->start;
- rlock->fl_end = smb_lock->end;
+ if (rlock) {
+ rlock->c.flc_type = F_UNLCK;
+ rlock->fl_start = smb_lock->start;
+ rlock->fl_end = smb_lock->end;

- rc = vfs_lock_file(filp, F_SETLK, rlock, NULL);
- if (rc)
- pr_err("rollback unlock fail : %d\n", rc);
+ rc = vfs_lock_file(filp, F_SETLK, rlock, NULL);
+ if (rc)
+ pr_err("rollback unlock fail : %d\n", rc);
+ } else {
+ pr_err("rollback unlock alloc failed\n");
+ }

list_del(&smb_lock->llist);
spin_lock(&work->conn->llist_lock);
@@ -7701,7 +7706,8 @@ int smb2_lock(struct ksmbd_work *work)
spin_unlock(&work->conn->llist_lock);

locks_free_lock(smb_lock->fl);
- locks_free_lock(rlock);
+ if (rlock)
+ locks_free_lock(rlock);
kfree(smb_lock);
}
out2:
```

Thanks,
ChenXiaoSong <chenxiaosong@kylinos.cn>

On 3/17/26 16:08, Werner Kasselman wrote:
> @@ -7685,6 +7691,19 @@ int smb2_lock(struct ksmbd_work *work)
> struct file_lock *rlock = NULL;
>
> rlock = smb_flock_init(filp);
> + if (!rlock) {
> + pr_err("rollback unlock alloc failed\n");
> + list_del(&smb_lock->llist);
> + spin_lock(&work->conn->llist_lock);
> + if (!list_empty(&smb_lock->flist))
> + list_del(&smb_lock->flist);
> + list_del(&smb_lock->clist);
> + spin_unlock(&work->conn->llist_lock);
> +
> + locks_free_lock(smb_lock->fl);
> + kfree(smb_lock);
> + continue;
> + }
> rlock->c.flc_type = F_UNLCK;
> rlock->fl_start = smb_lock->start;
> rlock->fl_end = smb_lock->end;