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

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

[Werner Kasselman]

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@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 fs/smb/server/oplock.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
index 393a4ae47cc1..4bc7737f7aa8 100644
--- a/fs/smb/server/oplock.c
+++ b/fs/smb/server/oplock.c
@@ -1291,8 +1291,17 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 	set_oplock_level(opinfo, req_op_level, lctx);
 
 out:
-	opinfo_count_inc(fp);
-	opinfo_add(opinfo, fp);
+	/*
+	 * Set o_fp before any publication so that concurrent readers
+	 * (e.g. find_same_lease_key() on the lease list) that
+	 * dereference opinfo->o_fp don't hit a NULL pointer.
+	 *
+	 * Add to lease global list before publishing on the inode
+	 * op list.  add_lease_global_list() can fail on allocation
+	 * and we must not leave a freed opinfo linked in ci->m_op_list
+	 * where concurrent opinfo_get_list() readers could find it.
+	 */
+	opinfo->o_fp = fp;
 
 	if (opinfo->is_lease) {
 		err = add_lease_global_list(opinfo);
@@ -1300,8 +1309,10 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 			goto err_out;
 	}
 
+	opinfo_count_inc(fp);
+	opinfo_add(opinfo, fp);
+
 	rcu_assign_pointer(fp->f_opinfo, opinfo);
-	opinfo->o_fp = fp;
 
 	return 0;
 err_out:
-- 
2.43.0

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

[ChenXiaoSong]

Hi Werner,

Thanks for your patch. It seems the changes below are not included. Do 
you have any follow-up patches that haven't been submitted yet?

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

在 2026/3/17 10:18, Werner Kasselman 写道:
> - 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.

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

[Werner Kasselman]

I sent an earlier version of the patch by mistake. The version with the complete changes (including alloc_lease_table() split and add_lease_global_list() signature change) was committed locally but the email went out before the final amend. I apologise for the confusion.

I will resend the correct patch as v2. The full diff is +45/-27 lines and includes:
 - New alloc_lease_table() helper (extracted from add_lease_global_list)
 - add_lease_global_list() changed to take preallocated lease_table, return type changed from int to void
 - smb_grant_oplock() restructured: set o_fp, preallocate, then publish
 - Error path uses opinfo_put() instead of __free_opinfo()

Kind regards,
Werner

-----Original Message-----
From: ChenXiaoSong <chenxiaosong@chenxiaosong.com> 
Sent: Tuesday, 17 March 2026 4:33 PM
To: Werner Kasselman <werner@verivus.ai>; linux-cifs@vger.kernel.org
Cc: linkinjeon@kernel.org; smfrench@gmail.com; senozhatsky@chromium.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
Subject: Re: [PATCH] ksmbd: fix use-after-free and NULL deref in smb_grant_oplock()

Hi Werner,

Thanks for your patch. It seems the changes below are not included. Do you have any follow-up patches that haven't been submitted yet?

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

在 2026/3/17 10:18, Werner Kasselman 写道:
> - 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.

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

[ChenXiaoSong]

I just saw your v2 patch, and it seems to be the same as v1: 
https://lore.kernel.org/linux-cifs/20260317063456.1696853-1-werner@verivus.com/

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

在 2026/3/17 14:36, Werner Kasselman 写道:
> I sent an earlier version of the patch by mistake. The version with the complete changes (including alloc_lease_table() split and add_lease_global_list() signature change) was committed locally but the email went out before the final amend. I apologise for the confusion.
> 
> I will resend the correct patch as v2. The full diff is +45/-27 lines and includes:
>   - New alloc_lease_table() helper (extracted from add_lease_global_list)
>   - add_lease_global_list() changed to take preallocated lease_table, return type changed from int to void
>   - smb_grant_oplock() restructured: set o_fp, preallocate, then publish
>   - Error path uses opinfo_put() instead of __free_opinfo()

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

[Werner Kasselman]

format-patch grabbed the wrong diff, sorry.  Homer D'oh!

-----Original Message-----
From: ChenXiaoSong <chenxiaosong@chenxiaosong.com> 
Sent: Tuesday, 17 March 2026 4:43 PM
To: Werner Kasselman <werner@verivus.com>; ChenXiaoSong <chenxiaosong@chenxiaosong.com>; linux-cifs@vger.kernel.org
Cc: linkinjeon@kernel.org; smfrench@gmail.com; senozhatsky@chromium.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
Subject: Re: [PATCH] ksmbd: fix use-after-free and NULL deref in smb_grant_oplock()

I just saw your v2 patch, and it seems to be the same as v1: 
https://lore.kernel.org/linux-cifs/20260317063456.1696853-1-werner@verivus.com/

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

在 2026/3/17 14:36, Werner Kasselman 写道:
> I sent an earlier version of the patch by mistake. The version with the complete changes (including alloc_lease_table() split and add_lease_global_list() signature change) was committed locally but the email went out before the final amend. I apologise for the confusion.
> 
> I will resend the correct patch as v2. The full diff is +45/-27 lines and includes:
>   - New alloc_lease_table() helper (extracted from add_lease_global_list)
>   - add_lease_global_list() changed to take preallocated lease_table, return type changed from int to void
>   - smb_grant_oplock() restructured: set o_fp, preallocate, then publish
>   - Error path uses opinfo_put() instead of __free_opinfo()

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

[Werner Kasselman]

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@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 fs/smb/server/oplock.c | 72 ++++++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
index 393a4ae47cc1..9b2bb8764a80 100644
--- a/fs/smb/server/oplock.c
+++ b/fs/smb/server/oplock.c
@@ -82,11 +82,19 @@ static void lease_del_list(struct oplock_info *opinfo)
 	spin_unlock(&lb->lb_lock);
 }
 
-static void lb_add(struct lease_table *lb)
+static struct lease_table *alloc_lease_table(struct oplock_info *opinfo)
 {
-	write_lock(&lease_list_lock);
-	list_add(&lb->l_entry, &lease_table_list);
-	write_unlock(&lease_list_lock);
+	struct lease_table *lb;
+
+	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
+	if (!lb)
+		return NULL;
+
+	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
+	       SMB2_CLIENT_GUID_SIZE);
+	INIT_LIST_HEAD(&lb->lease_list);
+	spin_lock_init(&lb->lb_lock);
+	return lb;
 }
 
 static int alloc_lease(struct oplock_info *opinfo, struct lease_ctx_info *lctx)
@@ -1042,34 +1050,27 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2)
 	lease2->version = lease1->version;
 }
 
-static int add_lease_global_list(struct oplock_info *opinfo)
+static void add_lease_global_list(struct oplock_info *opinfo,
+				  struct lease_table *new_lb)
 {
 	struct lease_table *lb;
 
-	read_lock(&lease_list_lock);
+	write_lock(&lease_list_lock);
 	list_for_each_entry(lb, &lease_table_list, l_entry) {
 		if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,
 			    SMB2_CLIENT_GUID_SIZE)) {
 			opinfo->o_lease->l_lb = lb;
 			lease_add_list(opinfo);
-			read_unlock(&lease_list_lock);
-			return 0;
+			write_unlock(&lease_list_lock);
+			kfree(new_lb);
+			return;
 		}
 	}
-	read_unlock(&lease_list_lock);
 
-	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
-	if (!lb)
-		return -ENOMEM;
-
-	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
-	       SMB2_CLIENT_GUID_SIZE);
-	INIT_LIST_HEAD(&lb->lease_list);
-	spin_lock_init(&lb->lb_lock);
-	opinfo->o_lease->l_lb = lb;
+	opinfo->o_lease->l_lb = new_lb;
 	lease_add_list(opinfo);
-	lb_add(lb);
-	return 0;
+	list_add(&new_lb->l_entry, &lease_table_list);
+	write_unlock(&lease_list_lock);
 }
 
 static void set_oplock_level(struct oplock_info *opinfo, int level,
@@ -1189,6 +1190,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 	int err = 0;
 	struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
 	struct ksmbd_inode *ci = fp->f_ci;
+	struct lease_table *new_lb = NULL;
 	bool prev_op_has_lease;
 	__le32 prev_op_state = 0;
 
@@ -1291,21 +1293,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 	set_oplock_level(opinfo, req_op_level, lctx);
 
 out:
-	opinfo_count_inc(fp);
-	opinfo_add(opinfo, fp);
-
+	/*
+	 * Set o_fp before any publication so that concurrent readers
+	 * (e.g. find_same_lease_key() on the lease list) that
+	 * dereference opinfo->o_fp don't hit a NULL pointer.
+	 *
+	 * Keep the original publication order so concurrent opens can
+	 * still observe the in-flight grant via ci->m_op_list, but make
+	 * everything after opinfo_add() no-fail by preallocating any new
+	 * lease_table first.
+	 */
+	opinfo->o_fp = fp;
 	if (opinfo->is_lease) {
-		err = add_lease_global_list(opinfo);
-		if (err)
+		new_lb = alloc_lease_table(opinfo);
+		if (!new_lb) {
+			err = -ENOMEM;
 			goto err_out;
+		}
 	}
 
+	opinfo_count_inc(fp);
+	opinfo_add(opinfo, fp);
+
+	if (opinfo->is_lease)
+		add_lease_global_list(opinfo, new_lb);
+
 	rcu_assign_pointer(fp->f_opinfo, opinfo);
-	opinfo->o_fp = fp;
 
 	return 0;
 err_out:
-	__free_opinfo(opinfo);
+	kfree(new_lb);
+	opinfo_put(opinfo);
 	return err;
 }
 
-- 
2.43.0

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

[Werner Kasselman]

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@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 fs/smb/server/oplock.c | 72 ++++++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
index 393a4ae47cc1..9b2bb8764a80 100644
--- a/fs/smb/server/oplock.c
+++ b/fs/smb/server/oplock.c
@@ -82,11 +82,19 @@ static void lease_del_list(struct oplock_info *opinfo)
 	spin_unlock(&lb->lb_lock);
 }
 
-static void lb_add(struct lease_table *lb)
+static struct lease_table *alloc_lease_table(struct oplock_info *opinfo)
 {
-	write_lock(&lease_list_lock);
-	list_add(&lb->l_entry, &lease_table_list);
-	write_unlock(&lease_list_lock);
+	struct lease_table *lb;
+
+	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
+	if (!lb)
+		return NULL;
+
+	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
+	       SMB2_CLIENT_GUID_SIZE);
+	INIT_LIST_HEAD(&lb->lease_list);
+	spin_lock_init(&lb->lb_lock);
+	return lb;
 }
 
 static int alloc_lease(struct oplock_info *opinfo, struct lease_ctx_info *lctx)
@@ -1042,34 +1050,27 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2)
 	lease2->version = lease1->version;
 }
 
-static int add_lease_global_list(struct oplock_info *opinfo)
+static void add_lease_global_list(struct oplock_info *opinfo,
+				  struct lease_table *new_lb)
 {
 	struct lease_table *lb;
 
-	read_lock(&lease_list_lock);
+	write_lock(&lease_list_lock);
 	list_for_each_entry(lb, &lease_table_list, l_entry) {
 		if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,
 			    SMB2_CLIENT_GUID_SIZE)) {
 			opinfo->o_lease->l_lb = lb;
 			lease_add_list(opinfo);
-			read_unlock(&lease_list_lock);
-			return 0;
+			write_unlock(&lease_list_lock);
+			kfree(new_lb);
+			return;
 		}
 	}
-	read_unlock(&lease_list_lock);
 
-	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
-	if (!lb)
-		return -ENOMEM;
-
-	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
-	       SMB2_CLIENT_GUID_SIZE);
-	INIT_LIST_HEAD(&lb->lease_list);
-	spin_lock_init(&lb->lb_lock);
-	opinfo->o_lease->l_lb = lb;
+	opinfo->o_lease->l_lb = new_lb;
 	lease_add_list(opinfo);
-	lb_add(lb);
-	return 0;
+	list_add(&new_lb->l_entry, &lease_table_list);
+	write_unlock(&lease_list_lock);
 }
 
 static void set_oplock_level(struct oplock_info *opinfo, int level,
@@ -1189,6 +1190,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 	int err = 0;
 	struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
 	struct ksmbd_inode *ci = fp->f_ci;
+	struct lease_table *new_lb = NULL;
 	bool prev_op_has_lease;
 	__le32 prev_op_state = 0;
 
@@ -1291,21 +1293,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 	set_oplock_level(opinfo, req_op_level, lctx);
 
 out:
-	opinfo_count_inc(fp);
-	opinfo_add(opinfo, fp);
-
+	/*
+	 * Set o_fp before any publication so that concurrent readers
+	 * (e.g. find_same_lease_key() on the lease list) that
+	 * dereference opinfo->o_fp don't hit a NULL pointer.
+	 *
+	 * Keep the original publication order so concurrent opens can
+	 * still observe the in-flight grant via ci->m_op_list, but make
+	 * everything after opinfo_add() no-fail by preallocating any new
+	 * lease_table first.
+	 */
+	opinfo->o_fp = fp;
 	if (opinfo->is_lease) {
-		err = add_lease_global_list(opinfo);
-		if (err)
+		new_lb = alloc_lease_table(opinfo);
+		if (!new_lb) {
+			err = -ENOMEM;
 			goto err_out;
+		}
 	}
 
+	opinfo_count_inc(fp);
+	opinfo_add(opinfo, fp);
+
+	if (opinfo->is_lease)
+		add_lease_global_list(opinfo, new_lb);
+
 	rcu_assign_pointer(fp->f_opinfo, opinfo);
-	opinfo->o_fp = fp;
 
 	return 0;
 err_out:
-	__free_opinfo(opinfo);
+	kfree(new_lb);
+	opinfo_put(opinfo);
 	return err;
 }
 
-- 
2.43.0

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

[Greg KH]

On Tue, Mar 17, 2026 at 01:05:56PM +0000, 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@vger.kernel.org
> Signed-off-by: Werner Kasselman <werner@verivus.com>
> ---
>  fs/smb/server/oplock.c | 72 ++++++++++++++++++++++++++----------------
>  1 file changed, 45 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
> index 393a4ae47cc1..9b2bb8764a80 100644
> --- a/fs/smb/server/oplock.c
> +++ b/fs/smb/server/oplock.c
> @@ -82,11 +82,19 @@ static void lease_del_list(struct oplock_info *opinfo)
>  	spin_unlock(&lb->lb_lock);
>  }
>  
> -static void lb_add(struct lease_table *lb)
> +static struct lease_table *alloc_lease_table(struct oplock_info *opinfo)
>  {
> -	write_lock(&lease_list_lock);
> -	list_add(&lb->l_entry, &lease_table_list);
> -	write_unlock(&lease_list_lock);
> +	struct lease_table *lb;
> +
> +	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
> +	if (!lb)
> +		return NULL;
> +
> +	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
> +	       SMB2_CLIENT_GUID_SIZE);
> +	INIT_LIST_HEAD(&lb->lease_list);
> +	spin_lock_init(&lb->lb_lock);
> +	return lb;
>  }
>  
>  static int alloc_lease(struct oplock_info *opinfo, struct lease_ctx_info *lctx)
> @@ -1042,34 +1050,27 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2)
>  	lease2->version = lease1->version;
>  }
>  
> -static int add_lease_global_list(struct oplock_info *opinfo)
> +static void add_lease_global_list(struct oplock_info *opinfo,
> +				  struct lease_table *new_lb)
>  {
>  	struct lease_table *lb;
>  
> -	read_lock(&lease_list_lock);
> +	write_lock(&lease_list_lock);
>  	list_for_each_entry(lb, &lease_table_list, l_entry) {
>  		if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,
>  			    SMB2_CLIENT_GUID_SIZE)) {
>  			opinfo->o_lease->l_lb = lb;
>  			lease_add_list(opinfo);
> -			read_unlock(&lease_list_lock);
> -			return 0;
> +			write_unlock(&lease_list_lock);
> +			kfree(new_lb);
> +			return;
>  		}
>  	}
> -	read_unlock(&lease_list_lock);
>  
> -	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
> -	if (!lb)
> -		return -ENOMEM;
> -
> -	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
> -	       SMB2_CLIENT_GUID_SIZE);
> -	INIT_LIST_HEAD(&lb->lease_list);
> -	spin_lock_init(&lb->lb_lock);
> -	opinfo->o_lease->l_lb = lb;
> +	opinfo->o_lease->l_lb = new_lb;
>  	lease_add_list(opinfo);
> -	lb_add(lb);
> -	return 0;
> +	list_add(&new_lb->l_entry, &lease_table_list);
> +	write_unlock(&lease_list_lock);
>  }
>  
>  static void set_oplock_level(struct oplock_info *opinfo, int level,
> @@ -1189,6 +1190,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>  	int err = 0;
>  	struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
>  	struct ksmbd_inode *ci = fp->f_ci;
> +	struct lease_table *new_lb = NULL;
>  	bool prev_op_has_lease;
>  	__le32 prev_op_state = 0;
>  
> @@ -1291,21 +1293,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>  	set_oplock_level(opinfo, req_op_level, lctx);
>  
>  out:
> -	opinfo_count_inc(fp);
> -	opinfo_add(opinfo, fp);
> -
> +	/*
> +	 * Set o_fp before any publication so that concurrent readers
> +	 * (e.g. find_same_lease_key() on the lease list) that
> +	 * dereference opinfo->o_fp don't hit a NULL pointer.
> +	 *
> +	 * Keep the original publication order so concurrent opens can
> +	 * still observe the in-flight grant via ci->m_op_list, but make
> +	 * everything after opinfo_add() no-fail by preallocating any new
> +	 * lease_table first.
> +	 */
> +	opinfo->o_fp = fp;
>  	if (opinfo->is_lease) {
> -		err = add_lease_global_list(opinfo);
> -		if (err)
> +		new_lb = alloc_lease_table(opinfo);
> +		if (!new_lb) {
> +			err = -ENOMEM;
>  			goto err_out;
> +		}
>  	}
>  
> +	opinfo_count_inc(fp);
> +	opinfo_add(opinfo, fp);
> +
> +	if (opinfo->is_lease)
> +		add_lease_global_list(opinfo, new_lb);
> +
>  	rcu_assign_pointer(fp->f_opinfo, opinfo);
> -	opinfo->o_fp = fp;
>  
>  	return 0;
>  err_out:
> -	__free_opinfo(opinfo);
> +	kfree(new_lb);
> +	opinfo_put(opinfo);
>  	return err;
>  }
>  
> -- 
> 2.43.0
> 
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

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

[ChenXiaoSong]

This patch seems to be identical to v3. Why did you resend it?

v3: 
https://lore.kernel.org/linux-cifs/435dda9f-93f5-41db-9d21-70371d31857b@chenxiaosong.com/T/#t

Thanks,
ChenXiaoSong <chenxiaosong@kylinos.cn>

On 3/17/26 21:00, 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@vger.kernel.org
> Signed-off-by: Werner Kasselman <werner@verivus.com>
> ---
>   fs/smb/server/oplock.c | 72 ++++++++++++++++++++++++++----------------
>   1 file changed, 45 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
> index 393a4ae47cc1..9b2bb8764a80 100644
> --- a/fs/smb/server/oplock.c
> +++ b/fs/smb/server/oplock.c
> @@ -82,11 +82,19 @@ static void lease_del_list(struct oplock_info *opinfo)
>   	spin_unlock(&lb->lb_lock);
>   }
>   
> -static void lb_add(struct lease_table *lb)
> +static struct lease_table *alloc_lease_table(struct oplock_info *opinfo)
>   {
> -	write_lock(&lease_list_lock);
> -	list_add(&lb->l_entry, &lease_table_list);
> -	write_unlock(&lease_list_lock);
> +	struct lease_table *lb;
> +
> +	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
> +	if (!lb)
> +		return NULL;
> +
> +	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
> +	       SMB2_CLIENT_GUID_SIZE);
> +	INIT_LIST_HEAD(&lb->lease_list);
> +	spin_lock_init(&lb->lb_lock);
> +	return lb;
>   }
>   
>   static int alloc_lease(struct oplock_info *opinfo, struct lease_ctx_info *lctx)
> @@ -1042,34 +1050,27 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2)
>   	lease2->version = lease1->version;
>   }
>   
> -static int add_lease_global_list(struct oplock_info *opinfo)
> +static void add_lease_global_list(struct oplock_info *opinfo,
> +				  struct lease_table *new_lb)
>   {
>   	struct lease_table *lb;
>   
> -	read_lock(&lease_list_lock);
> +	write_lock(&lease_list_lock);
>   	list_for_each_entry(lb, &lease_table_list, l_entry) {
>   		if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,
>   			    SMB2_CLIENT_GUID_SIZE)) {
>   			opinfo->o_lease->l_lb = lb;
>   			lease_add_list(opinfo);
> -			read_unlock(&lease_list_lock);
> -			return 0;
> +			write_unlock(&lease_list_lock);
> +			kfree(new_lb);
> +			return;
>   		}
>   	}
> -	read_unlock(&lease_list_lock);
>   
> -	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
> -	if (!lb)
> -		return -ENOMEM;
> -
> -	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
> -	       SMB2_CLIENT_GUID_SIZE);
> -	INIT_LIST_HEAD(&lb->lease_list);
> -	spin_lock_init(&lb->lb_lock);
> -	opinfo->o_lease->l_lb = lb;
> +	opinfo->o_lease->l_lb = new_lb;
>   	lease_add_list(opinfo);
> -	lb_add(lb);
> -	return 0;
> +	list_add(&new_lb->l_entry, &lease_table_list);
> +	write_unlock(&lease_list_lock);
>   }
>   
>   static void set_oplock_level(struct oplock_info *opinfo, int level,
> @@ -1189,6 +1190,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>   	int err = 0;
>   	struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
>   	struct ksmbd_inode *ci = fp->f_ci;
> +	struct lease_table *new_lb = NULL;
>   	bool prev_op_has_lease;
>   	__le32 prev_op_state = 0;
>   
> @@ -1291,21 +1293,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>   	set_oplock_level(opinfo, req_op_level, lctx);
>   
>   out:
> -	opinfo_count_inc(fp);
> -	opinfo_add(opinfo, fp);
> -
> +	/*
> +	 * Set o_fp before any publication so that concurrent readers
> +	 * (e.g. find_same_lease_key() on the lease list) that
> +	 * dereference opinfo->o_fp don't hit a NULL pointer.
> +	 *
> +	 * Keep the original publication order so concurrent opens can
> +	 * still observe the in-flight grant via ci->m_op_list, but make
> +	 * everything after opinfo_add() no-fail by preallocating any new
> +	 * lease_table first.
> +	 */
> +	opinfo->o_fp = fp;
>   	if (opinfo->is_lease) {
> -		err = add_lease_global_list(opinfo);
> -		if (err)
> +		new_lb = alloc_lease_table(opinfo);
> +		if (!new_lb) {
> +			err = -ENOMEM;
>   			goto err_out;
> +		}
>   	}
>   
> +	opinfo_count_inc(fp);
> +	opinfo_add(opinfo, fp);
> +
> +	if (opinfo->is_lease)
> +		add_lease_global_list(opinfo, new_lb);
> +
>   	rcu_assign_pointer(fp->f_opinfo, opinfo);
> -	opinfo->o_fp = fp;
>   
>   	return 0;
>   err_out:
> -	__free_opinfo(opinfo);
> +	kfree(new_lb);
> +	opinfo_put(opinfo);
>   	return err;
>   }
>   

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

[Werner Kasselman]

Namjae had issues with rendering what I sent.  I resent it to try to fix that.  I used Graph API to send the email from git and apparently Namjae couldn't read it.

-----Original Message-----
From: ChenXiaoSong <chenxiaosong@chenxiaosong.com> 
Sent: Tuesday, 17 March 2026 11:48 PM
To: Werner Kasselman <werner@verivus.ai>
Cc: sfrench@samba.org; linux-cifs@vger.kernel.org; stable@vger.kernel.org; linkinjeon@kernel.org
Subject: Re: [PATCH] ksmbd: fix use-after-free and NULL deref in smb_grant_oplock()

This patch seems to be identical to v3. Why did you resend it?

v3: 
https://lore.kernel.org/linux-cifs/435dda9f-93f5-41db-9d21-70371d31857b@chenxiaosong.com/T/#t

Thanks,
ChenXiaoSong <chenxiaosong@kylinos.cn>

On 3/17/26 21:00, 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@vger.kernel.org
> Signed-off-by: Werner Kasselman <werner@verivus.com>
> ---
>   fs/smb/server/oplock.c | 72 ++++++++++++++++++++++++++----------------
>   1 file changed, 45 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c index 
> 393a4ae47cc1..9b2bb8764a80 100644
> --- a/fs/smb/server/oplock.c
> +++ b/fs/smb/server/oplock.c
> @@ -82,11 +82,19 @@ static void lease_del_list(struct oplock_info *opinfo)
>   	spin_unlock(&lb->lb_lock);
>   }
>   
> -static void lb_add(struct lease_table *lb)
> +static struct lease_table *alloc_lease_table(struct oplock_info 
> +*opinfo)
>   {
> -	write_lock(&lease_list_lock);
> -	list_add(&lb->l_entry, &lease_table_list);
> -	write_unlock(&lease_list_lock);
> +	struct lease_table *lb;
> +
> +	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
> +	if (!lb)
> +		return NULL;
> +
> +	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
> +	       SMB2_CLIENT_GUID_SIZE);
> +	INIT_LIST_HEAD(&lb->lease_list);
> +	spin_lock_init(&lb->lb_lock);
> +	return lb;
>   }
>   
>   static int alloc_lease(struct oplock_info *opinfo, struct 
> lease_ctx_info *lctx) @@ -1042,34 +1050,27 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2)
>   	lease2->version = lease1->version;
>   }
>   
> -static int add_lease_global_list(struct oplock_info *opinfo)
> +static void add_lease_global_list(struct oplock_info *opinfo,
> +				  struct lease_table *new_lb)
>   {
>   	struct lease_table *lb;
>   
> -	read_lock(&lease_list_lock);
> +	write_lock(&lease_list_lock);
>   	list_for_each_entry(lb, &lease_table_list, l_entry) {
>   		if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,
>   			    SMB2_CLIENT_GUID_SIZE)) {
>   			opinfo->o_lease->l_lb = lb;
>   			lease_add_list(opinfo);
> -			read_unlock(&lease_list_lock);
> -			return 0;
> +			write_unlock(&lease_list_lock);
> +			kfree(new_lb);
> +			return;
>   		}
>   	}
> -	read_unlock(&lease_list_lock);
>   
> -	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
> -	if (!lb)
> -		return -ENOMEM;
> -
> -	memcpy(lb->client_guid, opinfo->conn->ClientGUID,
> -	       SMB2_CLIENT_GUID_SIZE);
> -	INIT_LIST_HEAD(&lb->lease_list);
> -	spin_lock_init(&lb->lb_lock);
> -	opinfo->o_lease->l_lb = lb;
> +	opinfo->o_lease->l_lb = new_lb;
>   	lease_add_list(opinfo);
> -	lb_add(lb);
> -	return 0;
> +	list_add(&new_lb->l_entry, &lease_table_list);
> +	write_unlock(&lease_list_lock);
>   }
>   
>   static void set_oplock_level(struct oplock_info *opinfo, int level, 
> @@ -1189,6 +1190,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>   	int err = 0;
>   	struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
>   	struct ksmbd_inode *ci = fp->f_ci;
> +	struct lease_table *new_lb = NULL;
>   	bool prev_op_has_lease;
>   	__le32 prev_op_state = 0;
>   
> @@ -1291,21 +1293,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>   	set_oplock_level(opinfo, req_op_level, lctx);
>   
>   out:
> -	opinfo_count_inc(fp);
> -	opinfo_add(opinfo, fp);
> -
> +	/*
> +	 * Set o_fp before any publication so that concurrent readers
> +	 * (e.g. find_same_lease_key() on the lease list) that
> +	 * dereference opinfo->o_fp don't hit a NULL pointer.
> +	 *
> +	 * Keep the original publication order so concurrent opens can
> +	 * still observe the in-flight grant via ci->m_op_list, but make
> +	 * everything after opinfo_add() no-fail by preallocating any new
> +	 * lease_table first.
> +	 */
> +	opinfo->o_fp = fp;
>   	if (opinfo->is_lease) {
> -		err = add_lease_global_list(opinfo);
> -		if (err)
> +		new_lb = alloc_lease_table(opinfo);
> +		if (!new_lb) {
> +			err = -ENOMEM;
>   			goto err_out;
> +		}
>   	}
>   
> +	opinfo_count_inc(fp);
> +	opinfo_add(opinfo, fp);
> +
> +	if (opinfo->is_lease)
> +		add_lease_global_list(opinfo, new_lb);
> +
>   	rcu_assign_pointer(fp->f_opinfo, opinfo);
> -	opinfo->o_fp = fp;
>   
>   	return 0;
>   err_out:
> -	__free_opinfo(opinfo);
> +	kfree(new_lb);
> +	opinfo_put(opinfo);
>   	return err;
>   }
>