public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ksmbd: fail share config requests when path allocation fails
@ 2026-04-29  8:59 Shuhao Fu
  2026-04-29  9:03 ` Shuhao Fu
  2026-04-29 13:43 ` Namjae Jeon
  0 siblings, 2 replies; 3+ messages in thread
From: Shuhao Fu @ 2026-04-29  8:59 UTC (permalink / raw)
  To: Namjae Jeon, Steve French, linux-cifs
  Cc: Sergey Senozhatsky, Tom Talpey, linux-kernel

Non-pipe shares must have a duplicated backing path before they can be
published. share_config_request() currently calls kstrndup() for that
path, but if the allocation fails it leaves ret unchanged. If veto list
parsing succeeds and share->name exists, the partially built share is
still inserted into the global share table with share->path left NULL.

A later share-root SMB2 create uses tree_conn->share_conf->path as the
lookup root. If the share was published with path == NULL, that request
passes a NULL pathname into do_getname_kernel()/strlen() and can crash
the ksmbd worker.

Set ret = -ENOMEM when path duplication fails so the incomplete share is
destroyed before publication.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
---
 fs/smb/server/mgmt/share_config.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/smb/server/mgmt/share_config.c b/fs/smb/server/mgmt/share_config.c
index 53f44ff4d376f3e..6f97f8d39657cd2 100644
--- a/fs/smb/server/mgmt/share_config.c
+++ b/fs/smb/server/mgmt/share_config.c
@@ -167,7 +167,10 @@ static struct ksmbd_share_config *share_config_request(struct ksmbd_work *work,
 
 		share->path = kstrndup(ksmbd_share_config_path(resp), path_len,
 				      KSMBD_DEFAULT_GFP);
-		if (share->path) {
+		if (!share->path) {
+			ret = -ENOMEM;
+		} else {
+			ret = 0;
 			share->path_sz = strlen(share->path);
 			while (share->path_sz > 1 &&
 			       share->path[share->path_sz - 1] == '/')
@@ -179,9 +182,10 @@ static struct ksmbd_share_config *share_config_request(struct ksmbd_work *work,
 		share->force_directory_mode = resp->force_directory_mode;
 		share->force_uid = resp->force_uid;
 		share->force_gid = resp->force_gid;
-		ret = parse_veto_list(share,
-				      KSMBD_SHARE_CONFIG_VETO_LIST(resp),
-				      resp->veto_list_sz);
+		if (!ret)
+			ret = parse_veto_list(share,
+					      KSMBD_SHARE_CONFIG_VETO_LIST(resp),
+					      resp->veto_list_sz);
 		if (!ret && share->path) {
 			if (__ksmbd_override_fsids(work, share)) {
 				kill_share(share);
-- 
2.49.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ksmbd: fail share config requests when path allocation fails
  2026-04-29  8:59 [PATCH] ksmbd: fail share config requests when path allocation fails Shuhao Fu
@ 2026-04-29  9:03 ` Shuhao Fu
  2026-04-29 13:43 ` Namjae Jeon
  1 sibling, 0 replies; 3+ messages in thread
From: Shuhao Fu @ 2026-04-29  9:03 UTC (permalink / raw)
  To: Namjae Jeon, Steve French, linux-cifs
  Cc: Sergey Senozhatsky, Tom Talpey, linux-kernel

Hi,

I did a live repro on a current mainline tree to confirm that the
published NULL share path is not just an internal invariant break and can
fault a normal ksmbd request path.

The reproducer forced only the share path duplication failure that
produces the published NULL path state. No consumer-side ksmbd path was
modified.

Repro setup:

1. Start from mainline with CONFIG_SMB_SERVER=y and CONFIG_CIFS=y.
2. Add a narrow temporary trigger in share_config_request() so that only
   one test share forces the path duplication step to fail, leaving
   share->path as NULL while still allowing the share to be published.
   No consumer-side ksmbd path was modified.
3. Boot that kernel under QEMU/KVM with a custom initramfs.
4. In the guest, run a tiny userspace generic-netlink responder that:
   - sends KSMBD_EVENT_STARTING_UP
   - answers login requests as guest
   - answers share config requests for the test share with a valid
     export path
   - answers tree connect requests as writable guest
5. From the same guest, mount that test share over loopback with:

   mount -t cifs //<guest-ip>/<test-share> /mnt/cifs \
     -o guest,vers=3.1.1,port=445,uid=0,gid=0

That mount attempt hit the expected fault path. The clean serial log
showed:

  [   21.045838] CIFS: Attempting to mount //<guest-ip>/<test-share>
  [   21.052302] BUG: kernel NULL pointer dereference, address: 0000000000000000
  [   21.062798] RIP: 0010:strlen+0xb/0x20
  [   21.077290]  do_getname_kernel+0x12/0xf0
  [   21.077707]  __ksmbd_vfs_kern_path+0x89/0x390
  [   21.078648]  smb2_open+0xa03/0x20b0

This matches the expected downstream flow:

- tree connect stores the published share in tree_conn->share_conf
- a share-root SMB2 create builds an empty pathname in smb2_open()
- ksmbd_vfs_path_lookup() interprets that as "use the share root" and
  substitutes tree_conn->share_conf->path
- the broken published share has tree_conn->share_conf->path == NULL
- that NULL pathname reaches do_getname_kernel()/strlen()

I also did a second run with the guest netlink responder logging directly
to the serial console. That log was noisy because the userspace prints
interleaved with the kernel Oops, but it still showed the login request
and the tree connect for the test share immediately around the crash.

I can provide more detail, including the temporary repro pieces and the
serial logs, if useful.

Thanks,
Shuhao

On Wed, Apr 29, 2026 at 05:00:13PM +0800, Shuhao Fu wrote:
> Non-pipe shares must have a duplicated backing path before they can be
> published. share_config_request() currently calls kstrndup() for that
> path, but if the allocation fails it leaves ret unchanged. If veto list
> parsing succeeds and share->name exists, the partially built share is
> still inserted into the global share table with share->path left NULL.
> 
> A later share-root SMB2 create uses tree_conn->share_conf->path as the
> lookup root. If the share was published with path == NULL, that request
> passes a NULL pathname into do_getname_kernel()/strlen() and can crash
> the ksmbd worker.
> 
> Set ret = -ENOMEM when path duplication fails so the incomplete share is
> destroyed before publication.
> 
> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ksmbd: fail share config requests when path allocation fails
  2026-04-29  8:59 [PATCH] ksmbd: fail share config requests when path allocation fails Shuhao Fu
  2026-04-29  9:03 ` Shuhao Fu
@ 2026-04-29 13:43 ` Namjae Jeon
  1 sibling, 0 replies; 3+ messages in thread
From: Namjae Jeon @ 2026-04-29 13:43 UTC (permalink / raw)
  To: Shuhao Fu
  Cc: Steve French, linux-cifs, Sergey Senozhatsky, Tom Talpey,
	linux-kernel

On Wed, Apr 29, 2026 at 6:00 PM Shuhao Fu <sfual@cse.ust.hk> wrote:
>
> Non-pipe shares must have a duplicated backing path before they can be
> published. share_config_request() currently calls kstrndup() for that
> path, but if the allocation fails it leaves ret unchanged. If veto list
> parsing succeeds and share->name exists, the partially built share is
> still inserted into the global share table with share->path left NULL.
>
> A later share-root SMB2 create uses tree_conn->share_conf->path as the
> lookup root. If the share was published with path == NULL, that request
> passes a NULL pathname into do_getname_kernel()/strlen() and can crash
> the ksmbd worker.
>
> Set ret = -ENOMEM when path duplication fails so the incomplete share is
> destroyed before publication.
>
> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
> Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
Applied it to #ksmbd-for-next-next.
Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-29 13:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-29  8:59 [PATCH] ksmbd: fail share config requests when path allocation fails Shuhao Fu
2026-04-29  9:03 ` Shuhao Fu
2026-04-29 13:43 ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox