linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kurz <groug@kaod.org>
To: Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: "Mickaël Salaün" <mic@digikod.net>,
	"Dominique Martinet" <asmadeus@codewreck.org>,
	qemu-devel@nongnu.org, "Eric Van Hensbergen" <ericvh@kernel.org>,
	"Latchesar Ionkov" <lucho@ionkov.net>,
	v9fs@lists.linux.dev, "Günther Noack" <gnoack@google.com>,
	linux-security-module@vger.kernel.org, "Jan Kara" <jack@suse.cz>,
	"Amir Goldstein" <amir73il@gmail.com>,
	"Matthew Bobrowski" <repnop@google.com>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	linux-fsdevel@vger.kernel.org, "Tingmao Wang" <m@maowtm.org>
Subject: Re: [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid)
Date: Mon, 13 Oct 2025 11:24:24 +0200	[thread overview]
Message-ID: <20251013112424.6b93659c@bahia> (raw)
In-Reply-To: <3061192.c3ltI2prpg@silver>

On Mon, 29 Sep 2025 15:06:59 +0200
Christian Schoenebeck <linux_oss@crudebyte.com> wrote:

> On Sunday, September 21, 2025 6:24:49 PM CEST Tingmao Wang wrote:
> > On 9/17/25 16:00, Mickaël Salaün wrote:
> [...]
> 
> Hi Greg,
> 

Hi Christian,

> I'd appreciate comments from your side as well, as you are much on longer on
> the QEMU 9p front than me.
> 
> I know you won't have the time to read up on the entire thread so I try to
> summarize: basically this is yet another user-after-unlink issue, this time on
> directories instead of files.
> 

Thread that never landed in my mailbox actually and it is quite
hard to understand the root problem with the content of this
e-mail actually ;-)

> > So I did some quick debugging and realized that I had a wrong
> > understanding of how fids relates to opened files on the host, under QEMU.
> > It turns out that in QEMU's 9p server implementation, a fid does not
> > actually correspond to any opened file descriptors - it merely represents
> > a (string-based) path that QEMU stores internally.  It only opens the
> > actual file if the client actually does an T(l)open, which is in fact
> > separate from acquiring the fid with T(l)walk.  The reason why renaming
> > file/dirs from the client doesn't break those fids is because QEMU will
> > actually fix those paths when a rename request is processed - c.f.
> > v9fs_fix_fid_paths [1].
> 
> Correct, that's based on what the 9p protocols define: a FID does not exactly
> translate to what a file handle is on a local system. Even after acquiring a
> new FID by sending a Twalk request, subsequently client would still need to
> send a Topen for server to actually open that file/directory.
> 
> And yes, QEMU's 9p server "fixes" the path string of a FID if it was moved
> upon client request. If the move happened on host side, outside of server's
> knowledge, then this won't happen ATM and hence it would break your use
> case.
> 
> > It turns out that even if a guest process opens the file with O_PATH, that
> > file descriptor does not cause an actual Topen, and therefore QEMU does
> > not open the file on the host, and later on reopening that fd with another
> > mode (via e.g. open("/proc/self/fd/...", O_RDONLY)) will fail if the file
> > has moved on the host without QEMU's knowledge.  Also, openat will fail if
> > provided with a dir fd that "points" to a moved directory, regardless of
> > whether the fd is opened with O_PATH or not, since path walk in QEMU is
> > completely string-based and does not actually issue openat on the host fs
> > [2].
> 
> I don't think the problem here is the string based walk per se, but rather
> that the string based walk always starts from the export root:
> 
> https://github.com/qemu/qemu/blob/4975b64efb5aa4248cbc3760312bbe08d6e71638/hw/9pfs/9p-local.c#L64
> 
> I guess that's something that could be changed in QEMU such that the walk
> starts from FID's fs point, as the code already uses openat() to walk relative
> to a file descriptor (for security reasons actually), Greg?
> 

Yes this was introduced for security reasons. In a nutshell, the idea is
to *not* follow symlinks in any element of the path being opened. It thus
naturally starts at the export root for which we have an fd.

> That alone would still not fix your use case though: things being moved on
> host side. For this to work, it would require to already have a fd open on
> host for the FID. This could be done by server for each FID as you suggested,
> or it could be done by client by opening the FID.
> 

Can you elaborate on the "things being move on host side" ? With
an example of code that breaks on the client side ?

> Also keep in mind: once the open file descriptor limit on host is exhausted,
> QEMU is forced to close older open file desciptors to keep the QEMU process
> alive. So this might still break what you are trying to achieve there.
> 

Correct.

> Having said that, I wonder whether it'd be simpler for server to track for
> file tree changes (inotify API) and fix the pathes accordingly for host
> side changes as well?
> 

The problem is how to have the guest know about such changes, e.g. in
order to invalidate a stale cache entry. 9P doesn't provide any way for
host->client notification.

> /Christian
> 
> 

Cheers,

-- 
Greg

  reply	other threads:[~2025-10-13  9:24 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-04  0:04 [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid) Tingmao Wang
2025-09-04  0:04 ` [PATCH v2 1/7] fs/9p: Add ability to identify inode by path for .L in uncached mode Tingmao Wang
2025-09-04  0:04 ` [PATCH v2 2/7] fs/9p: add option for path-based inodes Tingmao Wang
2025-09-04  0:04 ` [PATCH v2 3/7] fs/9p: Add ability to identify inode by path for non-.L in uncached mode Tingmao Wang
2025-09-04  0:04 ` [PATCH v2 4/7] fs/9p: .L: Refresh stale inodes on reuse Tingmao Wang
2025-09-04  0:04 ` [PATCH v2 5/7] fs/9p: non-.L: " Tingmao Wang
2025-09-04  0:04 ` [PATCH v2 6/7] fs/9p: update the target's ino_path on rename Tingmao Wang
2025-09-04  0:04 ` [PATCH v2 7/7] docs: fs/9p: Document the "inodeident" option Tingmao Wang
2025-09-14 21:25 ` [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid) Tingmao Wang
2025-09-15 12:53   ` Dominique Martinet
2025-09-15 13:44     ` Tingmao Wang
2025-09-15 23:31       ` Dominique Martinet
2025-09-16 12:44         ` Tingmao Wang
2025-09-16 13:35           ` Dominique Martinet
2025-09-16 14:01             ` Tingmao Wang
2025-09-16 19:22               ` Christian Schoenebeck
2025-09-16 23:59                 ` Tingmao Wang
2025-09-17  9:52                   ` Christian Schoenebeck
2025-09-17 15:00                     ` Mickaël Salaün
2025-09-21 16:24                       ` Tingmao Wang
2025-09-27 18:27                         ` Mickaël Salaün
2025-09-27 22:53                           ` Tingmao Wang
2025-09-29 13:06                         ` Christian Schoenebeck
2025-10-13  9:24                           ` Greg Kurz [this message]
2025-09-16 13:43           ` Christian Schoenebeck
2025-09-15 14:10     ` Christian Schoenebeck
2025-09-17 15:00       ` Mickaël Salaün

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=20251013112424.6b93659c@bahia \
    --to=groug@kaod.org \
    --cc=amir73il@gmail.com \
    --cc=asmadeus@codewreck.org \
    --cc=brauner@kernel.org \
    --cc=ericvh@kernel.org \
    --cc=gnoack@google.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=m@maowtm.org \
    --cc=mic@digikod.net \
    --cc=qemu-devel@nongnu.org \
    --cc=repnop@google.com \
    --cc=v9fs@lists.linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).