[PATCH AUTOSEL 6.18-6.1] ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency

[PATCH AUTOSEL 6.18-6.1] ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency

[Sasha Levin]

From: Namjae Jeon <linkinjeon@kernel.org>

[ Upstream commit b39a1833cc4a2755b02603eec3a71a85e9dff926 ]

Under high concurrency, A tree-connection object (tcon) is freed on
a disconnect path while another path still holds a reference and later
executes *_put()/write on it.

Reported-by: Qianchang Zhao <pioooooooooip@gmail.com>
Reported-by: Zhitong Liu <liuzhitong1993@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:



 fs/smb/server/mgmt/tree_connect.c | 18 ++++--------------
 fs/smb/server/mgmt/tree_connect.h |  1 -
 fs/smb/server/smb2pdu.c           |  3 ---
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/fs/smb/server/mgmt/tree_connect.c b/fs/smb/server/mgmt/tree_connect.c
index ecfc575086712..d3483d9c757c7 100644
--- a/fs/smb/server/mgmt/tree_connect.c
+++ b/fs/smb/server/mgmt/tree_connect.c
@@ -78,7 +78,6 @@ ksmbd_tree_conn_connect(struct ksmbd_work *work, const char *share_name)
 	tree_conn->t_state = TREE_NEW;
 	status.tree_conn = tree_conn;
 	atomic_set(&tree_conn->refcount, 1);
-	init_waitqueue_head(&tree_conn->refcount_q);
 
 	ret = xa_err(xa_store(&sess->tree_conns, tree_conn->id, tree_conn,
 			      KSMBD_DEFAULT_GFP));
@@ -100,14 +99,8 @@ ksmbd_tree_conn_connect(struct ksmbd_work *work, const char *share_name)
 
 void ksmbd_tree_connect_put(struct ksmbd_tree_connect *tcon)
 {
-	/*
-	 * Checking waitqueue to releasing tree connect on
-	 * tree disconnect. waitqueue_active is safe because it
-	 * uses atomic operation for condition.
-	 */
-	if (!atomic_dec_return(&tcon->refcount) &&
-	    waitqueue_active(&tcon->refcount_q))
-		wake_up(&tcon->refcount_q);
+	if (atomic_dec_and_test(&tcon->refcount))
+		kfree(tcon);
 }
 
 int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
@@ -119,14 +112,11 @@ int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
 	xa_erase(&sess->tree_conns, tree_conn->id);
 	write_unlock(&sess->tree_conns_lock);
 
-	if (!atomic_dec_and_test(&tree_conn->refcount))
-		wait_event(tree_conn->refcount_q,
-			   atomic_read(&tree_conn->refcount) == 0);
-
 	ret = ksmbd_ipc_tree_disconnect_request(sess->id, tree_conn->id);
 	ksmbd_release_tree_conn_id(sess, tree_conn->id);
 	ksmbd_share_config_put(tree_conn->share_conf);
-	kfree(tree_conn);
+	if (atomic_dec_and_test(&tree_conn->refcount))
+		kfree(tree_conn);
 	return ret;
 }
 
diff --git a/fs/smb/server/mgmt/tree_connect.h b/fs/smb/server/mgmt/tree_connect.h
index a42cdd0510411..f0023d86716f2 100644
--- a/fs/smb/server/mgmt/tree_connect.h
+++ b/fs/smb/server/mgmt/tree_connect.h
@@ -33,7 +33,6 @@ struct ksmbd_tree_connect {
 	int				maximal_access;
 	bool				posix_extensions;
 	atomic_t			refcount;
-	wait_queue_head_t		refcount_q;
 	unsigned int			t_state;
 };
 
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index a2830ec67e782..dba29881debdc 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -2200,7 +2200,6 @@ int smb2_tree_disconnect(struct ksmbd_work *work)
 		goto err_out;
 	}
 
-	WARN_ON_ONCE(atomic_dec_and_test(&tcon->refcount));
 	tcon->t_state = TREE_DISCONNECTED;
 	write_unlock(&sess->tree_conns_lock);
 
@@ -2210,8 +2209,6 @@ int smb2_tree_disconnect(struct ksmbd_work *work)
 		goto err_out;
 	}
 
-	work->tcon = NULL;
-
 	rsp->StructureSize = cpu_to_le16(4);
 	err = ksmbd_iov_pin_rsp(work, rsp,
 				sizeof(struct smb2_tree_disconnect_rsp));
-- 
2.51.0

[PATCH AUTOSEL 6.18-6.1] ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency

[Sasha Levin]

From: Namjae Jeon <linkinjeon@kernel.org>

[ Upstream commit b39a1833cc4a2755b02603eec3a71a85e9dff926 ]

Under high concurrency, A tree-connection object (tcon) is freed on
a disconnect path while another path still holds a reference and later
executes *_put()/write on it.

Reported-by: Qianchang Zhao <pioooooooooip@gmail.com>
Reported-by: Zhitong Liu <liuzhitong1993@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

### 3. CLASSIFICATION

This is a **security/stability bug fix**:
- **Use-after-free (UAF)** is a serious memory safety issue
- ksmbd is **network-facing code** (SMB server) - security-sensitive
- Can lead to kernel crashes, data corruption, or potential remote
  exploitation

### 4. SCOPE AND RISK ASSESSMENT

**Size**: ~30 lines changed across 3 files
**Files affected**:
- `fs/smb/server/mgmt/tree_connect.c` - core reference counting logic
- `fs/smb/server/mgmt/tree_connect.h` - struct definition
- `fs/smb/server/smb2pdu.c` - disconnect path

**Technical mechanism of the bug**:

The previous fix (commit 33b235a6e6ebe) introduced a waitqueue-based
refcount mechanism:
1. `ksmbd_tree_conn_disconnect()` would decrement refcount, wait for it
   to hit 0, then always call `kfree()`
2. `ksmbd_tree_connect_put()` would decrement refcount and wake up
   waiters

**The race condition**:
- Thread A: In disconnect, waits for refcount to hit 0, then runs more
  code before `kfree()`
- Thread B: Drops last reference via `_put()`, refcount hits 0
- Thread A: Wakes up, but Thread B might still be executing code that
  accesses `tcon`
- Thread A: Frees `tcon`
- Thread B: UAF when accessing `tcon` after `_put()` returns

**The fix**: Changes to standard "kref-style" reference counting:
- Whoever drops refcount to 0 immediately calls `kfree()`
- No window between refcount hitting 0 and free
- Removes the buggy waitqueue mechanism entirely

**Risk**: LOW
- The new pattern (free on last put) is the standard kernel pattern
  (kref)
- Simpler code is easier to verify correct
- Self-contained within tree_connect subsystem

### 5. USER IMPACT

- **Affected users**: Anyone running ksmbd (kernel SMB server)
- **Trigger**: High concurrency - realistic for file servers
- **Severity**: HIGH - kernel crash or potential security exploitation
- **Real-world occurrence**: Two Reported-by tags confirm users hit this

### 6. STABILITY INDICATORS

- Signed-off by ksmbd maintainer (Namjae Jeon) and CIFS/SMB maintainer
  (Steve French)
- Two independent reporters indicate real bug
- The buggy code was introduced in commit 33b235a6e6ebe (v6.6-rc1)

### 7. DEPENDENCY CHECK

- The fix is self-contained
- Depends only on commit 33b235a6e6ebe being present (which introduced
  the bug)
- Affects: v6.6 and all later versions
- Should apply cleanly - only 2 minor unrelated commits to
  tree_connect.c since the buggy commit

### Summary

| Criterion | Assessment |
|-----------|------------|
| Fixes real bug | ✅ UAF in network-facing code |
| Security impact | ✅ High - potential remote exploitation |
| Small and contained | ✅ ~30 lines, 3 files, single subsystem |
| No new features | ✅ Pure bug fix |
| User-reported | ✅ Two Reported-by tags |
| Clean backport | ✅ Self-contained fix |

This commit fixes a use-after-free vulnerability in ksmbd, the in-kernel
SMB server. The bug exists in the reference counting mechanism for tree
connections and can be triggered under concurrent access - a realistic
scenario for network file servers. UAF bugs in network-facing kernel
code are serious security issues. The fix is small, uses a well-
established kernel pattern (kref-style refcounting), and is self-
contained. It should be backported to all stable kernels containing
commit 33b235a6e6ebe (v6.6+).

**YES**

 fs/smb/server/mgmt/tree_connect.c | 18 ++++--------------
 fs/smb/server/mgmt/tree_connect.h |  1 -
 fs/smb/server/smb2pdu.c           |  3 ---
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/fs/smb/server/mgmt/tree_connect.c b/fs/smb/server/mgmt/tree_connect.c
index ecfc575086712..d3483d9c757c7 100644
--- a/fs/smb/server/mgmt/tree_connect.c
+++ b/fs/smb/server/mgmt/tree_connect.c
@@ -78,7 +78,6 @@ ksmbd_tree_conn_connect(struct ksmbd_work *work, const char *share_name)
 	tree_conn->t_state = TREE_NEW;
 	status.tree_conn = tree_conn;
 	atomic_set(&tree_conn->refcount, 1);
-	init_waitqueue_head(&tree_conn->refcount_q);
 
 	ret = xa_err(xa_store(&sess->tree_conns, tree_conn->id, tree_conn,
 			      KSMBD_DEFAULT_GFP));
@@ -100,14 +99,8 @@ ksmbd_tree_conn_connect(struct ksmbd_work *work, const char *share_name)
 
 void ksmbd_tree_connect_put(struct ksmbd_tree_connect *tcon)
 {
-	/*
-	 * Checking waitqueue to releasing tree connect on
-	 * tree disconnect. waitqueue_active is safe because it
-	 * uses atomic operation for condition.
-	 */
-	if (!atomic_dec_return(&tcon->refcount) &&
-	    waitqueue_active(&tcon->refcount_q))
-		wake_up(&tcon->refcount_q);
+	if (atomic_dec_and_test(&tcon->refcount))
+		kfree(tcon);
 }
 
 int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
@@ -119,14 +112,11 @@ int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
 	xa_erase(&sess->tree_conns, tree_conn->id);
 	write_unlock(&sess->tree_conns_lock);
 
-	if (!atomic_dec_and_test(&tree_conn->refcount))
-		wait_event(tree_conn->refcount_q,
-			   atomic_read(&tree_conn->refcount) == 0);
-
 	ret = ksmbd_ipc_tree_disconnect_request(sess->id, tree_conn->id);
 	ksmbd_release_tree_conn_id(sess, tree_conn->id);
 	ksmbd_share_config_put(tree_conn->share_conf);
-	kfree(tree_conn);
+	if (atomic_dec_and_test(&tree_conn->refcount))
+		kfree(tree_conn);
 	return ret;
 }
 
diff --git a/fs/smb/server/mgmt/tree_connect.h b/fs/smb/server/mgmt/tree_connect.h
index a42cdd0510411..f0023d86716f2 100644
--- a/fs/smb/server/mgmt/tree_connect.h
+++ b/fs/smb/server/mgmt/tree_connect.h
@@ -33,7 +33,6 @@ struct ksmbd_tree_connect {
 	int				maximal_access;
 	bool				posix_extensions;
 	atomic_t			refcount;
-	wait_queue_head_t		refcount_q;
 	unsigned int			t_state;
 };
 
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 447e76da44409..aae42d2abf7bc 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -2200,7 +2200,6 @@ int smb2_tree_disconnect(struct ksmbd_work *work)
 		goto err_out;
 	}
 
-	WARN_ON_ONCE(atomic_dec_and_test(&tcon->refcount));
 	tcon->t_state = TREE_DISCONNECTED;
 	write_unlock(&sess->tree_conns_lock);
 
@@ -2210,8 +2209,6 @@ int smb2_tree_disconnect(struct ksmbd_work *work)
 		goto err_out;
 	}
 
-	work->tcon = NULL;
-
 	rsp->StructureSize = cpu_to_le16(4);
 	err = ksmbd_iov_pin_rsp(work, rsp,
 				sizeof(struct smb2_tree_disconnect_rsp));
-- 
2.51.0

[PATCH AUTOSEL 6.18-6.1] smb/server: fix return value of smb2_ioctl()

[Sasha Levin]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

[ Upstream commit 269df046c1e15ab34fa26fd90db9381f022a0963 ]

__process_request() will not print error messages if smb2_ioctl()
always returns 0.

Fix this by returning the correct value at the end of function.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## ANALYSIS SUMMARY

### 1. Problem Being Fixed

The `smb2_ioctl()` function in ksmbd (the in-kernel SMB server) **always
returns 0** even when errors occur. Looking at the caller
`__process_request()` in `fs/smb/server/server.c:148-151`:

```c
ret = cmds->proc(work);  // calls smb2_ioctl

if (ret < 0)
    ksmbd_debug(CONN, "Failed to process %u [%d]\n", command, ret);
```

Since `smb2_ioctl()` returns 0 on error paths, the debug message is
never printed and errors are silently swallowed.

### 2. Technical Analysis of the Fix

The fix makes three changes:

1. **Line 8167**: Adds `ret = -EOPNOTSUPP` when `req->Flags !=
   SMB2_0_IOCTL_IS_FSCTL` (was previously not setting ret)

2. **Line 8187-8189**: For DFS referrals, adds `ret = -EOPNOTSUPP` and
   uses new `out2` label to skip the ret-to-status translation (since
   DFS needs specific STATUS_FS_DRIVER_REQUIRED)

3. **Line 8479**: Changes `return 0;` to `return ret;`

The function's documentation says: "Return: 0 on success, otherwise
error" - this fix makes the code match that contract.

### 3. Stable Kernel Criteria Assessment

| Criterion | Assessment |
|-----------|------------|
| Obviously correct | ✅ YES - Function was documented to return errors
but didn't |
| Fixes real bug | ✅ YES - Error reporting/debugging was broken |
| Small and contained | ✅ YES - ~10 lines changed in one function |
| No new features | ✅ YES - Only corrects error return behavior |
| Tested | ✅ YES - Acked by ksmbd maintainer (Namjae Jeon) |

### 4. Risk Assessment

**LOW RISK:**
- The fix only affects the return value in error paths
- Does not change the SMB protocol behavior or response status codes
- The `out2` label is a minor structural change to preserve DFS-specific
  status
- ksmbd is self-contained; this won't affect other subsystems
- Error logging/visibility improvement with zero functional risk

### 5. Concerns

- **No explicit stable tags** (no `Cc: stable@vger.kernel.org`)
- **No Fixes: tag** indicating when the bug was introduced
- The bug has likely existed since ksmbd was added (v5.15), so affects
  all stable branches with ksmbd

### 6. User Impact

Users of ksmbd who encounter errors during IOCTL handling:
- **Before**: Silent failures, no debug messages, harder to diagnose
  issues
- **After**: Proper error returns enabling logging and debugging

### Conclusion

This is a straightforward bug fix that corrects an obviously broken
return value. The fix is small, surgical, and low-risk. It improves
error visibility for ksmbd users and makes the code match its documented
behavior. The maintainer Ack from Namjae Jeon adds confidence. Despite
lacking explicit stable tags, it clearly meets all stable kernel
criteria.

**YES**

 fs/smb/server/smb2pdu.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 8975b6f2f5800..447e76da44409 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -8164,7 +8164,7 @@ int smb2_ioctl(struct ksmbd_work *work)
 		id = req->VolatileFileId;
 
 	if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
-		rsp->hdr.Status = STATUS_NOT_SUPPORTED;
+		ret = -EOPNOTSUPP;
 		goto out;
 	}
 
@@ -8184,8 +8184,9 @@ int smb2_ioctl(struct ksmbd_work *work)
 	case FSCTL_DFS_GET_REFERRALS:
 	case FSCTL_DFS_GET_REFERRALS_EX:
 		/* Not support DFS yet */
+		ret = -EOPNOTSUPP;
 		rsp->hdr.Status = STATUS_FS_DRIVER_REQUIRED;
-		goto out;
+		goto out2;
 	case FSCTL_CREATE_OR_GET_OBJECT_ID:
 	{
 		struct file_object_buf_type1_ioctl_rsp *obj_buf;
@@ -8475,8 +8476,10 @@ int smb2_ioctl(struct ksmbd_work *work)
 		rsp->hdr.Status = STATUS_BUFFER_TOO_SMALL;
 	else if (ret < 0 || rsp->hdr.Status == 0)
 		rsp->hdr.Status = STATUS_INVALID_PARAMETER;
+
+out2:
 	smb2_set_err_rsp(work);
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.51.0

[PATCH AUTOSEL 6.18-6.1] smb/server: fix return value of smb2_query_dir()

[Sasha Levin]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

[ Upstream commit dafe22bc676d4fcb1ccb193c8cc3dda57942509d ]

__process_request() will not print error messages if smb2_query_dir()
always returns 0.

Fix this by returning the correct value at the end of function.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of smb/server: fix return value of smb2_query_dir()

### 1. COMMIT MESSAGE ANALYSIS

The commit message clearly states:
- `__process_request()` will not print error messages if
  `smb2_query_dir()` always returns 0
- The fix returns the correct error value `rc` instead of 0

**Notable absences:**
- No `Cc: stable@vger.kernel.org` tag
- No `Fixes:` tag identifying when the bug was introduced

**Positive signals:**
- Acked by Namjae Jeon (ksmbd maintainer)
- Signed off by Steve French (SMB maintainer)

### 2. CODE CHANGE ANALYSIS

The change is a single-line fix in the error handling path:

```c
- return 0;
+       return rc;
```

**Technical mechanism of the bug:**
Looking at the context, this is in an error handling block where:
1. `rc` contains an error code (-EINVAL, -EACCES, -ENOENT, -EBADF,
   -ENOMEM, -EFAULT, or -EIO)
2. The appropriate SMB status is set in `rsp->hdr.Status`
3. Error response is prepared with `smb2_set_err_rsp(work)`
4. Cleanup is done with `ksmbd_fd_put()` and `ksmbd_revert_fsids()`
5. **BUG**: The function returns 0 (success) instead of `rc` (the actual
   error)

**Root cause:** The caller `__process_request()` uses the return value
to determine if an error occurred. Returning 0 masks all errors,
preventing proper error logging and handling.

### 3. CLASSIFICATION

This is a **bug fix** - incorrect error return value handling. The
function was silently discarding error information that callers need.

### 4. SCOPE AND RISK ASSESSMENT

| Factor | Assessment |
|--------|------------|
| Lines changed | 1 |
| Files touched | 1 |
| Complexity | Trivial |
| Subsystem | ksmbd (kernel SMB server) |
| Risk level | **Very Low** |

The fix is surgical and obviously correct - the `rc` variable already
contains the appropriate error code, it just wasn't being returned.

### 5. USER IMPACT

- **Affected users:** ksmbd server users
- **Severity:** Medium - error conditions in directory queries are not
  properly reported
- **Consequences of the bug:**
  - Error messages not printed when they should be
  - Callers may not handle error conditions properly
  - Debugging ksmbd issues becomes harder

### 6. STABILITY INDICATORS

- Acked by ksmbd maintainer
- Signed off by SMB maintainer
- Simple, self-contained change

### 7. DEPENDENCY CHECK

- No dependencies on other commits
- ksmbd has been in the kernel since 5.15
- The fix applies to existing code paths

### STABLE KERNEL CRITERIA EVALUATION

| Criterion | Met? | Notes |
|-----------|------|-------|
| Obviously correct | ✅ | Trivially correct - return error code instead
of 0 |
| Fixes real bug | ✅ | Error propagation was broken |
| Small and contained | ✅ | Single line change |
| No new features | ✅ | Pure bug fix |
| Tested | ⚠️ | No Tested-by tag, but very low risk |

### RISK VS BENEFIT

**Benefits:**
- Fixes broken error propagation in ksmbd directory queries
- Enables proper error logging for debugging
- Very low risk due to trivial nature of fix

**Risks:**
- Minimal - the change is from "always return 0" to "return actual error
  code"
- Behavior change only affects error paths

### CONCLUSION

This is a straightforward, obviously correct bug fix. The function was
incorrectly returning 0 (success) in all error cases, causing error
information to be lost. The fix is a single line change that returns the
actual error code that was already being computed.

While the commit lacks explicit stable tags, it meets all stable
criteria: obviously correct, fixes a real bug affecting error handling,
trivially small scope, and no new features. The risk is minimal and the
fix improves error handling in ksmbd.

**YES**

 fs/smb/server/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index f901ae18e68ad..8975b6f2f5800 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -4560,7 +4560,7 @@ int smb2_query_dir(struct ksmbd_work *work)
 	smb2_set_err_rsp(work);
 	ksmbd_fd_put(work, dir_fp);
 	ksmbd_revert_fsids(work);
-	return 0;
+	return rc;
 }
 
 /**
-- 
2.51.0

[PATCH AUTOSEL 6.18-6.6] ksmbd: vfs: fix race on m_flags in vfs_cache

[Sasha Levin]

From: Qianchang Zhao <pioooooooooip@gmail.com>

[ Upstream commit 991f8a79db99b14c48d20d2052c82d65b9186cad ]

ksmbd maintains delete-on-close and pending-delete state in
ksmbd_inode->m_flags. In vfs_cache.c this field is accessed under
inconsistent locking: some paths read and modify m_flags under
ci->m_lock while others do so without taking the lock at all.

Examples:

 - ksmbd_query_inode_status() and __ksmbd_inode_close() use
   ci->m_lock when checking or updating m_flags.
 - ksmbd_inode_pending_delete(), ksmbd_set_inode_pending_delete(),
   ksmbd_clear_inode_pending_delete() and ksmbd_fd_set_delete_on_close()
   used to read and modify m_flags without ci->m_lock.

This creates a potential data race on m_flags when multiple threads
open, close and delete the same file concurrently. In the worst case
delete-on-close and pending-delete bits can be lost or observed in an
inconsistent state, leading to confusing delete semantics (files that
stay on disk after delete-on-close, or files that disappear while still
in use).

Fix it by:

 - Making ksmbd_query_inode_status() look at m_flags under ci->m_lock
   after dropping inode_hash_lock.
 - Adding ci->m_lock protection to all helpers that read or modify
   m_flags (ksmbd_inode_pending_delete(), ksmbd_set_inode_pending_delete(),
   ksmbd_clear_inode_pending_delete(), ksmbd_fd_set_delete_on_close()).
 - Keeping the existing ci->m_lock protection in __ksmbd_inode_close(),
   and moving the actual unlink/xattr removal outside the lock.

This unifies the locking around m_flags and removes the data race while
preserving the existing delete-on-close behaviour.

Reported-by: Qianchang Zhao <pioooooooooip@gmail.com>
Reported-by: Zhitong Liu <liuzhitong1993@gmail.com>
Signed-off-by: Qianchang Zhao <pioooooooooip@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis: ksmbd: vfs: fix race on m_flags in vfs_cache

### 1. Commit Message Analysis

**Bug Description**: The commit fixes a data race on `m_flags` field in
ksmbd's VFS cache. Multiple functions access/modify this field with
inconsistent locking - some use `ci->m_lock`, others don't.

**Keywords**: "race", "data race", "fix" - clear bug fix language

**Impact described**: "Delete-on-close and pending-delete bits can be
lost or observed in an inconsistent state, leading to confusing delete
semantics (files that stay on disk after delete-on-close, or files that
disappear while still in use)"

**Tags present**:
- Two `Reported-by:` tags (Qianchang Zhao, Zhitong Liu) - indicates real
  users hit this bug
- `Acked-by:` from Namjae Jeon (ksmbd maintainer)
- `Signed-off-by:` from Steve French (SMB maintainer)

**Tags missing**: No `Cc: stable@vger.kernel.org`, no `Fixes:` tag

### 2. Code Change Analysis

The fix is straightforward and mechanical - it adds proper locking
around all `m_flags` accesses:

| Function | Change |
|----------|--------|
| `ksmbd_query_inode_status()` | Moved m_flags check outside
inode_hash_lock, added `down_read(&ci->m_lock)` |
| `ksmbd_inode_pending_delete()` | Added read lock around flag check |
| `ksmbd_set_inode_pending_delete()` | Added write lock around flag
modification |
| `ksmbd_clear_inode_pending_delete()` | Added write lock around flag
modification |
| `ksmbd_fd_set_delete_on_close()` | Added write lock around flag
modification |
| `__ksmbd_inode_close()` | Restructured to hold lock only during flag
check/modify, moves I/O (unlink, xattr removal) outside the lock |

The pattern is consistent: acquire lock → read/modify flags → release
lock → perform any I/O operations outside lock.

### 3. Classification

**Bug type**: Concurrency bug (data race)
- NOT a feature addition
- NOT adding new APIs
- NOT a cleanup or optimization
- This is a correctness fix for a real race condition

### 4. Scope and Risk Assessment

**Scope**:
- Single file changed: `fs/smb/server/vfs_cache.c`
- ~60 lines of changes
- All changes are adding locking around existing operations

**Risk**: LOW
- Uses existing `ci->m_lock` rwsem that's already in the structure
- No new locking primitives introduced
- The restructuring in `__ksmbd_inode_close()` to move I/O outside the
  lock is actually safer (avoids holding lock during I/O)
- Pattern is well-understood: protect shared data with locks

### 5. User Impact

**Who is affected**: Users running ksmbd (in-kernel SMB3 server) with
concurrent file access/deletion

**Severity**: MEDIUM-HIGH
- File deletion semantics are broken (files may not be deleted when they
  should be, or disappear unexpectedly)
- This affects data integrity expectations
- Any ksmbd deployment with multiple concurrent clients could hit this

### 6. Stability Indicators

- Maintainer acks from both ksmbd maintainer (Namjae Jeon) and SMB
  maintainer (Steve French)
- Two independent reporters suggest this is a known/reproducible issue

### 7. Dependencies Check

The fix is self-contained:
- Uses existing `ci->m_lock` (already present in `ksmbd_inode`
  structure)
- Uses standard kernel locking APIs (`down_read/up_read`,
  `down_write/up_write`)
- No dependency on other patches

### 8. Stable Tree Applicability

ksmbd was added in Linux 5.15, so this applies to 5.15.y, 6.1.y, 6.6.y,
and later stable trees. The code structure appears stable enough that
this should apply cleanly.

---

## Summary

**Should this be backported?**

**YES** - This commit should be backported because:

1. **Fixes a real bug**: Data race causing incorrect file deletion
   behavior that users can actually hit
2. **User-visible impact**: Files not deleted when they should be
   (delete-on-close failing) or files disappearing unexpectedly
3. **Has real bug reports**: Two Reported-by tags indicate real users
   encountered this
4. **Small and contained**: Single file, straightforward addition of
   missing locking
5. **Low regression risk**: Adds locking around existing operations
   using existing infrastructure
6. **Maintainer approved**: Acked by ksmbd maintainer, signed off by SMB
   maintainer
7. **Correct fix**: The approach (unify locking around m_flags) is
   obviously correct

The lack of explicit `Cc: stable` tag is not disqualifying - the nature
of the bug (concurrency issue with data integrity implications) and the
quality of the fix (mechanical addition of proper locking) make this
appropriate stable material.

**YES**

 fs/smb/server/vfs_cache.c | 88 +++++++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 26 deletions(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index dfed6fce89049..6ef116585af64 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -112,40 +112,62 @@ int ksmbd_query_inode_status(struct dentry *dentry)
 
 	read_lock(&inode_hash_lock);
 	ci = __ksmbd_inode_lookup(dentry);
-	if (ci) {
-		ret = KSMBD_INODE_STATUS_OK;
-		if (ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS))
-			ret = KSMBD_INODE_STATUS_PENDING_DELETE;
-		atomic_dec(&ci->m_count);
-	}
 	read_unlock(&inode_hash_lock);
+	if (!ci)
+		return ret;
+
+	down_read(&ci->m_lock);
+	if (ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS))
+		ret = KSMBD_INODE_STATUS_PENDING_DELETE;
+	else
+		ret = KSMBD_INODE_STATUS_OK;
+	up_read(&ci->m_lock);
+
+	atomic_dec(&ci->m_count);
 	return ret;
 }
 
 bool ksmbd_inode_pending_delete(struct ksmbd_file *fp)
 {
-	return (fp->f_ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS));
+	struct ksmbd_inode *ci = fp->f_ci;
+	int ret;
+
+	down_read(&ci->m_lock);
+	ret = (ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS));
+	up_read(&ci->m_lock);
+
+	return ret;
 }
 
 void ksmbd_set_inode_pending_delete(struct ksmbd_file *fp)
 {
-	fp->f_ci->m_flags |= S_DEL_PENDING;
+	struct ksmbd_inode *ci = fp->f_ci;
+
+	down_write(&ci->m_lock);
+	ci->m_flags |= S_DEL_PENDING;
+	up_write(&ci->m_lock);
 }
 
 void ksmbd_clear_inode_pending_delete(struct ksmbd_file *fp)
 {
-	fp->f_ci->m_flags &= ~S_DEL_PENDING;
+	struct ksmbd_inode *ci = fp->f_ci;
+
+	down_write(&ci->m_lock);
+	ci->m_flags &= ~S_DEL_PENDING;
+	up_write(&ci->m_lock);
 }
 
 void ksmbd_fd_set_delete_on_close(struct ksmbd_file *fp,
 				  int file_info)
 {
-	if (ksmbd_stream_fd(fp)) {
-		fp->f_ci->m_flags |= S_DEL_ON_CLS_STREAM;
-		return;
-	}
+	struct ksmbd_inode *ci = fp->f_ci;
 
-	fp->f_ci->m_flags |= S_DEL_ON_CLS;
+	down_write(&ci->m_lock);
+	if (ksmbd_stream_fd(fp))
+		ci->m_flags |= S_DEL_ON_CLS_STREAM;
+	else
+		ci->m_flags |= S_DEL_ON_CLS;
+	up_write(&ci->m_lock);
 }
 
 static void ksmbd_inode_hash(struct ksmbd_inode *ci)
@@ -257,27 +279,41 @@ static void __ksmbd_inode_close(struct ksmbd_file *fp)
 	struct file *filp;
 
 	filp = fp->filp;
-	if (ksmbd_stream_fd(fp) && (ci->m_flags & S_DEL_ON_CLS_STREAM)) {
-		ci->m_flags &= ~S_DEL_ON_CLS_STREAM;
-		err = ksmbd_vfs_remove_xattr(file_mnt_idmap(filp),
-					     &filp->f_path,
-					     fp->stream.name,
-					     true);
-		if (err)
-			pr_err("remove xattr failed : %s\n",
-			       fp->stream.name);
+
+	if (ksmbd_stream_fd(fp)) {
+		bool remove_stream_xattr = false;
+
+		down_write(&ci->m_lock);
+		if (ci->m_flags & S_DEL_ON_CLS_STREAM) {
+			ci->m_flags &= ~S_DEL_ON_CLS_STREAM;
+			remove_stream_xattr = true;
+		}
+		up_write(&ci->m_lock);
+
+		if (remove_stream_xattr) {
+			err = ksmbd_vfs_remove_xattr(file_mnt_idmap(filp),
+						     &filp->f_path,
+						     fp->stream.name,
+						     true);
+			if (err)
+				pr_err("remove xattr failed : %s\n",
+				       fp->stream.name);
+		}
 	}
 
 	if (atomic_dec_and_test(&ci->m_count)) {
+		bool do_unlink = false;
+
 		down_write(&ci->m_lock);
 		if (ci->m_flags & (S_DEL_ON_CLS | S_DEL_PENDING)) {
 			ci->m_flags &= ~(S_DEL_ON_CLS | S_DEL_PENDING);
-			up_write(&ci->m_lock);
-			ksmbd_vfs_unlink(filp);
-			down_write(&ci->m_lock);
+			do_unlink = true;
 		}
 		up_write(&ci->m_lock);
 
+		if (do_unlink)
+			ksmbd_vfs_unlink(filp);
+
 		ksmbd_inode_free(ci);
 	}
 }
-- 
2.51.0