mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] 9p: handle long directory entry names in readdir
@ 2026-09-16 13:54 Haobin Wu
  2026-09-16 13:54 ` [PATCH v2 1/2] 9p: skip intermediate directory entry name copy in p9dirent_read() Haobin Wu
  2026-09-16 13:54 ` [PATCH v2 2/2] 9p: skip directory entries with names longer than NAME_MAX Haobin Wu
  0 siblings, 2 replies; 5+ messages in thread
From: Haobin Wu @ 2026-09-16 13:54 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus, v9fs
  Cc: linux_oss, davem, edumazet, kuba, pabeni, horms, aneesh.kumar,
	netdev, linux-kernel, linux-fsdevel, jack, willy, dhowells, viro,
	brauner

Hi Dominique,

Thanks for the review, and sorry for the delay. Here is v2 with the
changes you and Jan asked for; details below.

A host directory entry whose name is longer than 255 bytes currently
makes 9p's readdir fail with -EIO and hides every entry after it, which
is how this was noticed on WSL (Plan 9 shares of Windows drives).

Patch 1 drops the copy into the fixed 256-byte p9_dirent::d_name and
uses the string p9pdu_vreadf() already allocated, so the listing no
longer aborts.

Patch 2 then skips entries whose name is longer than NAME_MAX instead
of returning them to userspace, as discussed with Dominique and Jan on
v1: the VFS only enforces PATH_MAX in verify_dirent_name(), and nothing
can operate on such a name afterwards anyway.

Changes since v1:
- Use my real name in From/Signed-off-by (Dominique).
- Split into two patches (Dominique).
- Reworded the subject and commit message of patch 1 to describe the
  existing readdir path and clarify that no allocation is added
  (Dominique).
- New patch 2 skipping entries longer than NAME_MAX (Dominique, Jan).
- Dropped the bouncing sripathik@in.ibm.com address from Cc.

v1: https://lore.kernel.org/all/20260826064819.52523-1-853555@gmail.com/

Haobin Wu (2):
  9p: skip intermediate directory entry name copy in p9dirent_read()
  9p: skip directory entries with names longer than NAME_MAX

 fs/9p/vfs_dir.c         | 17 +++++++++++++----
 include/net/9p/client.h |  2 +-
 net/9p/protocol.c       | 12 ++----------
 3 files changed, 16 insertions(+), 15 deletions(-)


base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.54.0 (Apple Git-157)


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

* [PATCH v2 1/2] 9p: skip intermediate directory entry name copy in p9dirent_read()
  2026-09-16 13:54 [PATCH v2 0/2] 9p: handle long directory entry names in readdir Haobin Wu
@ 2026-09-16 13:54 ` Haobin Wu
  2026-09-16 13:54 ` [PATCH v2 2/2] 9p: skip directory entries with names longer than NAME_MAX Haobin Wu
  1 sibling, 0 replies; 5+ messages in thread
From: Haobin Wu @ 2026-09-16 13:54 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus, v9fs
  Cc: linux_oss, davem, edumazet, kuba, pabeni, horms, aneesh.kumar,
	netdev, linux-kernel, linux-fsdevel, jack, willy, dhowells, viro,
	brauner

v9fs_dir_readdir_dotl() decodes each entry of a Rreaddir reply with
p9dirent_read(), which parses it through p9pdu_readf("Qqbs"). The 's'
conversion in p9pdu_vreadf() already allocates a NUL-terminated copy
of the name from the wire buffer; p9dirent_read() then strscpy()s that
copy into the fixed 256-byte p9_dirent::d_name and frees the original.

The wire format carries the name length in 16 bits, so a name longer
than 255 bytes is valid on the wire and may well be valid on the
server's filesystem, but it makes strscpy() return -E2BIG.
v9fs_dir_readdir_dotl() turns that into -EIO and aborts getdents64(),
so every entry after the long one disappears from the listing.

Drop the second copy: keep the string allocated by p9pdu_vreadf() in
p9_dirent and let v9fs_dir_readdir_dotl(), its only user, free it once
dir_emit() has consumed it. p9_dirent is a short-lived stack object, so
the string's lifetime does not change.

Note that the VFS only rejects names of PATH_MAX bytes or more in
verify_dirent_name(), so after this change a name between NAME_MAX and
PATH_MAX is returned by getdents64() even though any later lookup on it
fails with -ENAMETOOLONG. The next patch skips such entries.

Fixes: 7751bdb3a095 ("9p: readdir implementation for 9p2000.L")
Closes: https://github.com/microsoft/WSL/issues/41192
Assisted-by: Codex:gpt-5
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Haobin Wu <853555@gmail.com>
---
 fs/9p/vfs_dir.c         |  6 +++++-
 include/net/9p/client.h |  2 +-
 net/9p/protocol.c       | 12 ++----------
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index e0d34e4e9076..af00b79d801e 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -185,8 +185,12 @@ static int v9fs_dir_readdir_dotl(struct file *file, struct dir_context *ctx)
 			if (!dir_emit(ctx, curdirent.d_name,
 				      strlen(curdirent.d_name),
 				      QID2INO(&curdirent.qid),
-				      curdirent.d_type))
+				      curdirent.d_type)) {
+				kfree(curdirent.d_name);
 				return 0;
+			}
+
+			kfree(curdirent.d_name);
 
 			ctx->pos = curdirent.d_off;
 			rdir->head += err;
diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index 838a94218b59..9f3079c6f386 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -268,7 +268,7 @@ struct p9_dirent {
 	struct p9_qid qid;
 	u64 d_off;
 	unsigned char d_type;
-	char d_name[256];
+	char *d_name;
 };
 
 struct iov_iter;
diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index 67b0586d807f..d4335884e0c0 100644
--- a/net/9p/protocol.c
+++ b/net/9p/protocol.c
@@ -770,7 +770,7 @@ int p9dirent_read(struct p9_client *clnt, char *buf, int len,
 {
 	struct p9_fcall fake_pdu;
 	int ret;
-	char *nameptr;
+	char *nameptr = NULL;
 
 	fake_pdu.size = len;
 	fake_pdu.capacity = len;
@@ -785,15 +785,7 @@ int p9dirent_read(struct p9_client *clnt, char *buf, int len,
 		return ret;
 	}
 
-	ret = strscpy(dirent->d_name, nameptr, sizeof(dirent->d_name));
-	if (ret < 0) {
-		p9_debug(P9_DEBUG_ERROR,
-			 "On the wire dirent name too long: %s\n",
-			 nameptr);
-		kfree(nameptr);
-		return ret;
-	}
-	kfree(nameptr);
+	dirent->d_name = nameptr;
 
 	return fake_pdu.offset;
 }
-- 
2.54.0 (Apple Git-157)


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

* [PATCH v2 2/2] 9p: skip directory entries with names longer than NAME_MAX
  2026-09-16 13:54 [PATCH v2 0/2] 9p: handle long directory entry names in readdir Haobin Wu
  2026-09-16 13:54 ` [PATCH v2 1/2] 9p: skip intermediate directory entry name copy in p9dirent_read() Haobin Wu
@ 2026-09-16 13:54 ` Haobin Wu
  2026-09-16 20:02   ` Christian Schoenebeck
  1 sibling, 1 reply; 5+ messages in thread
From: Haobin Wu @ 2026-09-16 13:54 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus, v9fs
  Cc: linux_oss, davem, edumazet, kuba, pabeni, horms, aneesh.kumar,
	netdev, linux-kernel, linux-fsdevel, jack, willy, dhowells, viro,
	brauner

The 9p wire format carries directory entry names of up to 65535 bytes
and nothing on the client checks them against NAME_MAX. Since the
previous patch, v9fs_dir_readdir_dotl() passes such names straight to
dir_emit(), and the VFS only rejects names of PATH_MAX bytes or more in
verify_dirent_name(), as -EIO, which again fails the whole getdents64()
call.

A name between NAME_MAX and PATH_MAX is therefore returned to userspace
even though every later operation on it fails with -ENAMETOOLONG, and
POSIX requires readdir() to only return components of at most NAME_MAX
bytes. Nothing can use such an entry, so skip it instead of returning it
or failing the listing, reusing the strlen() result that was already
computed for dir_emit().

Suggested-by: Dominique Martinet <asmadeus@codewreck.org>
Link: https://lore.kernel.org/all/vz5bum547fqyxf5z4m3x7tuqkuq52jlopm65t7hvynqeulh7i3@2t4wnhfxs7qv/
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Haobin Wu <853555@gmail.com>
---
 fs/9p/vfs_dir.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index af00b79d801e..ad6fdc99883d 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -173,6 +173,7 @@ static int v9fs_dir_readdir_dotl(struct file *file, struct dir_context *ctx)
 		}
 
 		while (rdir->head < rdir->tail) {
+			size_t namelen;
 
 			err = p9dirent_read(fid->clnt, rdir->buf + rdir->head,
 					    rdir->tail - rdir->head,
@@ -182,10 +183,14 @@ static int v9fs_dir_readdir_dotl(struct file *file, struct dir_context *ctx)
 				return -EIO;
 			}
 
-			if (!dir_emit(ctx, curdirent.d_name,
-				      strlen(curdirent.d_name),
-				      QID2INO(&curdirent.qid),
-				      curdirent.d_type)) {
+			namelen = strlen(curdirent.d_name);
+			if (namelen > NAME_MAX) {
+				p9_debug(P9_DEBUG_VFS,
+					 "skipping entry with %zu byte name\n",
+					 namelen);
+			} else if (!dir_emit(ctx, curdirent.d_name, namelen,
+					     QID2INO(&curdirent.qid),
+					     curdirent.d_type)) {
 				kfree(curdirent.d_name);
 				return 0;
 			}
-- 
2.54.0 (Apple Git-157)


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

* Re: [PATCH v2 2/2] 9p: skip directory entries with names longer than NAME_MAX
  2026-09-16 13:54 ` [PATCH v2 2/2] 9p: skip directory entries with names longer than NAME_MAX Haobin Wu
@ 2026-09-16 20:02   ` Christian Schoenebeck
  2026-09-16 22:45     ` Dominique Martinet
  0 siblings, 1 reply; 5+ messages in thread
From: Christian Schoenebeck @ 2026-09-16 20:02 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus, v9fs, Haobin Wu
  Cc: davem, edumazet, kuba, pabeni, horms, aneesh.kumar, netdev,
	linux-kernel, linux-fsdevel, jack, willy, dhowells, viro,
	brauner

On Wednesday, 16 September 2026 15:54:03 CEST Haobin Wu wrote:
> The 9p wire format carries directory entry names of up to 65535 bytes
> and nothing on the client checks them against NAME_MAX. Since the
> previous patch, v9fs_dir_readdir_dotl() passes such names straight to
> dir_emit(), and the VFS only rejects names of PATH_MAX bytes or more in
> verify_dirent_name(), as -EIO, which again fails the whole getdents64()
> call.
> 
> A name between NAME_MAX and PATH_MAX is therefore returned to userspace
> even though every later operation on it fails with -ENAMETOOLONG, and
> POSIX requires readdir() to only return components of at most NAME_MAX
> bytes. Nothing can use such an entry, so skip it instead of returning it
> or failing the listing, reusing the strlen() result that was already
> computed for dir_emit().
> 
> Suggested-by: Dominique Martinet <asmadeus@codewreck.org>
> Link:
> https://lore.kernel.org/all/vz5bum547fqyxf5z4m3x7tuqkuq52jlopm65t7hvynqeulh
> 7i3@2t4wnhfxs7qv/ Assisted-by: Claude:claude-fable-5-1
> Signed-off-by: Haobin Wu <853555@gmail.com>
> ---
>  fs/9p/vfs_dir.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
> index af00b79d801e..ad6fdc99883d 100644
> --- a/fs/9p/vfs_dir.c
> +++ b/fs/9p/vfs_dir.c
> @@ -173,6 +173,7 @@ static int v9fs_dir_readdir_dotl(struct file *file,
> struct dir_context *ctx) }
> 
>  		while (rdir->head < rdir->tail) {
> +			size_t namelen;
> 
>  			err = p9dirent_read(fid->clnt, rdir->buf + rdir->head,
>  					    rdir->tail - rdir->head,
> @@ -182,10 +183,14 @@ static int v9fs_dir_readdir_dotl(struct file *file,
> struct dir_context *ctx) return -EIO;
>  			}
> 
> -			if (!dir_emit(ctx, curdirent.d_name,
> -				      strlen(curdirent.d_name),
> -				      QID2INO(&curdirent.qid),
> -				      curdirent.d_type)) {
> +			namelen = strlen(curdirent.d_name);
> +			if (namelen > NAME_MAX) {
> +				p9_debug(P9_DEBUG_VFS,
> +					 "skipping entry with %zu byte name\n",
> +					 namelen);

Fair to say why:

  "skip dentry: name length %zu > NAME_MAX"

This is only handled for 9p2000.L so far. For legacy 9p2000(.u) this should
then also be limited in v9fs_dir_readdir() IMO.

/Christian

> +			} else if (!dir_emit(ctx, curdirent.d_name, namelen,
> +					     QID2INO(&curdirent.qid),
> +					     curdirent.d_type)) {
>  				kfree(curdirent.d_name);
>  				return 0;
>  			}





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

* Re: [PATCH v2 2/2] 9p: skip directory entries with names longer than NAME_MAX
  2026-09-16 20:02   ` Christian Schoenebeck
@ 2026-09-16 22:45     ` Dominique Martinet
  0 siblings, 0 replies; 5+ messages in thread
From: Dominique Martinet @ 2026-09-16 22:45 UTC (permalink / raw)
  To: Christian Schoenebeck
  Cc: ericvh, lucho, v9fs, Haobin Wu, davem, edumazet, kuba, pabeni,
	horms, aneesh.kumar, netdev, linux-kernel, linux-fsdevel, jack,
	willy, dhowells, viro, brauner

Christian Schoenebeck wrote on Wed, Sep 16, 2026 at 10:02:08PM +0200:
> > +			if (namelen > NAME_MAX) {
> > +				p9_debug(P9_DEBUG_VFS,
> > +					 "skipping entry with %zu byte name\n",
> > +					 namelen);
> 
> Fair to say why:
> 
>   "skip dentry: name length %zu > NAME_MAX"

Agreed.

I'd also say to keep this as P9_DEBUG_ERROR rather than VFS: it should
be rare enough and nobody will enable DEBUG_VFS immediately, this keeps
the message at the same log level as it was previously

(P9_DEBUG_ERROR is not printed by default, but I've been meaning to
try to change that eventually... Someday™)

> This is only handled for 9p2000.L so far. For legacy 9p2000(.u) this should
> then also be limited in v9fs_dir_readdir() IMO.

In practice this was already the case because of the processing in
p9dirent_read(), so I have no strong opinion here, but I do agree it
makes more sense to be symetrical yes.
If done it should be noted as a behavior change in the commit message
(or split in yet another commit) though

-- 
Dominique Martinet | Asmadeus

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

end of thread, other threads:[~2026-09-16 22:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 13:54 [PATCH v2 0/2] 9p: handle long directory entry names in readdir Haobin Wu
2026-09-16 13:54 ` [PATCH v2 1/2] 9p: skip intermediate directory entry name copy in p9dirent_read() Haobin Wu
2026-09-16 13:54 ` [PATCH v2 2/2] 9p: skip directory entries with names longer than NAME_MAX Haobin Wu
2026-09-16 20:02   ` Christian Schoenebeck
2026-09-16 22:45     ` Dominique Martinet

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

all inboxes | Powered by JetHome®