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; }
/**