Linux CIFS filesystem development
 help / color / mirror / Atom feed
* [PATCH] ksmbd: fix OOB read in ksmbd_validate_name_reconnect()
@ 2026-09-27  6:33 Dairui Zhang
  2026-09-28  4:54 ` Namjae Jeon
  2026-09-28  5:20 ` [PATCH v2] ksmbd: fix OOB read and cross-share confusion " Dairui Zhang
  0 siblings, 2 replies; 7+ messages in thread
From: Dairui Zhang @ 2026-09-27  6:33 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Steve French, Sergey Senozhatsky, Tom Talpey,
	Paulo Alcantara, Dairui Zhang, stable

ksmbd_validate_name_reconnect() indexes
ab_pathname[share->path_sz + 1] without checking that the path is
longer than share->path_sz + 1 (d_path() fills the buffer from the
end). A durable fp is not bound to its original share, so a client
can preserve a file from a short-path share and reconnect it on a
long-path share, making strcmp() read past the PATH_MAX buffer -
with a long enough share path this oopses the kernel, and on a
mapped page it also gives a byte-equality oracle on adjacent heap
(the reconnect succeeds exactly when the out-of-bounds string
equals the requested name). A same-share variant with the durable
fp at the share root reads 1 byte past the buffer
(strlen(ab_pathname) == share->path_sz).

Check the prefix before indexing: the path must be longer than
share->path_sz and the next character must be '/'. The comparison
for the valid case is unchanged.

Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
 fs/smb/server/vfs_cache.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 293dab9..52e83cf 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1714,9 +1714,13 @@ int ksmbd_validate_name_reconnect(struct ksmbd_share_config *share,
 		return -EACCES;
 	}
 
-	if (name && strcmp(&ab_pathname[share->path_sz + 1], name)) {
-		ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
-		ret = -EINVAL;
+	if (name) {
+		if (strlen(ab_pathname) <= share->path_sz ||
+		    ab_pathname[share->path_sz] != '/' ||
+		    strcmp(&ab_pathname[share->path_sz + 1], name)) {
+			ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
+			ret = -EINVAL;
+		}
 	}
 
 	kfree(pathname);
-- 
2.53.0


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

* Re: [PATCH] ksmbd: fix OOB read in ksmbd_validate_name_reconnect()
  2026-09-27  6:33 [PATCH] ksmbd: fix OOB read in ksmbd_validate_name_reconnect() Dairui Zhang
@ 2026-09-28  4:54 ` Namjae Jeon
  2026-09-28  5:20 ` [PATCH v2] ksmbd: fix OOB read and cross-share confusion " Dairui Zhang
  1 sibling, 0 replies; 7+ messages in thread
From: Namjae Jeon @ 2026-09-28  4:54 UTC (permalink / raw)
  To: Dairui Zhang
  Cc: linux-cifs, Steve French, Sergey Senozhatsky, Tom Talpey,
	Paulo Alcantara, stable

> @@ -1714,9 +1714,13 @@ int ksmbd_validate_name_reconnect(struct ksmbd_share_config *share,
>                 return -EACCES;
>         }
>
> -       if (name && strcmp(&ab_pathname[share->path_sz + 1], name)) {
> -               ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
> -               ret = -EINVAL;
> +       if (name) {
> +               if (strlen(ab_pathname) <= share->path_sz ||
> +                   ab_pathname[share->path_sz] != '/' ||
> +                   strcmp(&ab_pathname[share->path_sz + 1], name)) {
> +                       ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
> +                       ret = -EINVAL;
> +               }
This change adds path length and separator checks, but still does not
verify that  ab_pathname starts with share->path. A reconnect through
/srv/b can therefore still accept a durable handle for /srv/a/file
when the requested name is file. Please check the share path prefix
and component boundary before comparing the relative name.

And can you check Sashiko' review comments ?

https://sashiko.dev/#/patchset/20260927063339.1731061-1-zhangdairui%40gmail.com

Thanks.

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

* [PATCH v2] ksmbd: fix OOB read and cross-share confusion in ksmbd_validate_name_reconnect()
  2026-09-27  6:33 [PATCH] ksmbd: fix OOB read in ksmbd_validate_name_reconnect() Dairui Zhang
  2026-09-28  4:54 ` Namjae Jeon
@ 2026-09-28  5:20 ` Dairui Zhang
  2026-09-28 14:55   ` Namjae Jeon
  2026-09-30  0:16   ` [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk Dairui Zhang
  1 sibling, 2 replies; 7+ messages in thread
From: Dairui Zhang @ 2026-09-28  5:20 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Steve French, Sergey Senozhatsky, Tom Talpey,
	Paulo Alcantara, Dairui Zhang, stable

ksmbd_validate_name_reconnect() indexes
ab_pathname[share->path_sz + 1] without checking that the path is
longer than share->path_sz + 1 (d_path() fills the buffer from the
end). A durable fp is not bound to its original share, so a client
can preserve a file from a short-path share and reconnect it on a
long-path share, making strcmp() read past the PATH_MAX buffer -
with a long enough share path this oopses the kernel, and on a
mapped page it also gives a byte-equality oracle on adjacent heap
(the reconnect succeeds exactly when the out-of-bounds string
equals the requested name). A same-share variant with the durable
fp at the share root reads 1 byte past the buffer
(strlen(ab_pathname) == share->path_sz).

Check before comparing: the path must be longer than share->path_sz,
it must start with share->path, and the next character must be '/'.
The share root itself is accepted only for an empty name.

Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
v1 -> v2:
- Also verify the share path prefix and component boundary before
  comparing the relative name, per Namjae's review (a same-length
  share could otherwise pass the separator check for a file from a
  different share).
- Accept the share-root case explicitly (fp is the share root,
  name is empty), per the Sashiko review note - v1 rejected it.
- v1: https://lore.kernel.org/linux-cifs/20260927063339.1731061-1-zhangdairui@gmail.com/
---
 fs/smb/server/vfs_cache.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 293dab9..1acab9a 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1714,9 +1714,21 @@ int ksmbd_validate_name_reconnect(struct ksmbd_share_config *share,
 		return -EACCES;
 	}
 
-	if (name && strcmp(&ab_pathname[share->path_sz + 1], name)) {
-		ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
-		ret = -EINVAL;
+	if (name) {
+		size_t len = strlen(ab_pathname);
+
+		if (len == share->path_sz && !strncmp(ab_pathname, share->path, len)) {
+			/* the durable fp is the share root itself */
+			if (name[0])
+				ret = -EINVAL;
+		} else if (len <= share->path_sz ||
+			   strncmp(ab_pathname, share->path, share->path_sz) ||
+			   ab_pathname[share->path_sz] != '/' ||
+			   strcmp(&ab_pathname[share->path_sz + 1], name)) {
+			ret = -EINVAL;
+		}
+		if (ret)
+			ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
 	}
 
 	kfree(pathname);
-- 
2.53.0


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

* Re: [PATCH v2] ksmbd: fix OOB read and cross-share confusion in ksmbd_validate_name_reconnect()
  2026-09-28  5:20 ` [PATCH v2] ksmbd: fix OOB read and cross-share confusion " Dairui Zhang
@ 2026-09-28 14:55   ` Namjae Jeon
  2026-09-30  0:16   ` [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk Dairui Zhang
  1 sibling, 0 replies; 7+ messages in thread
From: Namjae Jeon @ 2026-09-28 14:55 UTC (permalink / raw)
  To: Dairui Zhang
  Cc: linux-cifs, Steve French, Sergey Senozhatsky, Tom Talpey,
	Paulo Alcantara, stable

On Mon, Sep 28, 2026 at 2:20 PM Dairui Zhang <zhangdairui@gmail.com> wrote:
>
> ksmbd_validate_name_reconnect() indexes
> ab_pathname[share->path_sz + 1] without checking that the path is
> longer than share->path_sz + 1 (d_path() fills the buffer from the
> end). A durable fp is not bound to its original share, so a client
> can preserve a file from a short-path share and reconnect it on a
> long-path share, making strcmp() read past the PATH_MAX buffer -
> with a long enough share path this oopses the kernel, and on a
> mapped page it also gives a byte-equality oracle on adjacent heap
> (the reconnect succeeds exactly when the out-of-bounds string
> equals the requested name). A same-share variant with the durable
> fp at the share root reads 1 byte past the buffer
> (strlen(ab_pathname) == share->path_sz).
>
> Check before comparing: the path must be longer than share->path_sz,
> it must start with share->path, and the next character must be '/'.
> The share root itself is accepted only for an empty name.
>
> Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
> Reported-by: Dairui Zhang <zhangdairui@gmail.com>
> Assisted-by: LLM
> Cc: stable@vger.kernel.org
> Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
Applied it to #ksmbd-for-next.
Thanks!

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

* [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk
  2026-09-28  5:20 ` [PATCH v2] ksmbd: fix OOB read and cross-share confusion " Dairui Zhang
  2026-09-28 14:55   ` Namjae Jeon
@ 2026-09-30  0:16   ` Dairui Zhang
  2026-10-04  1:23     ` Namjae Jeon
  2026-10-04 15:11     ` Dairui Zhang
  1 sibling, 2 replies; 7+ messages in thread
From: Dairui Zhang @ 2026-09-30  0:16 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-cifs, smfrench, senozhatsky, tom, pc, stable, Dairui Zhang

Namjae,

I went over my v2 patch again and found some problems with it.
Sorry for the issues in my earlier patch. I wrote a more
rigorous version; could you please take a look and let me know
if you see any problems with it?

The string prefix check in v2 rejects legitimate durable
reconnects when the share path contains a symlink, because
d_path() of the fp is resolved while the configured string is
not. The new version replaces the string check with a dentry
walk towards share->vfs_path, and also fixes the older case of
a share exported at "/".

From c8efcc786146a951091588e5fa7e3c754850cb3c Mon Sep 17 00:00:00 2001
From: Dairui Zhang <zhangdairui@gmail.com>
Date: Wed, 30 Sep 2026 03:10:00 +0800
Subject: [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a
 dentry walk

The name check in ksmbd_validate_name_reconnect() compares
d_path() of the durable fp against share->path as strings. The
fp path is resolved, the configured string is not, so when an
intermediate component of the share path is a symlink, a
legitimate reconnect to the same file on the same share is
rejected. Example: /a/link -> /real with the share configured
as /a/link/b: a durable handle for /real/b/foo is rejected.

Replace the string checks with an object-based walk from the
fp's dentry towards share->vfs_path, the same root the open
paths use. Each component is compared against the requested
name from its end, under rename_lock so a concurrent rename
cannot mix snapshots. Mount roots are detected by mnt_root and
crossed with follow_up(), so files in nested mounts (crossmnt)
keep working. Reaching the filesystem root without meeting the
share root means the fp does not belong to the share. The share
root itself only matches an empty name, and an unlinked fp is
rejected explicitly. Each walk is limited to 8192 hops and 16
walk attempts, returning -EAGAIN when a limit is hit.

This also fixes reconnects through a share exported at "/",
where the original code compared "oo" for /foo. With no PATH_MAX
buffer or d_path() left in this function, the out-of-bounds
read fixed by v1/v2 cannot reoccur.

Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
Follow-up to the v2 patch in ksmbd-for-next: the string prefix
check still rejects legitimate reconnects on symlinked share
paths, this rework fixes that and the older root-"/" case.
- v2: https://lore.kernel.org/linux-cifs/20260928052026.1789765-1-zhangdairui@gmail.com/
---
 fs/smb/server/vfs_cache.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+), 27 deletions(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -11,6 +11,7 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 #include <linux/dcache.h>
+#include <linux/namei.h>
 
 #include "glob.h"
 #include "vfs_cache.h"
@@ -1701,38 +1702,98 @@
 int ksmbd_validate_name_reconnect(struct ksmbd_share_config *share,
 				  struct ksmbd_file *fp, char *name)
 {
-	char *pathname, *ab_pathname;
+	const struct path *root = &share->vfs_path;
+	struct path cur;
+	const char *p, *end;
+	unsigned int seq, attempts = 0, hops;
 	int ret = 0;
 
-	pathname = kmalloc(PATH_MAX, KSMBD_DEFAULT_GFP);
-	if (!pathname)
-		return -EACCES;
-
-	ab_pathname = d_path(&fp->filp->f_path, pathname, PATH_MAX);
-	if (IS_ERR(ab_pathname)) {
-		kfree(pathname);
-		return -EACCES;
-	}
-
-	if (name) {
-		size_t len = strlen(ab_pathname);
-
-		if (len == share->path_sz && !strncmp(ab_pathname, share->path, len)) {
-			/* the durable fp is the share root itself */
-			if (name[0])
+	if (!name)
+		return 0;
+	if (name[0] == '/' || d_unlinked(fp->filp->f_path.dentry))
+		return -EINVAL;
+
+	for (;;) {
+		if (++attempts > 16)
+			return -EAGAIN;
+		ret = 0;
+		p = name;
+		end = name + strlen(name);
+		cur = fp->filp->f_path;
+		path_get(&cur);
+		seq = read_seqbegin(&rename_lock);
+		hops = 0;
+		for (;;) {
+			const char *comp;
+			size_t clen;
+			struct dentry *parent;
+
+			/* Bound the walk so mount/rename storms cannot spin forever. */
+			if (++hops > 8192) {
+				ret = -EAGAIN;
+				break;
+			}
+
+			if (cur.dentry == root->dentry &&
+			    cur.mnt == root->mnt) {
+				/* the share root itself only matches an empty name */
+				if (end != p)
+					ret = -EINVAL;
+				break;
+			}
+
+			if (cur.dentry == cur.mnt->mnt_root) {
+				/*
+				 * Mount root: hop into the parent mount and
+				 * let its mountpoint supply the component.
+				 * Mount topology can change mid-walk
+				 * (mount_lock is not available here), but
+				 * racing that needs local mount privileges.
+				 */
+				if (!follow_up(&cur)) {
+					ret = -EINVAL;
+					break;
+				}
+				continue;
+			}
+
+			for (comp = end; comp > p && comp[-1] != '/'; comp--)
+				;
+			clen = end - comp;
+			if (!clen) {
+				/* name exhausted while the fp is deeper, or empty component */
 				ret = -EINVAL;
-		} else if (len <= share->path_sz ||
-			   strncmp(ab_pathname, share->path, share->path_sz) ||
-			   ab_pathname[share->path_sz] != '/' ||
-			   strcmp(&ab_pathname[share->path_sz + 1], name)) {
-			ret = -EINVAL;
-		}
-		if (ret)
-			ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
-	}
+				break;
+			}
 
-	kfree(pathname);
+			spin_lock(&cur.dentry->d_lock);
+			if (cur.dentry->d_name.len != clen ||
+			    memcmp(cur.dentry->d_name.name, comp, clen))
+				ret = -EINVAL;
+			spin_unlock(&cur.dentry->d_lock);
+			if (ret)
+				break;
+
+			parent = dget_parent(cur.dentry);
+			if (parent == cur.dentry) {
+				/* escaped the mount without reaching the share root */
+				dput(parent);
+				ret = -EINVAL;
+				break;
+			}
+			dput(cur.dentry);
+			cur.dentry = parent;
 
+			end = comp;
+			if (end > p && end[-1] == '/')
+				end--;
+		}
+		path_put(&cur);
+		if (!read_seqretry(&rename_lock, seq))
+			break;
+	}
+	if (ret)
+		ksmbd_debug(SMB, "invalid name reconnect %s\n", name);
 	return ret;
 }
 
-- 
2.53.0

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

* Re: [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk
  2026-09-30  0:16   ` [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk Dairui Zhang
@ 2026-10-04  1:23     ` Namjae Jeon
  2026-10-04 15:11     ` Dairui Zhang
  1 sibling, 0 replies; 7+ messages in thread
From: Namjae Jeon @ 2026-10-04  1:23 UTC (permalink / raw)
  To: Dairui Zhang; +Cc: linux-cifs, smfrench, senozhatsky, tom, pc

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]

On Wed, Sep 30, 2026 at 9:16 AM Dairui Zhang <zhangdairui@gmail.com> wrote:
>
> Namjae,
Hi Dairui,
>
> I went over my v2 patch again and found some problems with it.
> Sorry for the issues in my earlier patch. I wrote a more
> rigorous version; could you please take a look and let me know
> if you see any problems with it?
>
> The string prefix check in v2 rejects legitimate durable
> reconnects when the share path contains a symlink, because
> d_path() of the fp is resolved while the configured string is
> not. The new version replaces the string check with a dentry
> walk towards share->vfs_path, and also fixes the older case of
> a share exported at "/".
Can you check if the attached patches fix this issue?

Thanks!

[-- Attachment #2: 0002-ksmbd-fix-durable-reconnects-for-root-shares.patch --]
[-- Type: text/x-patch, Size: 1526 bytes --]

From 0274201c77cacfb7f7914a604b2a28c468c5333d Mon Sep 17 00:00:00 2001
From: Namjae Jeon <linkinjeon@kernel.org>
Date: Sun, 4 Oct 2026 09:58:56 +0900
Subject: [PATCH 2/2] ksmbd: fix durable reconnects for root shares

When a share exports "/", ksmbd_validate_name_reconnect() expects
another slash after the share path. For a file at /foo, this rejects
the valid reconnect name "foo" even though the file is in the share.

Compare the requested name with the rendered file path after its
leading slash when the resolved share root is "/". Keep the
exact-root/empty-name check and reject rendered paths without an
initial slash.

Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/smb/server/vfs_cache.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 38afc5a3799d..ae30ee08e5e8 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1838,6 +1838,10 @@ int ksmbd_validate_name_reconnect(struct ksmbd_share_config *share,
 			/* the durable fp is the share root itself */
 			if (name[0])
 				ret = -EINVAL;
+		} else if (share_path_sz == 1 && share_path[0] == '/') {
+			if (ab_pathname[0] != '/' ||
+			    strcmp(ab_pathname + 1, name))
+				ret = -EINVAL;
 		} else if (len <= share_path_sz ||
 			   strncmp(ab_pathname, share_path, share_path_sz) ||
 			   ab_pathname[share_path_sz] != '/' ||
-- 
2.25.1


[-- Attachment #3: 0001-ksmbd-cache-the-resolved-share-path-for-durable-reco.patch --]
[-- Type: text/x-patch, Size: 4929 bytes --]

From 4505f6085832c2ee6abf723291c3e4db38d5cfc6 Mon Sep 17 00:00:00 2001
From: Namjae Jeon <linkinjeon@kernel.org>
Date: Sun, 4 Oct 2026 09:58:34 +0900
Subject: [PATCH 1/2] ksmbd: cache the resolved share path for durable
 reconnects

A share configured as /a/link/b, with /a/link pointing to /real, is
opened relative to share->vfs_path at /real/b. However,
ksmbd_validate_name_reconnect() compares the file's d_path() result
against the original /a/link/b string and rejects a valid reconnect.

Render share->vfs_path with d_path() after the existing kern_path()
lookup and cache a copy in real_path along with its length. Use the
resolved string for durable reconnect name validation. Keep path and
path_sz as the configured spelling for existing consumers, including
the volume serial calculation and path lookups.

Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/smb/server/mgmt/share_config.c | 43 +++++++++++++++++++++++++++++--
 fs/smb/server/mgmt/share_config.h |  2 ++
 fs/smb/server/vfs_cache.c         | 12 +++++----
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/fs/smb/server/mgmt/share_config.c b/fs/smb/server/mgmt/share_config.c
index cc9f18ede80d..f85134c38cdd 100644
--- a/fs/smb/server/mgmt/share_config.c
+++ b/fs/smb/server/mgmt/share_config.c
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <linux/rwsem.h>
 #include <linux/parser.h>
+#include <linux/dcache.h>
 #include <linux/namei.h>
 #include <linux/fs_struct.h>
 #include <linux/sched.h>
@@ -108,6 +109,7 @@ static void kill_share(struct ksmbd_share_config *share)
 		path_put(&share->vfs_path);
 	kfree(share->name);
 	kfree(share->path);
+	kfree(share->real_path);
 	kfree(share);
 }
 
@@ -182,6 +184,43 @@ static int parse_veto_list(struct ksmbd_share_config *share,
 	return 0;
 }
 
+static int share_config_resolve_path(struct ksmbd_share_config *share)
+{
+	char *buf, *path;
+	int ret;
+
+	ret = kern_path(share->path, 0, &share->vfs_path);
+	if (ret)
+		return ret;
+
+	buf = kmalloc(PATH_MAX, KSMBD_DEFAULT_GFP);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	path = d_path(&share->vfs_path, buf, PATH_MAX);
+	if (IS_ERR(path)) {
+		ret = PTR_ERR(path);
+		goto out_buf;
+	}
+
+	path = kstrdup(path, KSMBD_DEFAULT_GFP);
+	if (!path) {
+		ret = -ENOMEM;
+		goto out_buf;
+	}
+
+	share->real_path = path;
+	share->real_path_sz = strlen(path);
+out_buf:
+	kfree(buf);
+out:
+	if (ret)
+		path_put(&share->vfs_path);
+	return ret;
+}
+
 static struct ksmbd_share_config *share_config_request(struct ksmbd_work *work,
 						       const char *name)
 {
@@ -272,12 +311,12 @@ static struct ksmbd_share_config *share_config_request(struct ksmbd_work *work,
 			}
 
 			scoped_with_init_fs()
-				ret = kern_path(share->path, 0, &share->vfs_path);
+				ret = share_config_resolve_path(share);
 			ksmbd_revert_fsids(work);
 			if (ret) {
 				ksmbd_debug(SMB, "failed to access '%s'\n",
 					    share->path);
-				/* Avoid put_path() */
+				/* No path reference is retained on failure. */
 				kfree(share->path);
 				share->path = NULL;
 			}
diff --git a/fs/smb/server/mgmt/share_config.h b/fs/smb/server/mgmt/share_config.h
index d157545fe7d1..91691b525907 100644
--- a/fs/smb/server/mgmt/share_config.h
+++ b/fs/smb/server/mgmt/share_config.h
@@ -16,8 +16,10 @@ struct ksmbd_work;
 struct ksmbd_share_config {
 	char			*name;
 	char			*path;
+	char			*real_path;
 
 	unsigned int		path_sz;
+	unsigned int		real_path_sz;
 	unsigned int		flags;
 	struct list_head	veto_list;
 
diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 7bc95fae4ac0..38afc5a3799d 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1816,6 +1816,8 @@ void ksmbd_free_global_file_table(void)
 int ksmbd_validate_name_reconnect(struct ksmbd_share_config *share,
 				  struct ksmbd_file *fp, char *name)
 {
+	const char *share_path = share->real_path;
+	size_t share_path_sz = share->real_path_sz;
 	char *pathname, *ab_pathname;
 	int ret = 0;
 
@@ -1832,14 +1834,14 @@ int ksmbd_validate_name_reconnect(struct ksmbd_share_config *share,
 	if (name) {
 		size_t len = strlen(ab_pathname);
 
-		if (len == share->path_sz && !strncmp(ab_pathname, share->path, len)) {
+		if (len == share_path_sz && !strncmp(ab_pathname, share_path, len)) {
 			/* the durable fp is the share root itself */
 			if (name[0])
 				ret = -EINVAL;
-		} else if (len <= share->path_sz ||
-			   strncmp(ab_pathname, share->path, share->path_sz) ||
-			   ab_pathname[share->path_sz] != '/' ||
-			   strcmp(&ab_pathname[share->path_sz + 1], name)) {
+		} else if (len <= share_path_sz ||
+			   strncmp(ab_pathname, share_path, share_path_sz) ||
+			   ab_pathname[share_path_sz] != '/' ||
+			   strcmp(&ab_pathname[share_path_sz + 1], name)) {
 			ret = -EINVAL;
 		}
 		if (ret)
-- 
2.25.1


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

* Re: [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk
  2026-09-30  0:16   ` [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk Dairui Zhang
  2026-10-04  1:23     ` Namjae Jeon
@ 2026-10-04 15:11     ` Dairui Zhang
  1 sibling, 0 replies; 7+ messages in thread
From: Dairui Zhang @ 2026-10-04 15:11 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: linux-cifs, smfrench, senozhatsky, tom, pc, stable

Namjae,

Thanks for the patches. I went through both and they look
correct, including the failure-path refcounting. My patch was
not good - it was too complicated. Yours is simpler and just
as effective.

One corner case: if the share root is unlinked between
kern_path() and d_path() at init, real_path can end up with a
" (deleted)" suffix, and a later cross-share fp at that
literal path would pass the prefix check.

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

end of thread, other threads:[~2026-10-04 15:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-27  6:33 [PATCH] ksmbd: fix OOB read in ksmbd_validate_name_reconnect() Dairui Zhang
2026-09-28  4:54 ` Namjae Jeon
2026-09-28  5:20 ` [PATCH v2] ksmbd: fix OOB read and cross-share confusion " Dairui Zhang
2026-09-28 14:55   ` Namjae Jeon
2026-09-30  0:16   ` [PATCH] ksmbd: rework ksmbd_validate_name_reconnect() to use a dentry walk Dairui Zhang
2026-10-04  1:23     ` Namjae Jeon
2026-10-04 15:11     ` Dairui Zhang

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