Re: [PATCH] 9p: dynamically allocate directory entry names

[Jan Kara]

On Mon 14-09-26 00:12:25, Dominique Martinet wrote:
> +fsdevel@ & vfs maintainers to Cc TL;DR:
> Should the 9p .iterate_shared() dir_emit files with path
> components > NAME_LEN, as allowed in fs/readdir.c verify_dirent_name()
> only enforcing length < PATH_MAX, or should we skip any such entry given
> nothing can interact with them later anyway?

So my take on this is that I'd refuse any entries larger than NAME_MAX
(255). As you say it is very likely something in userspace or other parts
of the kernel (fs drivers, ...) will break with such names so I don't think
there's a good reason to provoke these bugs. For example POSIX states
readdir(2) should be returning component of at most NAME_MAX length and
although we don't take POSIX very seriously in the kernel in this case I
don't think there's a good enough reason to deviate from it.

								Honza

> hoobnn wrote on Wed, Aug 26, 2026 at 02:48:19PM +0800:
> > p9dirent_read() copies names into a fixed 256-byte buffer. A name that is
> > valid on the host filesystem but longer than that buffer makes strscpy()
> > fail and aborts getdents64(), hiding the remaining directory entries.
> > 
> > Keep ownership of the protocol-allocated string in p9_dirent and free it
> > after dir_emit() consumes the name.
> > 
> > Fixes: 7751bdb3a095 ("9p: readdir implementation for 9p2000.L")
> > Closes: https://github.com/microsoft/WSL/issues/41192
> > Assisted-by: Codex:gpt-5
> > Signed-off-by: hoobnn <853555@xxxxxxxxx>
> 
> Sorry, but pseudonyms are not allowed to contribute to the kernel:
> please use a real name here.
> 
> 
> Okay, I've just tried with a dummy server that adds a few bytes to every
> filenames, and it's weirder than I expected... The vfs only checks for
> PATH_MAX length, not NAME_MAX, so e.g. getdents64 will list the long
> file name but then any operation on that file name will fail with
> ENAMETOOLONG...
> I think it'd make sense to be more clear about this in the commit
> message.
> (The patch subject is also pretty bad, by the way, in that "dynamically
> allocate ..." sounds like it adds an extra allocation whereas it just
> reuses the one that exists, so it should be more like "9p: skip
> intermediate directory entry name copy in p9dirent_read()" and explain a
> bit more how the current readdir processing works in the commit message
> e.g. first alloc+copy in p9pdu_vreadf etc)
> 
> 
> Anyway, code-wise:
> - p9_dirent are very temporary and are meant to get freed immediately,
> I think it's perfectly fine to skip the strscpy here and pass the
> pointer directly.
> - I'd like a second opinion on whether it's acceptable that we return a
> file whose lenght is > NAME_MAX to userspace here; we're already calling
> strlen() again in v9fs_dir_readdir_dotl() so we could just skip the
> dir_emit() call here if required.
> 
> 
> Thanks,
> -- 
> Dominique Martinet | Asmadeus
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR