mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: hoobnn <853555@gmail.com>
Cc: ericvh@kernel.org, lucho@ionkov.net, v9fs@lists.linux.dev,
	linux_oss@crudebyte.com, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	horms@kernel.org, sripathik@in.ibm.com,
	aneesh.kumar@linux.vnet.ibm.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Matthew Wilcox <willy@infradead.org>,
	David Howells <dhowells@redhat.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>
Subject: Re: [PATCH] 9p: dynamically allocate directory entry names
Date: Mon, 14 Sep 2026 00:12:25 +0900	[thread overview]
Message-ID: <aqa9WXmrgd_P7yxw@codewreck.org> (raw)
In-Reply-To: <20260826064819.52523-1-853555@gmail.com>


+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?


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@gmail.com>

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

      reply	other threads:[~2026-09-13 15:12 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26  6:48 hoobnn
2026-09-13 15:12 ` Dominique Martinet [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqa9WXmrgd_P7yxw@codewreck.org \
    --to=asmadeus@codewreck.org \
    --cc=853555@gmail.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=brauner@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=edumazet@google.com \
    --cc=ericvh@kernel.org \
    --cc=horms@kernel.org \
    --cc=jack@suse.cz \
    --cc=kuba@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sripathik@in.ibm.com \
    --cc=v9fs@lists.linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®