* [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®