[PATCH 7.1.y] ksmbd: fix use-after-free in oplock break notification

[PATCH 7.1.y] ksmbd: fix use-after-free in oplock break notification

[Abdifatah Suruur]

smb2_oplock_break_noti() reads opinfo->conn without any lock and
dereferences it after two allocations which may sleep.  When the
durable handle owning the oplock is disconnected, session_fd_check()
clears opinfo->conn and drops its conn reference under ci->m_lock, and
the last ksmbd_conn_put() frees the connection.  A break triggered by
another connection that races with the teardown can then resurrect the
freed connection: ksmbd_conn_get() is a plain atomic_inc, and the
queued break work later dereferences the stale conn via
ksmbd_conn_write(), a use-after-free reachable by any authenticated
client holding a durable batch oplock.

Thread the caller's inode into the notification path instead of taking
a new reference on it.  Every caller of oplock_break() already holds a
live ksmbd_file (or an explicit ksmbd_inode_lookup_lock() reference,
in the parent lease break paths) on the inode that owns the break
target's oplock list, so ci cannot be freed during the call, and its
lock can be taken without dereferencing opinfo->o_fp, which a
concurrent close may free.  Select and pin the connection under
ci->m_lock, the same lock session_fd_check() and
ksmbd_reopen_durable_fd() use to update opinfo->conn, so a concurrent
detach either loses the race to the clear or keeps the connection
alive until the notification work releases it.  Transfer the reference
to the work item and release it on allocation failures.

[ This tree predates the deferred break machinery
  (oplock_break_add()/oplock_break_drain_none()) that the original
  commit is layered on; oplock_break() here calls the notifier
  directly and the caller-pinning argument is unchanged. ]

Fixes: b003086d7696 ("ksmbd: fix NULL-deref of opinfo->conn in oplock/lease break notifiers")
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
commit 0e753899627b5e28a9fea8bca98262a6f65a2452 upstream.
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
---
 fs/smb/server/oplock.c | 61 +++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 12 deletions(-)

diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
index 3f28dc3b7fc41..b8bc0d16e042c 100644
--- a/fs/smb/server/oplock.c
+++ b/fs/smb/server/oplock.c
@@ -708,31 +708,67 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)
 	ksmbd_conn_put(conn);
 }
 
+/*
+ * Select and pin the connection used for an oplock break before doing any
+ * allocations which may sleep.  The caller of oplock_break() holds a live
+ * reference on ci (a file being opened, a file being operated on, or an
+ * explicit ksmbd_inode_lookup_lock() reference in the parent lease break
+ * paths), so the inode cannot be freed during the call.
+ *
+ * opinfo->conn is cleared under ci->m_lock by session_fd_check() when the
+ * durable handle owning the oplock is disconnected, reassigned by
+ * ksmbd_reopen_durable_fd() under the same lock, and the last
+ * ksmbd_conn_put() of the old connection frees it.  Holding the read lock
+ * excludes both writers, so the connection cannot be freed while it is
+ * selected.
+ */
+static struct ksmbd_conn *smb2_oplock_break_conn_get(struct oplock_info *opinfo,
+						     struct ksmbd_inode *ci)
+{
+	struct ksmbd_conn *conn;
+
+	down_read(&ci->m_lock);
+	conn = READ_ONCE(opinfo->conn);
+	if (conn && !ksmbd_conn_releasing(conn))
+		conn = ksmbd_conn_get(conn);
+	else
+		conn = NULL;
+	up_read(&ci->m_lock);
+
+	return conn;
+}
+
 /**
  * smb2_oplock_break_noti() - send smb2 exclusive/batch to level2 oplock
  *		break command from server to client
  * @opinfo:		oplock info object
+ * @ci:		inode owning the break target's oplock list, pinned by
+ *		the caller
  *
  * Return:      0 on success, otherwise error
  */
-static int smb2_oplock_break_noti(struct oplock_info *opinfo)
+static int smb2_oplock_break_noti(struct oplock_info *opinfo,
+				  struct ksmbd_inode *ci)
 {
 	struct ksmbd_conn *conn;
 	struct oplock_break_info *br_info;
 	int ret = 0;
 	struct ksmbd_work *work;
 
-	conn = READ_ONCE(opinfo->conn);
+	conn = smb2_oplock_break_conn_get(opinfo, ci);
 	if (!conn)
 		return 0;
 
 	work = ksmbd_alloc_work_struct();
-	if (!work)
+	if (!work) {
+		ksmbd_conn_put(conn);
 		return -ENOMEM;
+	}
 
 	br_info = kmalloc_obj(struct oplock_break_info, KSMBD_DEFAULT_GFP);
 	if (!br_info) {
 		ksmbd_free_work_struct(work);
+		ksmbd_conn_put(conn);
 		return -ENOMEM;
 	}
 
@@ -741,7 +777,8 @@ static int smb2_oplock_break_noti(struct oplock_info *opinfo)
 	br_info->open_trunc = opinfo->open_trunc;
 
 	work->request_buf = (char *)br_info;
-	work->conn = ksmbd_conn_get(conn);
+	/* Transfer the reference acquired by smb2_oplock_break_conn_get(). */
+	work->conn = conn;
 	work->sess = opinfo->sess;
 
 	ksmbd_conn_r_count_inc(conn);
@@ -890,8 +927,8 @@ static void wait_lease_breaking(struct oplock_info *opinfo)
 	}
 }
 
-static int oplock_break(struct oplock_info *brk_opinfo, int req_op_level,
-			struct ksmbd_work *in_work)
+static int oplock_break(struct oplock_info *brk_opinfo, struct ksmbd_inode *ci,
+			int req_op_level, struct ksmbd_work *in_work)
 {
 	int err = 0;
 
@@ -957,7 +994,7 @@ static int oplock_break(struct oplock_info *brk_opinfo, int req_op_level,
 	if (brk_opinfo->is_lease)
 		err = smb2_lease_break_noti(brk_opinfo);
 	else
-		err = smb2_oplock_break_noti(brk_opinfo);
+		err = smb2_oplock_break_noti(brk_opinfo, ci);
 
 	ksmbd_debug(OPLOCK, "oplock granted = %d\n", brk_opinfo->level);
 	if (brk_opinfo->op_state == OPLOCK_CLOSING)
@@ -1137,7 +1174,7 @@ void smb_send_parent_lease_break_noti(struct ksmbd_file *fp,
 				continue;
 			}
 
-			oplock_break(opinfo, SMB2_OPLOCK_LEVEL_NONE, NULL);
+			oplock_break(opinfo, p_ci, SMB2_OPLOCK_LEVEL_NONE, NULL);
 			opinfo_put(opinfo);
 		}
 	}
@@ -1178,7 +1215,7 @@ void smb_lazy_parent_lease_break_close(struct ksmbd_file *fp)
 				continue;
 			}
 
-			oplock_break(opinfo, SMB2_OPLOCK_LEVEL_NONE, NULL);
+			oplock_break(opinfo, p_ci, SMB2_OPLOCK_LEVEL_NONE, NULL);
 			opinfo_put(opinfo);
 		}
 	}
@@ -1280,7 +1317,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 		goto op_break_not_needed;
 	}
 
-	err = oplock_break(prev_opinfo, SMB2_OPLOCK_LEVEL_II, work);
+	err = oplock_break(prev_opinfo, fp->f_ci, SMB2_OPLOCK_LEVEL_II, work);
 	opinfo_put(prev_opinfo);
 	if (err == -ENOENT)
 		goto set_lev;
@@ -1366,7 +1403,7 @@ static void smb_break_all_write_oplock(struct ksmbd_work *work,
 	}
 
 	brk_opinfo->open_trunc = is_trunc;
-	oplock_break(brk_opinfo, SMB2_OPLOCK_LEVEL_II, work);
+	oplock_break(brk_opinfo, fp->f_ci, SMB2_OPLOCK_LEVEL_II, work);
 	opinfo_put(brk_opinfo);
 }
 
@@ -1430,7 +1467,7 @@ void smb_break_all_levII_oplock(struct ksmbd_work *work, struct ksmbd_file *fp,
 			    SMB2_LEASE_KEY_SIZE))
 			goto next;
 		brk_op->open_trunc = is_trunc;
-		oplock_break(brk_op, SMB2_OPLOCK_LEVEL_NONE, NULL);
+		oplock_break(brk_op, ci, SMB2_OPLOCK_LEVEL_NONE, NULL);
 next:
 		opinfo_put(brk_op);
 	}
-- 
2.53.0

Re: [PATCH 7.1.y] ksmbd: fix use-after-free in oplock break notification

[Greg KH]

On Wed, Sep 09, 2026 at 10:58:35AM +0300, Abdifatah Suruur wrote:
> smb2_oplock_break_noti() reads opinfo->conn without any lock and
> dereferences it after two allocations which may sleep.  When the
> durable handle owning the oplock is disconnected, session_fd_check()
> clears opinfo->conn and drops its conn reference under ci->m_lock, and
> the last ksmbd_conn_put() frees the connection.  A break triggered by
> another connection that races with the teardown can then resurrect the
> freed connection: ksmbd_conn_get() is a plain atomic_inc, and the
> queued break work later dereferences the stale conn via
> ksmbd_conn_write(), a use-after-free reachable by any authenticated
> client holding a durable batch oplock.
> 
> Thread the caller's inode into the notification path instead of taking
> a new reference on it.  Every caller of oplock_break() already holds a
> live ksmbd_file (or an explicit ksmbd_inode_lookup_lock() reference,
> in the parent lease break paths) on the inode that owns the break
> target's oplock list, so ci cannot be freed during the call, and its
> lock can be taken without dereferencing opinfo->o_fp, which a
> concurrent close may free.  Select and pin the connection under
> ci->m_lock, the same lock session_fd_check() and
> ksmbd_reopen_durable_fd() use to update opinfo->conn, so a concurrent
> detach either loses the race to the clear or keeps the connection
> alive until the notification work releases it.  Transfer the reference
> to the work item and release it on allocation failures.
> 
> [ This tree predates the deferred break machinery
>   (oplock_break_add()/oplock_break_drain_none()) that the original
>   commit is layered on; oplock_break() here calls the notifier
>   directly and the caller-pinning argument is unchanged. ]
> 
> Fixes: b003086d7696 ("ksmbd: fix NULL-deref of opinfo->conn in oplock/lease break notifiers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> commit 0e753899627b5e28a9fea8bca98262a6f65a2452 upstream.
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> ---
>  fs/smb/server/oplock.c | 61 +++++++++++++++++++++++++++++++++---------
>  1 file changed, 49 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
> index 3f28dc3b7fc41..b8bc0d16e042c 100644
> --- a/fs/smb/server/oplock.c
> +++ b/fs/smb/server/oplock.c
> @@ -708,31 +708,67 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)
>  	ksmbd_conn_put(conn);
>  }
>  
> +/*
> + * Select and pin the connection used for an oplock break before doing any
> + * allocations which may sleep.  The caller of oplock_break() holds a live
> + * reference on ci (a file being opened, a file being operated on, or an
> + * explicit ksmbd_inode_lookup_lock() reference in the parent lease break
> + * paths), so the inode cannot be freed during the call.
> + *
> + * opinfo->conn is cleared under ci->m_lock by session_fd_check() when the
> + * durable handle owning the oplock is disconnected, reassigned by
> + * ksmbd_reopen_durable_fd() under the same lock, and the last
> + * ksmbd_conn_put() of the old connection frees it.  Holding the read lock
> + * excludes both writers, so the connection cannot be freed while it is
> + * selected.
> + */
> +static struct ksmbd_conn *smb2_oplock_break_conn_get(struct oplock_info *opinfo,
> +						     struct ksmbd_inode *ci)
> +{
> +	struct ksmbd_conn *conn;
> +
> +	down_read(&ci->m_lock);
> +	conn = READ_ONCE(opinfo->conn);
> +	if (conn && !ksmbd_conn_releasing(conn))
> +		conn = ksmbd_conn_get(conn);
> +	else
> +		conn = NULL;
> +	up_read(&ci->m_lock);
> +
> +	return conn;
> +}
> +
>  /**
>   * smb2_oplock_break_noti() - send smb2 exclusive/batch to level2 oplock
>   *		break command from server to client
>   * @opinfo:		oplock info object
> + * @ci:		inode owning the break target's oplock list, pinned by
> + *		the caller
>   *
>   * Return:      0 on success, otherwise error
>   */
> -static int smb2_oplock_break_noti(struct oplock_info *opinfo)
> +static int smb2_oplock_break_noti(struct oplock_info *opinfo,
> +				  struct ksmbd_inode *ci)
>  {
>  	struct ksmbd_conn *conn;
>  	struct oplock_break_info *br_info;
>  	int ret = 0;
>  	struct ksmbd_work *work;
>  
> -	conn = READ_ONCE(opinfo->conn);
> +	conn = smb2_oplock_break_conn_get(opinfo, ci);
>  	if (!conn)
>  		return 0;
>  
>  	work = ksmbd_alloc_work_struct();
> -	if (!work)
> +	if (!work) {
> +		ksmbd_conn_put(conn);
>  		return -ENOMEM;
> +	}
>  
>  	br_info = kmalloc_obj(struct oplock_break_info, KSMBD_DEFAULT_GFP);
>  	if (!br_info) {
>  		ksmbd_free_work_struct(work);
> +		ksmbd_conn_put(conn);
>  		return -ENOMEM;
>  	}
>  
> @@ -741,7 +777,8 @@ static int smb2_oplock_break_noti(struct oplock_info *opinfo)
>  	br_info->open_trunc = opinfo->open_trunc;
>  
>  	work->request_buf = (char *)br_info;
> -	work->conn = ksmbd_conn_get(conn);
> +	/* Transfer the reference acquired by smb2_oplock_break_conn_get(). */
> +	work->conn = conn;
>  	work->sess = opinfo->sess;
>  
>  	ksmbd_conn_r_count_inc(conn);
> @@ -890,8 +927,8 @@ static void wait_lease_breaking(struct oplock_info *opinfo)
>  	}
>  }
>  
> -static int oplock_break(struct oplock_info *brk_opinfo, int req_op_level,
> -			struct ksmbd_work *in_work)
> +static int oplock_break(struct oplock_info *brk_opinfo, struct ksmbd_inode *ci,
> +			int req_op_level, struct ksmbd_work *in_work)
>  {
>  	int err = 0;
>  
> @@ -957,7 +994,7 @@ static int oplock_break(struct oplock_info *brk_opinfo, int req_op_level,
>  	if (brk_opinfo->is_lease)
>  		err = smb2_lease_break_noti(brk_opinfo);
>  	else
> -		err = smb2_oplock_break_noti(brk_opinfo);
> +		err = smb2_oplock_break_noti(brk_opinfo, ci);
>  
>  	ksmbd_debug(OPLOCK, "oplock granted = %d\n", brk_opinfo->level);
>  	if (brk_opinfo->op_state == OPLOCK_CLOSING)
> @@ -1137,7 +1174,7 @@ void smb_send_parent_lease_break_noti(struct ksmbd_file *fp,
>  				continue;
>  			}
>  
> -			oplock_break(opinfo, SMB2_OPLOCK_LEVEL_NONE, NULL);
> +			oplock_break(opinfo, p_ci, SMB2_OPLOCK_LEVEL_NONE, NULL);
>  			opinfo_put(opinfo);
>  		}
>  	}
> @@ -1178,7 +1215,7 @@ void smb_lazy_parent_lease_break_close(struct ksmbd_file *fp)
>  				continue;
>  			}
>  
> -			oplock_break(opinfo, SMB2_OPLOCK_LEVEL_NONE, NULL);
> +			oplock_break(opinfo, p_ci, SMB2_OPLOCK_LEVEL_NONE, NULL);
>  			opinfo_put(opinfo);
>  		}
>  	}
> @@ -1280,7 +1317,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>  		goto op_break_not_needed;
>  	}
>  
> -	err = oplock_break(prev_opinfo, SMB2_OPLOCK_LEVEL_II, work);
> +	err = oplock_break(prev_opinfo, fp->f_ci, SMB2_OPLOCK_LEVEL_II, work);
>  	opinfo_put(prev_opinfo);
>  	if (err == -ENOENT)
>  		goto set_lev;
> @@ -1366,7 +1403,7 @@ static void smb_break_all_write_oplock(struct ksmbd_work *work,
>  	}
>  
>  	brk_opinfo->open_trunc = is_trunc;
> -	oplock_break(brk_opinfo, SMB2_OPLOCK_LEVEL_II, work);
> +	oplock_break(brk_opinfo, fp->f_ci, SMB2_OPLOCK_LEVEL_II, work);
>  	opinfo_put(brk_opinfo);
>  }
>  
> @@ -1430,7 +1467,7 @@ void smb_break_all_levII_oplock(struct ksmbd_work *work, struct ksmbd_file *fp,
>  			    SMB2_LEASE_KEY_SIZE))
>  			goto next;
>  		brk_op->open_trunc = is_trunc;
> -		oplock_break(brk_op, SMB2_OPLOCK_LEVEL_NONE, NULL);
> +		oplock_break(brk_op, ci, SMB2_OPLOCK_LEVEL_NONE, NULL);
>  next:
>  		opinfo_put(brk_op);
>  	}
> -- 
> 2.53.0
> 
> 

7.1.y is long end-of-life, sorry.