All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: ChenXiaoSong <chenxiaosong@kylinos.cn>,
	Namjae Jeon <linkinjeon@kernel.org>,
	Steve French <stfrench@microsoft.com>,
	Sasha Levin <sashal@kernel.org>,
	smfrench@gmail.com, linux-cifs@vger.kernel.org
Subject: [PATCH AUTOSEL 6.18-6.1] smb/server: fix return value of smb2_ioctl()
Date: Mon,  8 Dec 2025 19:14:59 -0500	[thread overview]
Message-ID: <20251209001610.611575-7-sashal@kernel.org> (raw)
In-Reply-To: <20251209001610.611575-1-sashal@kernel.org>

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


  parent reply	other threads:[~2025-12-09  0:16 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-09  0:14 [PATCH AUTOSEL 6.18-6.1] ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency Sasha Levin
2025-12-09  0:14 ` [PATCH AUTOSEL 6.18-6.17] wifi: rtw89: use skb_dequeue() for queued ROC packets to prevent racing Sasha Levin
2025-12-09  0:14 ` [PATCH AUTOSEL 6.18-6.6] ipv6: clean up routes when manually removing address with a lifetime Sasha Levin
2025-12-09  0:14 ` [PATCH AUTOSEL 6.18-5.10] ext4: remove page offset calculation in ext4_block_zero_page_range() Sasha Levin
2025-12-09  0:14 ` [PATCH AUTOSEL 6.18-6.6] fs/ntfs3: fix KMSAN uninit-value in ni_create_attr_list Sasha Levin
2025-12-09  0:14 ` [PATCH AUTOSEL 6.18-6.6] btrfs: abort transaction on item count overflow in __push_leaf_left() Sasha Levin
2025-12-09  0:14 ` Sasha Levin [this message]
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.1] gfs2: Fix use of bio_chain Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] Bluetooth: btusb: Add new VID/PID 13d3/3533 for RTL8821CE Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] wifi: mac80211: reset CRC valid after CSA Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] Bluetooth: btusb: Add new VID/PID 0x0489/0xE12F for RTL8852BE-VT Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] wifi: mt76: mmio_*_copy fix byte order and alignment Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] btrfs: scrub: always update btrfs_scrub_progress::last_physical Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] bpf: Skip bounds adjustment for conditional jumps on same scalar register Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] wifi: rtl8xxxu: Fix HT40 channel config for RTL8192CU, RTL8723AU Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] Bluetooth: btusb: MT7920: Add VID/PID 0489/e135 Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] Bluetooth: btusb: MT7922: Add VID/PID 0489/e170 Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] virtio_blk: NULL out vqs to avoid double free on failed resume Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.1] kbuild: Use objtree for module signing key path Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.17] btrfs: use kvcalloc for btrfs_bio::csum allocation Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] net: sched: Don't use WARN_ON_ONCE() for -ENOMEM in tcf_classify() Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] hfsplus: Verify inode mode when loading from disk Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.6] gfs2: fix remote evict for read-only filesystems Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] net: amd-xgbe: use EOPNOTSUPP instead of ENOTSUPP in xgbe_phy_mii_read_c45 Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] net: init shinfo->gso_segs from qdisc_pkt_len_init() Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.17] Bluetooth: btusb: add new custom firmwares Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] hfsplus: fix missing hfs_bnode_get() in __hfs_bnode_create Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] cxgb4: Rename sched_class to avoid type clash Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] net: mana: Drop TX skb on post_work_request failure and unmap resources Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] hfsplus: fix volume corruption issue for generic/070 Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.17] wifi: rtw89: rtw8852bu: Added dev id for ASUS AX57 NANO USB Wifi dongle Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] net: restore napi_consume_skb()'s NULL-handling Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.15] fs/ntfs3: Support timestamps prior to epoch Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.1] smb/server: fix return value of smb2_query_dir() Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.17] wifi: rtw88: Add BUFFALO WI-U3-866DHP to the USB ID list Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.6] Bluetooth: btusb: Add new VID/PID 2b89/6275 for RTL8761BUV Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] bpf: Disable file_alloc_security hook Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.1] wifi: rtw89: phy: fix out-of-bounds access in rtw89_phy_read_txpwr_limit() Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.6] ntfs: set dummy blocksize to read boot_block when mounting Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-5.10] hfsplus: fix volume corruption issue for generic/073 Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] wifi: mt76: mt792x: fix wifi init fail by setting MCU_RUNNING after CLC load Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.12] gfs2: Fix "gfs2: Switch to wait_event in gfs2_quotad" Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.6] ksmbd: vfs: fix race on m_flags in vfs_cache Sasha Levin
2025-12-09  0:15 ` [PATCH AUTOSEL 6.18-6.1] wifi: rtw89: flush TX queue before deleting key Sasha Levin
2025-12-09  0:15 ` [Intel-wired-lan] [PATCH AUTOSEL 6.18-6.12] ice: Allow 100M speed for E825C SGMII device Sasha Levin
2025-12-09  0:15   ` Sasha Levin
  -- strict thread matches above, loose matches on Subject: below --
2025-12-06 14:02 [PATCH AUTOSEL 6.18-6.1] ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency Sasha Levin
2025-12-06 14:02 ` [PATCH AUTOSEL 6.18-6.1] smb/server: fix return value of smb2_ioctl() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251209001610.611575-7-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=chenxiaosong@kylinos.cn \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=smfrench@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=stfrench@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.