Skip to contentSkip to conversation

fs/9p: Reuse inode based on path (in addition to qid)

PATCH In review Tingmao Wang 5 participants 29 messages Sep 4, 2025 – Sep 13

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover fs/9p: Reuse inode based on path (in addition to qid)

Hi!

This is the second version of this series. The individual commits contains changelogs (most of them are in the first patch), but overall, most significantly cached mode (loose or metadata) is now unchanged, there is no longer a "don't reuse inodes at all" mode, bug fixes, using the right functions, basic rename handling, and new documentation.

Thanks in advance for the review effort :)

v1: https://lore.kernel.org/all/[email protected]/

Background
----------

(This section has basically the same content as the v1 cover letter)

Previously [1], I noticed that when using 9pfs filesystems, the Landlock LSM is blocking access even for files / directories allowed by rules, and that this has something to do with 9pfs creating new inodes despite Landlock holding a reference to the existing one. Because Landlock uses inodes' in-memory state (i_security) to identify allowed fs objects/hierarchies, this causes Landlock to partially break on 9pfs, at least in uncached mode, which is the default:

    # mount -t 9p -o trans=virtio test /mnt
    # env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/readme LL_FS_RW= /sandboxer bash
    Executing the sandboxed command...
    # cat /mnt/readme
    cat: /mnt/readme: Permission denied

This, however, works if somebody is holding onto the dentry (and it also works with cache=loose), as in both cases the inode is reused:

    # tail -f /mnt/readme &
    [1] 196
    # env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/readme LL_FS_RW= /sandboxer bash
    Executing the sandboxed command...
    # cat /mnt/readme
    aa

It also works on directories if one have a shell that cd into the directory. Note that this means only certain usage of Landlock are affected - for example, sandboxing applications that takes a list of files to allow, landlocks itself, then evecve. On the other hand, this does not affect applications that opens a file, then Landlocks itself while keeping the file it needs open.

While the above is a very simple example, this is problematic in real-world use cases if Landlock is used to sandox applications on system that has files mounted via 9pfs, or use 9pfs as the root filesystem. In addition, this also affects fanotify / inotify when using inode mark (for local access):

    root@d8c28a676d72:/# ./fanotify-basic-open /readme & # on virtiofs
    [1] 173
    root@d8c28a676d72:/# cat readme
    aa
    FAN_OPEN: File /readme
    root@d8c28a676d72:/# mount -t 9p -o trans=virtio test /mnt
    root@d8c28a676d72:/# ./fanotify-basic-open /mnt/readme & # on 9pfs
    [2] 176
    root@d8c28a676d72:/# cat /mnt/readme
    aa
    root@d8c28a676d72:/#

Same can be demonstrated with inotifywait. The source code for fanotify-basic-open, adopted from the fanotify man page, is available at
https://fileshare.maowtm.org/9pfs-landlock-fix/20250903/fanotify-basic-open.c [2].

Note that this is not a security bug for Landlock since it can only cause legitimate access to be denied, but might be a problem for fanotify perm (although I do recognize that using perm on individual inodes is already perhaps a bit unreliable?)

It seems that there was an attempt at making 9pfs reuse inodes on uncached mode as well, based on qid.path, however it was reverted [3] due to issues with servers that present duplicate qids, for example on a QEMU host that has multiple filesystems mounted under a single 9pfs export without multidevs=remap, or in the case of other servers that doesn't necessarily support remapping qids ([4] and more). I've done some testing on v6.12-rc4 which has the simplified 9pfs inode code before it was reverted, and found that Landlock works (however, we of course then have the issue demonstrated in [3]).

What this series do
-------------------

(Changes since v1: added more reasoning for the ino_path struct)

With the above in mind, I have a proposal for 9pfs to:
1. Reuse inodes even in uncached mode
2. However, reuse them based on qid.path AND the actual pathname, by doing the appropriate testing in v9fs_test_inode(_dotl)?

The main problem here is how to store the pathname in a sensible way and tie it to the inode. For now I opted with an array of names acquired with take_dentry_name_snapshot, which reuses the same memory as the dcache to store the actual strings, but doesn't tie the lifetime of the dentry with the inode (I thought about holding a reference to the dentry in the v9fs_inode, but it seemed like a wrong approach and would cause dentries to not be evicted/released).

Additional discussions
----------------------

(New section)

From some QEMU documentation I read [5] it seems like there is a plan to resolve these kind of problems in a new version of the protocol, by expanding the qid to include the filesystem identifier of a file on the host, so maybe this can be disabled after a successful protocol version check with the host? For now, inodeident=path will be the default for uncached filesystems, which can be set to 'qid' to instead to reuse based only on server-provided inode numbers.

This patchset currently uses strncmp to compare paths but this might be able to be optimized into a hash comparison first (not done yet). Alternatively the path can be stored more compactly in the form of a single string with / in it (like normal paths). However, we should normally only need to do this comparison for one pair of filenames, as the test is only done if qid.path matches in the first place.

This patchset currently does not support enabling path-based inodes in cached mode. Additional care needs to be taken to ensure we can refresh an inode that potentially has data cached, but since Dominique is happy with cached mode behaving as-is (reusing inodes via qid only), this is not done.

The current implementation will handle client-side renames of a single file (or empty directory) correctly, but server side renames, or renaming a non-empty directory (client or server side), will cause the files being renamed (or files under the renamed directory) to use new inodes (unless they are renamed back). The decision to not update the children of a client-renamed directory is purely to reduce the complexity of this patch, but is in principle possible.

Testing and explanations
------------------------

(New section)

    # mount -t 9p -o ... test /mnt
        with the following options:
        - trans=virtio
        - trans=virtio,inodeident=qid
        - trans=virtio,cache=loose
    # env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/readme LL_FS_RW= /sandboxer bash
    Executing the sandboxed command...
    # cat /mnt/readme
    hi
    ^^ landlock works
    # mount -t 9p -o trans=virtio test /mnt
    # mkdir /mnt/dir
    # mv /mnt/readme /mnt/dir/readme
    # env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/dir/readme LL_FS_RW= /sandboxer bash
    Executing the sandboxed command...
    # cat /mnt/dir/readme
    hi
    ^^ landlock works

# # another terminal in guest: mv /mnt/dir/readme /mnt/dir/readme.2 # cat /mnt/dir/readme.2
hi
^^ ino_path is carried with renames

# # host: mv 9pfs/dir/readme.2 9pfs/dir/readme
# cat /mnt/dir/readme.2
cat: /mnt/dir/readme.2: No such file or directory
# cat /mnt/dir/readme
cat: /mnt/dir/readme: Permission denied ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ we can't track renames on the server side # # host: mv 9pfs/dir/readme 9pfs/dir/readme.2
# cat /mnt/dir/readme.2
hi
^^ once the file is back at its original place it works as expected.

# # another terminal in guest: mv /mnt/dir/readme.2 /mnt/dir/readme
# cat /mnt/dir/readme
hi
^^ we can track renames of the file directly...
# # another terminal in guest: mv /mnt/dir /mnt/dir.2
# cat /mnt/dir.2/readme
cat: /mnt/dir.2/readme: Permission denied
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ but not renames of the parent directory, even if done client-side

# # another terminal in guest: mv /mnt/dir.2 /mnt/dir
# cat /mnt/dir/readme
hi
^^ works once it's back
# # another terminal in guest: mv /mnt/dir /mnt/dir.2 && mkdir /mnt/dir && echo hi2 > /mnt/dir/readme # cat /mnt/dir/readme
cat: /mnt/dir/readme: Permission denied
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a different file uses a different inode even if same path

# # another terminal in guest: mv /mnt/dir.2/readme /mnt/dir/readme
# cat /mnt/dir/readme
hi
# # host: rm 9pfs/dir/readme && echo hi3 > 9pfs/dir/readme
# cat /mnt/dir/readme
cat: /mnt/dir/readme: Permission denied
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a different file (identified by server-side qid changes) uses different inode

fanotify also works, as tested with the program attached at the end.

In addition, I ran xfstests on a uncached 9pfs mount, and while there are some test failures, it is the same set of failures as on the current mainline. Test logs at https://fileshare.maowtm.org/9pfs-landlock-fix/20250903/index.html

Tested also with Mickaël's new v9fs landlock tests [6] (unmerged yet):

# RUN layout3_fs.v9fs.tag_inode_dir_parent ...
# OK layout3_fs.v9fs.tag_inode_dir_parent
ok 129 layout3_fs.v9fs.tag_inode_dir_parent
# RUN layout3_fs.v9fs.tag_inode_dir_mnt ...
# OK layout3_fs.v9fs.tag_inode_dir_mnt
ok 130 layout3_fs.v9fs.tag_inode_dir_mnt
# RUN layout3_fs.v9fs.tag_inode_dir_child ...
# OK layout3_fs.v9fs.tag_inode_dir_child
ok 131 layout3_fs.v9fs.tag_inode_dir_child
# RUN layout3_fs.v9fs.tag_inode_file ...
# OK layout3_fs.v9fs.tag_inode_file
ok 132 layout3_fs.v9fs.tag_inode_file
# RUN layout3_fs.v9fs.release_inodes ...
# OK layout3_fs.v9fs.release_inodes
ok 133 layout3_fs.v9fs.release_inodes

This patch series was based on, and mostly tested on v6.17-rc1 + [7]

Kind regards,
Tingmao

[1]: https://github.com/landlock-lsm/linux/issues/45
[2]: https://fileshare.maowtm.org/9pfs-landlock-fix/20250903/fanotify-basic-open.c
[3]: https://lore.kernel.org/all/[email protected]/
[4]: https://lore.kernel.org/all/20240923100508.GA32066@willie-the-truck/
[5]: https://wiki.qemu.org/Documentation/9p#Protocol_Plans
[6]: https://lore.kernel.org/all/[email protected]/
[7]: https://lore.kernel.org/all/[email protected]/

Tingmao Wang (7):
fs/9p: Add ability to identify inode by path for .L in uncached mode fs/9p: add option for path-based inodes
fs/9p: Add ability to identify inode by path for non-.L in uncached
mode
fs/9p: .L: Refresh stale inodes on reuse
fs/9p: non-.L: Refresh stale inodes on reuse
fs/9p: update the target's ino_path on rename
docs: fs/9p: Document the "inodeident" option

 Documentation/filesystems/9p.rst |  42 +++++++
 fs/9p/Makefile                   |   3 +-
 fs/9p/ino_path.c                 | 111 ++++++++++++++++++
 fs/9p/v9fs.c                     |  59 +++++++++-
 fs/9p/v9fs.h                     |  87 ++++++++++----
 fs/9p/vfs_inode.c                | 195 ++++++++++++++++++++++++++-----
 fs/9p/vfs_inode_dotl.c           | 171 +++++++++++++++++++++++----
 fs/9p/vfs_super.c                |  13 ++-
 8 files changed, 611 insertions(+), 70 deletions(-)
 create mode 100644 fs/9p/ino_path.c

base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
prerequisite-patch-id: 3dae487a4b3d676de7c20b269553e3e2176b1e36
prerequisite-patch-id: 93ab54c52a41fa44b8d0baf55df949d0ad27e99a
prerequisite-patch-id: 5f558bf969e6eaa3d011c98de0806ca8ad369efe

Signature · 1 line
2.51.0
#1 lore raw

PATCH v2 1/7 fs/9p: Add ability to identify inode by path for .L in uncached mode

The intention of this patch is to allow features like Landlock and fanotify (inode mark mode) to work on uncached 9pfs. These features rely on holding a specific inode and handling further access to the same file (as identified by that inode), however, currently in uncached mode, we always get a new inode on each access, due to concerns regarding server side inode number collision.

On cached mode (either CACHE_LOOSE or CACHE_META), inode is already reused only by looking at the qid (server-side inode number). Since introducing this additional check would regress hard links (as they will have different path, and thus the two ends of a hard link won't be the same inode anymore under this approach), this won't be done for cached mode.

Currently this patch doesn't actually have any effect - the next commit will introduce a config option to control inodeident=path enablement and default it to on for uncached mode.

---
Changes since v1:
- Assume inodeident=path will not be set in cached mode.

- Fix various issues (rcu usage etc) in ino_path.c with feedback from Al Viro and Mickaël Salaün

- Use d_same_name instead of strncmp

- Instead of changing v9fs_test_new_inode_dotl to add the path check (thus hijacking the meaning of "new" to actually mean "uncached"), we add the path check (conditional on the right flags in v9ses) to the cached test function (v9fs_test_inode_dotl) and use that function for both cached and uncached mode, by adding additional conditionals within in for the version/generation check. The v9fs_test_new_inode_dotl function is thus used only for mknod, mkdir and atomic_open in the "need to create" case.

- Instead of never reusing inode if path-based ident is not enabled, we always reuse in uncached mode, but if path-based ident is not enabled, we don't check the path. This makes the code easier to reason about, and gets rid of the complexity of having to support two quite different mode of operation (re-using and not re-using inodes).

- Fix crash due to uninitialized v9inode->path when inode is allocated then immediately deallocated in iget5_locked as a result of two iget racing with each other to insert the inode. Spotted via xfstests.

- Don't allocate v9fs_ino_path within v9fs_set_inode_dotl, as iget5_locked specifies that it can't sleep. Doing so means that we need to handle a special case of inode being created and hashed into the inode list, and thus may be tested by another iget5_locked call, but its v9inode->path has not been populated yet. This is resolved via waiting for iget5_locked to return before checking the path. This edge case was spotted via xfstests.

 fs/9p/Makefile         |   3 +-
 fs/9p/ino_path.c       | 110 ++++++++++++++++++++++++++++++
 fs/9p/v9fs.h           |  74 +++++++++++++++-----
 fs/9p/vfs_inode.c      |  16 +++--
 fs/9p/vfs_inode_dotl.c | 149 +++++++++++++++++++++++++++++++++++------
 fs/9p/vfs_super.c      |  13 +++-
 6 files changed, 321 insertions(+), 44 deletions(-)
 create mode 100644 fs/9p/ino_path.c
fs/9p/Makefile +2 -1
index e7800a5c7395..38c3ceb26274 100644
@@ -11,7 +11,8 @@ obj-$(CONFIG_9P_FS) := 9p.o
 	vfs_dentry.o \
 	v9fs.o \
 	fid.o  \
-	xattr.o
+	xattr.o \
+	ino_path.o
 
 9p-$(CONFIG_9P_FSCACHE) += cache.o
 9p-$(CONFIG_9P_FS_POSIX_ACL) += acl.o
fs/9p/ino_path.c new file +110 -0

110 lines not on this page · raw message

fs/9p/v9fs.h +56 -18

134 lines not on this page · raw message

fs/9p/vfs_inode.c +11 -5
index caff65d8b2bb..5e56c13da733 100644
@@ -232,6 +232,7 @@ struct inode *v9fs_alloc_inode(struct super_block *sb)
 	if (!v9inode)
 		return NULL;
 	v9inode->cache_validity = 0;
+	v9inode->path = NULL;
 	mutex_init(&v9inode->v_mutex);
 	return &v9inode->netfs.inode;
 }
@@ -243,6 +244,7 @@ struct inode *v9fs_alloc_inode(struct super_block *sb)
 
 void v9fs_free_inode(struct inode *inode)
 {
+	free_ino_path(V9FS_I(inode)->path);
 	kmem_cache_free(v9fs_inode_cache, V9FS_I(inode));
 }
 
@@ -607,15 +609,17 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir,
 			goto error;
 		}
 		/*
-		 * instantiate inode and assign the unopened fid to the dentry
+		 * Instantiate inode.  On .L fs, pass in dentry for inodeident=path.
 		 */
-		inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+		inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb,
+			v9fs_proto_dotl(v9ses) ? dentry : NULL);
 		if (IS_ERR(inode)) {
 			err = PTR_ERR(inode);
 			p9_debug(P9_DEBUG_VFS,
 				   "inode creation failed %d\n", err);
 			goto error;
 		}
+		/* Assign the unopened fid to the dentry */
 		v9fs_fid_add(dentry, &fid);
 		d_instantiate(dentry, inode);
 	}
@@ -732,14 +736,16 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
 	name = dentry->d_name.name;
 	fid = p9_client_walk(dfid, 1, &name, 1);
 	p9_fid_put(dfid);
+
 	if (fid == ERR_PTR(-ENOENT))
 		inode = NULL;
 	else if (IS_ERR(fid))
 		inode = ERR_CAST(fid);
-	else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
-		inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
+	else if (v9ses->cache & (CACHE_META | CACHE_LOOSE))
+		/* Cached fs will not use inode path identification */
+		inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, NULL);
 	else
-		inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+		inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, dentry);
 	/*
 	 * If we had a rename on the server and a parallel lookup
 	 * for the new name, then make sure we instantiate with
fs/9p/vfs_inode_dotl.c +130 -19

236 lines not on this page · raw message

fs/9p/vfs_super.c +12 -1
index 795c6388744c..bb9e66f4631e 100644
@@ -141,7 +141,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
 		sb->s_d_flags |= DCACHE_DONTCACHE;
 	}
 
-	inode = v9fs_get_new_inode_from_fid(v9ses, fid, sb);
+	inode = v9fs_get_new_inode_from_fid(v9ses, fid, sb, NULL);
 	if (IS_ERR(inode)) {
 		retval = PTR_ERR(inode);
 		goto release_sb;
@@ -153,6 +153,17 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
 		goto release_sb;
 	}
 	sb->s_root = root;
+
+	if (v9fs_inode_ident_path(v9ses)) {
+		/*
+		 * This down_read is probably not necessary, just to satisfy
+		 * lockdep_assert
+		 */
+		down_read(&v9ses->rename_sem);
+		V9FS_I(inode)->path = make_ino_path(root);
+		up_read(&v9ses->rename_sem);
+	}
+
 	retval = v9fs_get_acl(inode, fid);
 	if (retval)
 		goto release_sb;
Signature · 1 line
2.51.0
#2 lore raw

PATCH v2 2/7 fs/9p: add option for path-based inodes

In reply to Tingmao Wang

By this point we have two ways to test for inode reuse - qid and qid+path. By default, uncached mode uses qid+path and cached mode uses qid (and in fact does not support qid+path). This patch adds the option to control the behaviour for uncached mode.

In a future version, if we can negotiate with the server and be sure that it won't give us duplicate qid.path, the default for those cases can be qid-based.

Cc "Mickaël Salaün" <[email protected]>
Cc "Günther Noack" <[email protected]>

---
Changes since v1:
- Removed inodeident=none and instead supports inodeident=qid. This means that there is no longer an option to not re-use inodes at all.

- No longer supports inodeident=path on cached mode, checks added at option init time.

- Added explicit bits for both V9FS_INODE_IDENT_PATH and V9FS_INODE_IDENT_QID, in order to set a default based on cache bits when neither are set explicitly by the user.

 fs/9p/v9fs.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/9p/v9fs.h |  3 +++
 2 files changed, 61 insertions(+), 1 deletion(-)
fs/9p/v9fs.c +58 -1
index 77e9c4387c1d..f87d6680b85a 100644
@@ -36,7 +36,7 @@ enum {
 	/* Options that take integer arguments */
 	Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid,
 	/* String options */
-	Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag,
+	Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag, Opt_inodeident,
 	/* Options that take no arguments */
 	Opt_nodevmap, Opt_noxattr, Opt_directio, Opt_ignoreqv,
 	/* Access options */
@@ -63,6 +63,7 @@ static const match_table_t tokens = {
 	{Opt_access, "access=%s"},
 	{Opt_posixacl, "posixacl"},
 	{Opt_locktimeout, "locktimeout=%u"},
+	{Opt_inodeident, "inodeident=%s"},
 	{Opt_err, NULL}
 };
 
@@ -149,6 +150,21 @@ int v9fs_show_options(struct seq_file *m, struct dentry *root)
 	if (v9ses->flags & V9FS_NO_XATTR)
 		seq_puts(m, ",noxattr");
 
+	switch (v9ses->flags & V9FS_INODE_IDENT_MASK) {
+	case V9FS_INODE_IDENT_QID:
+		seq_puts(m, ",inodeident=qid");
+		break;
+	case V9FS_INODE_IDENT_PATH:
+		seq_puts(m, ",inodeident=path");
+		break;
+	default:
+		/*
+		 * Unspecified, will be set later in v9fs_session_init depending on
+		 * cache setting
+		 */
+		break;
+	}
+
 	return p9_show_client_options(m, v9ses->clnt);
 }
 
@@ -369,6 +385,26 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
 			v9ses->session_lock_timeout = (long)option * HZ;
 			break;
 
+		case Opt_inodeident:
+			s = match_strdup(&args[0]);
+			if (!s) {
+				ret = -ENOMEM;
+				p9_debug(P9_DEBUG_ERROR,
+					 "problem allocating copy of inodeident arg\n");
+				goto free_and_return;
+			}
+			v9ses->flags &= ~V9FS_INODE_IDENT_MASK;
+			if (strcmp(s, "qid") == 0) {
+				v9ses->flags |= V9FS_INODE_IDENT_QID;
+			} else if (strcmp(s, "path") == 0) {
+				v9ses->flags |= V9FS_INODE_IDENT_PATH;
+			} else {
+				ret = -EINVAL;
+				p9_debug(P9_DEBUG_ERROR, "Unknown inodeident argument %s\n", s);
+			}
+			kfree(s);
+			break;
+
 		default:
 			continue;
 		}
@@ -393,6 +429,7 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
 {
 	struct p9_fid *fid;
 	int rc = -ENOMEM;
+	bool cached;
 
 	v9ses->uname = kstrdup(V9FS_DEFUSER, GFP_KERNEL);
 	if (!v9ses->uname)
@@ -427,6 +464,26 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
 	if (rc < 0)
 		goto err_clnt;
 
+	cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
+
+	if (cached && v9ses->flags & V9FS_INODE_IDENT_PATH) {
+		rc = -EINVAL;
+		p9_debug(P9_DEBUG_ERROR,
+			 "inodeident=path not supported in cached mode\n");
+		goto err_clnt;
+	}
+
+	if (!(v9ses->flags & V9FS_INODE_IDENT_MASK)) {
+		/* Unspecified - use default */
+		if (cached) {
+			/* which is qid in cached mode (path not supported) */
+			v9ses->flags |= V9FS_INODE_IDENT_QID;
+		} else {
+			/* ...or path in uncached mode */
+			v9ses->flags |= V9FS_INODE_IDENT_PATH;
+		}
+	}
+
 	v9ses->maxdata = v9ses->clnt->msize - P9_IOHDRSZ;
 
 	if (!v9fs_proto_dotl(v9ses) &&
fs/9p/v9fs.h +3 -0
index 134b55a605be..b4e738c1bba5 100644
@@ -43,8 +43,11 @@ enum p9_session_flags {
 	V9FS_DIRECT_IO        = 0x100,
 	V9FS_SYNC             = 0x200,
 	V9FS_INODE_IDENT_PATH = 0x400,
+	V9FS_INODE_IDENT_QID  = 0x800,
 };
 
+#define V9FS_INODE_IDENT_MASK (V9FS_INODE_IDENT_PATH | V9FS_INODE_IDENT_QID)
+
 /**
  * enum p9_cache_shortcuts - human readable cache preferences
  * @CACHE_SC_NONE: disable all caches
Signature · 1 line
2.51.0
#3 lore raw

PATCH v2 3/7 fs/9p: Add ability to identify inode by path for non-.L in uncached mode

In reply to Tingmao Wang

This replicates the earlier .L patch for non-.L, and removing some previously inserted conditionals in shared code.

---
Changes since v1:
- Reflect v2 changes to the .L counterpart of this.

 fs/9p/v9fs.h      |   7 ++-
 fs/9p/vfs_inode.c | 150 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 130 insertions(+), 27 deletions(-)
fs/9p/v9fs.h +4 -3
index b4e738c1bba5..bacd0052e22c 100644
@@ -202,7 +202,8 @@ extern int v9fs_vfs_rename(struct mnt_idmap *idmap,
 			   unsigned int flags);
 extern struct inode *v9fs_inode_from_fid(struct v9fs_session_info *v9ses,
 					 struct p9_fid *fid,
-					 struct super_block *sb, int new);
+					 struct super_block *sb,
+					 struct dentry *dentry, int new);
 extern const struct inode_operations v9fs_dir_inode_operations_dotl;
 extern const struct inode_operations v9fs_file_inode_operations_dotl;
 extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
@@ -267,7 +268,7 @@ v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
 	if (v9fs_proto_dotl(v9ses))
 		return v9fs_inode_from_fid_dotl(v9ses, fid, sb, dentry, 0);
 	else
-		return v9fs_inode_from_fid(v9ses, fid, sb, 0);
+		return v9fs_inode_from_fid(v9ses, fid, sb, dentry, 0);
 }
 
 /**
@@ -291,7 +292,7 @@ v9fs_get_new_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
 	if (v9fs_proto_dotl(v9ses))
 		return v9fs_inode_from_fid_dotl(v9ses, fid, sb, dentry, 1);
 	else
-		return v9fs_inode_from_fid(v9ses, fid, sb, 1);
+		return v9fs_inode_from_fid(v9ses, fid, sb, dentry, 1);
 }
 
 #endif
fs/9p/vfs_inode.c +126 -24

232 lines not on this page · raw message

Signature · 1 line
2.51.0
#4 lore raw

PATCH v2 4/7 fs/9p: .L: Refresh stale inodes on reuse

In reply to Tingmao Wang

This uses the stat struct we already got as part of lookup process to refresh the inode "for free".

Only for uncached mode for now. We will need to revisit this for cached mode once we sort out reusing an old inode with changed qid.version. (Currently this is not done in this series, which would be fine unless server side change happens, which is not supposed to happen in cached mode anyway)

Note that v9fs_test_inode_dotl already makes sure we don't reuse inodes of the wrong type or different qid.

---
Changes since v1:
- Check cache bits instead of using new - uncached mode now also have new=0.

 fs/9p/vfs_inode_dotl.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
fs/9p/vfs_inode_dotl.c +15 -1
index 86adaf5bcc0e..d008e82256ac 100644
@@ -164,6 +164,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
 		.dentry = dentry,
 		.need_double_check = false,
 	};
+	bool cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
 
 	if (new)
 		test = v9fs_test_new_inode_dotl;
@@ -203,8 +204,21 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
 
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
-	if (!(inode->i_state & I_NEW))
+	if (!(inode->i_state & I_NEW)) {
+		/*
+		 * If we're returning an existing inode, we might as well refresh
+		 * it with the metadata we just got.  Refreshing the i_size also
+		 * prevents read errors.
+		 *
+		 * We only do this for uncached mode, since in cached move, any
+		 * change on the inode will bump qid.version, which will result in
+		 * us getting a new inode in the first place.  If we got an old
+		 * inode, let's not touch it for now.
+		 */
+		if (!cached)
+			v9fs_stat2inode_dotl(st, inode, 0);
 		return inode;
+	}
 	/*
 	 * initialize the inode with the stat info
 	 * FIXME!! we may need support for stale inodes
Signature · 1 line
2.51.0
#5 lore raw

PATCH v2 5/7 fs/9p: non-.L: Refresh stale inodes on reuse

In reply to Tingmao Wang

This replicates the previous .L commit for non-.L

---
Changes since v1:
- Check cache bits instead of using new - uncached mode now also have new=0.

 fs/9p/vfs_inode.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
fs/9p/vfs_inode.c +6 -1
index 606760f966fd..4b4712eafe4d 100644
@@ -473,6 +473,7 @@ static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
 		.dentry = dentry,
 		.need_double_check = false,
 	};
+	bool cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
 
 	if (new)
 		test = v9fs_test_new_inode;
@@ -512,8 +513,12 @@ static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
 
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
-	if (!(inode->i_state & I_NEW))
+	if (!(inode->i_state & I_NEW)) {
+		/* See explanation in v9fs_qid_iget_dotl */
+		if (!cached)
+			v9fs_stat2inode(st, inode, sb, 0);
 		return inode;
+	}
 	/*
 	 * initialize the inode with the stat info
 	 * FIXME!! we may need support for stale inodes
Signature · 1 line
2.51.0
#6 lore raw

PATCH v2 6/7 fs/9p: update the target's ino_path on rename

In reply to Tingmao Wang

This makes it possible for the inode to "move along" to the new location when a file under a inodeident=path 9pfs is moved, and it will be reused on next access to the new location.

Modifying the ino_path of children when renaming a directory is currently not handled. Renaming non-empty directories still work, but the children won't have their the inodes be reused after renaming.

Inodes will also not be reused on server-side rename, since there is no way for us to know about it. From our perspective this is indistinguishable from a new file being created in the destination that just happened to have the same qid, and the original file being deleted.

Cc "Mickaël Salaün" <[email protected]>
Cc "Günther Noack" <[email protected]>

---
New patch in v2

 fs/9p/ino_path.c       |  3 ++-
 fs/9p/v9fs.h           |  3 +++
 fs/9p/vfs_inode.c      | 30 ++++++++++++++++++++++++++++++
 fs/9p/vfs_inode_dotl.c |  6 ++++++
 4 files changed, 41 insertions(+), 1 deletion(-)
fs/9p/ino_path.c +2 -1
index a03145e08a9d..ee4752b9f796 100644
@@ -27,7 +27,8 @@ struct v9fs_ino_path *make_ino_path(struct dentry *dentry)
 	struct dentry *curr = dentry;
 	ssize_t i;
 
-	lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
+	/* Either read or write lock held is ok */
+	lockdep_assert_held(&v9fs_dentry2v9ses(dentry)->rename_sem);
 	might_sleep(); /* Allocation below might block */
 
 	rcu_read_lock();
fs/9p/v9fs.h +3 -0
index bacd0052e22c..c441fa8e757b 100644
@@ -157,6 +157,9 @@ struct v9fs_inode {
 	/*
 	 * Stores the path of the file this inode is for, only for filesystems
 	 * with inode_ident=path.  Lifetime is the same as this inode.
+	 * Read/write to this pointer should be under the target v9fs's
+	 * rename_sem to protect against races (except when initializing or
+	 * freeing an inode, at which point nobody else has reference to us)
 	 */
 	struct v9fs_ino_path *path;
 };
fs/9p/vfs_inode.c +30 -0
index 4b4712eafe4d..68a1837ff3dc 100644
@@ -532,6 +532,12 @@ static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
 
 	v9inode = V9FS_I(inode);
 	if (dentry) {
+		/*
+		 * In order to make_ino_path, we need at least a read lock on the
+		 * rename_sem.  Since we re initializing a new inode, there is no
+		 * risk of races with another task trying to write to
+		 * v9inode->path, so we do not need an actual down_write.
+		 */
 		down_read(&v9ses->rename_sem);
 		v9inode->path = make_ino_path(dentry);
 		up_read(&v9ses->rename_sem);
@@ -983,18 +989,21 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 {
 	int retval;
 	struct inode *old_inode;
+	struct v9fs_inode *old_v9inode;
 	struct inode *new_inode;
 	struct v9fs_session_info *v9ses;
 	struct p9_fid *oldfid = NULL, *dfid = NULL;
 	struct p9_fid *olddirfid = NULL;
 	struct p9_fid *newdirfid = NULL;
 	struct p9_wstat wstat;
+	struct v9fs_ino_path *new_ino_path = NULL;
 
 	if (flags)
 		return -EINVAL;
 
 	p9_debug(P9_DEBUG_VFS, "\n");
 	old_inode = d_inode(old_dentry);
+	old_v9inode = V9FS_I(old_inode);
 	new_inode = d_inode(new_dentry);
 	v9ses = v9fs_inode2v9ses(old_inode);
 	oldfid = v9fs_fid_lookup(old_dentry);
@@ -1022,6 +1031,17 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 	}
 
 	down_write(&v9ses->rename_sem);
+	if (v9fs_inode_ident_path(v9ses)) {
+		/*
+		 * Try to allocate this first, and don't actually do rename if
+		 * allocation fails.
+		 */
+		new_ino_path = make_ino_path(new_dentry);
+		if (!new_ino_path) {
+			retval = -ENOMEM;
+			goto error_locked;
+		}
+	}
 	if (v9fs_proto_dotl(v9ses)) {
 		retval = p9_client_renameat(olddirfid, old_dentry->d_name.name,
 					    newdirfid, new_dentry->d_name.name);
@@ -1061,6 +1081,15 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 		v9fs_invalidate_inode_attr(old_inode);
 		v9fs_invalidate_inode_attr(old_dir);
 		v9fs_invalidate_inode_attr(new_dir);
+		if (v9fs_inode_ident_path(v9ses)) {
+			/*
+			 * We currently have rename_sem write lock, which protects all
+			 * v9inode->path in this fs.
+			 */
+			free_ino_path(old_v9inode->path);
+			old_v9inode->path = new_ino_path;
+			new_ino_path = NULL;
+		}
 
 		/* successful rename */
 		d_move(old_dentry, new_dentry);
@@ -1068,6 +1097,7 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 	up_write(&v9ses->rename_sem);
 
 error:
+	free_ino_path(new_ino_path);
 	p9_fid_put(newdirfid);
 	p9_fid_put(olddirfid);
 	p9_fid_put(oldfid);
fs/9p/vfs_inode_dotl.c +6 -0
index d008e82256ac..a3f70dd422fb 100644
@@ -232,6 +232,12 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
 
 	v9inode = V9FS_I(inode);
 	if (dentry) {
+		/*
+		 * In order to make_ino_path, we need at least a read lock on the
+		 * rename_sem.  Since we re initializing a new inode, there is no
+		 * risk of races with another task trying to write to
+		 * v9inode->path, so we do not need an actual down_write.
+		 */
 		down_read(&v9ses->rename_sem);
 		v9inode->path = make_ino_path(dentry);
 		up_read(&v9ses->rename_sem);
Signature · 1 line
2.51.0
#7 lore raw

PATCH v2 7/7 docs: fs/9p: Document the "inodeident" option

In reply to Tingmao Wang

Add a row for this option in the Options table.

---
New patch in v2

 Documentation/filesystems/9p.rst | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
Documentation/filesystems/9p.rst +42 -0
index be3504ca034a..8b570a7ae698 100644
@@ -238,6 +238,48 @@ Options
   cachetag	cache tag to use the specified persistent cache.
 		cache tags for existing cache sessions can be listed at
 		/sys/fs/9p/caches. (applies only to cache=fscache)
+
+  inodeident	this setting controls how inodes work on this filesystem.
+		More specifically, how they are "reused".  This is most
+		relevant when used with features like Landlock and
+		fanotify (in inode mark mode).  These features rely on
+		holding a specific inode and identifying further access to
+		the same file (as identified by that inode).
+
+		There are 2 possible values:
+			qid
+				This is the default and the only possible
+				option if loose or metadata cache is
+				enabled.  In this mode, 9pfs assumes that
+				the server will not present different
+				files with the same inode number, and will
+				use the presented inode number to lookup
+				inodes.  For QEMU users, this can be
+				ensured by setting multidevs=remap.  If
+				the server does present inode number
+				collisions, this may lead to unpredictable
+				behaviour when both files are accessed.
+			path
+				This is the default if neither loose nor
+				metadata cache bits are enabled.  This
+				option causes 9pfs to internally track the
+				file path that an inode originated from,
+				and will only use an existing inode
+				(instead of allocating a new one) if the
+				path matches, even if the file's inode
+				number matches that of an existing inode.
+
+		.. note::
+			For inodeident=path, when a directory is renamed
+			or moved, inodeident=path mode currently does not
+			update its children's inodes to point to the new
+			path, and thus further access to them via the new
+			location will use newly allocated inodes, and
+			existing inode marks placed by Landlock and
+			fanotify on them will no longer work.
+
+			The inode path for the target being renamed itself
+			(not its children) *is* updated, however.
   ============= ===============================================================
 
 Behavior
Signature · 1 line
2.51.0
#8 lore raw
In reply to Tingmao Wang

Hi Dominique and others,

I had a chat with Mickaël earlier this week and some discussion following that, and we thought of a potential alternative to what I was proposing here that might work for Landlock: using the inode number (or more correctly, qid.path) directly as the keys for Landlock rules when accessing 9p files. I'm not sure how sound this is from the perspective of 9pfs (there are pros and caveats), and I would like to gather some thoughts on this idea.

Technically a 9pfs server is not supposed to return colliding qid.paths for different files. In fact, according to [1], the qid must not be the same even for files which are deleted then recreated using the same name (whereas for e.g. ext4, inode number is reused if a file is deleted and recreated, possibly with a different name, in the same directory). However, this is in practice not the case for many actual 9pfs server implementations (thus the reason for this patch series in the first place).

This is a bad problem for the 9pfs client in Linux as it can lead to data corruption if the wrong inode is used, but for Landlock, the only effect of this is allowing access to more files then the sandboxing application intended (and only in the presence of an "erroneous" 9pfs server). Any other alternative, including this patch series, has the opposite risk - files that should be allowed might be denied (even if the server implementation is fully correct in terms of no reusing of qids). In particular, this patch cannot correctly handle server-side renames of an allowed file, or rename of a directory with children in it from the client (although this might be solved, with the expense of adding more complicated code in the rename path to rewrite all the struct ino_paths).

In discussion with Mickaël he thought that it would be acceptable for Landlock to assume that the server is well-behaved, and Landlock could specialize for 9pfs to allow access if the qid matches what's previously seen when creating the Landlock ruleset (by using the qid as the key of the rule, instead of a pointer to the inode).

There are, however, several immediate issues with this approach:

1. The qid is 9pfs internal data, and we may need extra API for 9pfs to expose this to Landlock. On 64bit, this is easy as it's just the inode number (offset by 2), which we can already get from the struct inode. But perhaps on 32bit we need a way to expose the full 64bit server-sent qid to Landlock (or other kernel subsystems), if we're going to do this.

2. Even though qids are supposed to be unique across the lifetime of a filesystem (including deleted files), this is not the case even for QEMU in multidevs=remap mode, when running on ext4, as tested on QEMU 10.1.0. And thus in practice a Landlock ruleset would need to hold a reference to the file to keep it open, so that the server will not re-use the qid for other files (having a reference to the struct inode alone doesn't seem to do that).

Unfortunately, holding a dentry in Landlock prevents the filesystem from being unmounted (causes WARNs), with no (proper) chance for Landlock to release those dentries. We might do it in security_sb_umount, but then at that point it is not guaranteed that the unmount will happen - perhaps we would need a new security_ hooks in the umount path?

Alternatively, I think if we could somehow tell 9pfs to keep a fid open (until either the Landlock domain is closed, or the filesystem is unmounted), it could also work.

I'm not sure what's the best way to do this, it seems like unless we can get a new pre_umount / pre_sb_delete hook in which we can free dentries, 9pfs would need to expose some new API, or alternatively, in uncached mode, have the v9fs inode itself hold a (strong) reference to the fid, so that if Landlock has a reference to the inode, the file is kept open server-side.

The advantage of doing this is that, for a server with reasonable behaviour, Landlock users would not get incorrect denials (i.e. things "just work"), while still maintaining security if the 9p server is "reasonable" (in particular, an application sandboxed under Landlock would not get access to unrelated files if it does not have a way to somehow get those files to be recreated with an allowed inode number), whereas the current patch has the problem with server side renames and directory renames (server or client side), and also can't deal with hard links.

I'm not sure how attractive this solution is to various people here - Mickaël is happy with special-casing 9pfs in Landlock, and in fact he suggested this idea in the first place, but I think this has the potential to be quite complicated (but technically more correct). It would also only work for Landlock, and if e.g. fsnotify wants to have the same behaviour, that would need its own changes too.

Apologies for the long-winded explanation, any thoughts on this?

[1]: https://ericvh.github.io/9p-rfc/rfc9p2000.html#msgs
"If a file is deleted and recreated with the same name in the same directory, the old and new path components of the qids should be different."

---

Note: Even with the above, there's another potential problem - QEMU does not, for some reason (I've not really investigated this very deeply, but it's observation from /proc/.../fd), keep a directory open when the guest has a fid to it. This means that if a directory is deleted while we have an active Landlock rule on it, a new file or directory may get the same qid. (However, at least this still correctly handles directory renames, and the only effect is Landlock allowing more files than intended in the presence of a buggy server.)

(The Hyper-V 9p server, used by WSL, seems to have the same problem, and a bit worse since even client-side renames breaks opened dir fds on the WSL-to-Windows 9pfs (/mnt/c/...))

(Another challenge is that Landlock would have to know when a file is on a 9pfs in uncached mode - we probably don't need this behaviour for cached mode filesystems, as we assume no server changes in that case and the inode is reused already. We can certainly determine the FS of a file, but not sure about specific 9pfs cache options)

#9 lore raw

Hi Tingmao,

thanks for pushing this forward, I still have very little time so haven't been able to review this properly

Tingmao Wang wrote on Sun, Sep 14, 2025 at 10:25:02PM +0100:

Quoted 7 lines

I had a chat with Mickaël earlier this week and some discussion following that, and we thought of a potential alternative to what I was proposing here that might work for Landlock: using the inode number (or more correctly, qid.path) directly as the keys for Landlock rules when accessing 9p files. I'm not sure how sound this is from the perspective of 9pfs (there are pros and caveats), and I would like to gather some thoughts on this idea.

I'm honestly split on this:
- I really don't like tracking the full path of each file around; there are various corner cases with files being removed (possibly server side!) or hard links; and it's potentially slowing down all operations a bit...
- OTOH as you pointed out qid isn't as reliable, and having file paths around opens the way to rebuilding fids on reconnect for non-local servers, which could potentially be interesting (not that I ever see myself having time to work on this as I no longer have any stake there, I just know that would have interested my previous employer when they were still using 9p/rdma...)

Quoted 5 lines

In discussion with Mickaël he thought that it would be acceptable for Landlock to assume that the server is well-behaved, and Landlock could specialize for 9pfs to allow access if the qid matches what's previously seen when creating the Landlock ruleset (by using the qid as the key of the rule, instead of a pointer to the inode).

I'm not familiar at all with landlock so forgive this question: what is this key about exactly?
When a program loads a ruleset, paths referred in that ruleset are looked up by the kernel and the inodes involved kept around in some hash table for lookup on further accesses?

I'm fuzzy on the details but I don't see how inode pointers would be stable for other filesystems as well, what prevents
e.g. vm.drop_caches=3 to drop these inodes on ext4?

In general I'd see the file handle (as exposed to userspace by name_to_handle_at) as a stable key, that works for all filesystems supporting fhandles (... so, not 9p, right... But in general it's something like inode number + generation, and we could expose that as handle and "just" return ENOTSUP on open_by_handle_at if that helps)

Although looking at the patches what 9p seems to need isn't a new stable handle, but "just" not allocating new inodes in iget5... This was attempted in 724a08450f74 ("fs/9p: simplify iget to remove unnecessary paths"), but later reverted in be2ca3825372 ("Revert "fs/9p:
simplify iget to remove unnecessary paths"") because it broke too many users, but if you're comfortable with a new mount option for the lookup by path I think we could make a new option saying "yes_my_server_has_unique_qids"... Which I assume would work for landlock/fsnotify?

If you'd like to try, you can re-revert these 4 patches:

Fixes be2ca3825372 ("Revert "fs/9p: simplify iget to remove unnecessary paths"")
Fixes 26f8dd2dde68 ("Revert "fs/9p: fix uaf in in v9fs_stat2inode_dotl"")
Fixes fedd06210b14 ("Revert "fs/9p: remove redundant pointer v9ses"")
Fixes f69999b5f9b4 ("Revert " fs/9p: mitigate inode collisions"")

If that works, and having this only work when a non-default option is set is acceptable, I think that's as good a way forward as we'll find.

Quoted 6 lines

1. The qid is 9pfs internal data, and we may need extra API for 9pfs to expose this to Landlock. On 64bit, this is easy as it's just the inode number (offset by 2), which we can already get from the struct inode. But perhaps on 32bit we need a way to expose the full 64bit server-sent qid to Landlock (or other kernel subsystems), if we're going to do this.

I'm not sure how much effort we want to spend on 32bit: as far as I know, if we have inode number collision on 32 bit we're already in trouble (tools like tar will consider such files to be hardlink of each other and happily skip reading data, producing corrupted archives); this is not a happy state but I don't know how to do better in any reasonable way, so we can probably keep a similar limitation for 32bit and use inode number directly...

Quoted 4 lines

2. Even though qids are supposed to be unique across the lifetime of a filesystem (including deleted files), this is not the case even for QEMU in multidevs=remap mode, when running on ext4, as tested on QEMU 10.1.0.

I'm not familiar with the qid remap implementation in qemu, but I'm curious in what case you hit that.
Deleting and recreating files? Or as you seem to say below the 'qid' is "freed" when fd is closed qemu-side and re-used by later open of other files?

If this is understood I think this can be improved, reusing the qid on different files could yield problems with caching as well so I think it's something that warrants investigations.

Quoted 6 lines

Unfortunately, holding a dentry in Landlock prevents the filesystem from being unmounted (causes WARNs), with no (proper) chance for Landlock to release those dentries. We might do it in security_sb_umount, but then at that point it is not guaranteed that the unmount will happen - perhaps we would need a new security_ hooks in the umount path?

Hmm yeah that is problematic, I don't see how to take "weak" refs that wouldn't cause a warning for the umount to free yet still prevent recycling the inode, so another hook to free up resources when really umounting sounds appropriate if we go that way... At least umount isn't usually performance sensitive so it would probably be acceptable?

Signature · 1 line
Dominique Martinet | Asmadeus
#10 lore raw
Quoted 4 lines · Dominique Martinet

Hi Tingmao,

thanks for pushing this forward, I still have very little time so haven't been able to review this properly

No worries and thanks for the quick reply :)

Quoted 10 lines

Tingmao Wang wrote on Sun, Sep 14, 2025 at 10:25:02PM +0100:

Quoted 7 lines

I had a chat with Mickaël earlier this week and some discussion following that, and we thought of a potential alternative to what I was proposing here that might work for Landlock: using the inode number (or more correctly, qid.path) directly as the keys for Landlock rules when accessing 9p files. I'm not sure how sound this is from the perspective of 9pfs (there are pros and caveats), and I would like to gather some thoughts on this idea.

Quoted 5 lines

I'm honestly split on this:
- I really don't like tracking the full path of each file around; there are various corner cases with files being removed (possibly server side!) or hard links; and it's potentially slowing down all operations a bit...

The way I see it, this tracking is really a best-effort, pragmatic solution, and in the presence of server-side changes, it's not going to always be correct (but then, because of the possibility of qid collisions, there is not really a fully fool-proof way in any case). In some sense currently hard links doesn't "work" anyway in uncached mode since each inode is separate - this doesn't change that. Files being removed is not a problem, and if another file with the same name is recreated, if it has a different qid we will create a new inode anyway (since this patch really matches against path _and_ qid). I've not measured the slowdown, but I think in uncached mode the time is mostly domainated communication overhead in any case... I think.

However, alternatives do exist (as discussed above and below)

Quoted 6 lines

- OTOH as you pointed out qid isn't as reliable, and having file paths around opens the way to rebuilding fids on reconnect for non-local servers, which could potentially be interesting (not that I ever see myself having time to work on this as I no longer have any stake there, I just know that would have interested my previous employer when they were still using 9p/rdma...)

If you / anyone else would be interested in that I can implement it, but that would probably be best done by landing the ino_path struct in this patchset first.

Quoted 7 lines
Quoted 6 lines

In discussion with Mickaël he thought that it would be acceptable for Landlock to assume that the server is well-behaved, and Landlock could specialize for 9pfs to allow access if the qid matches what's previously seen when creating the Landlock ruleset (by using the qid as the key of the rule, instead of a pointer to the inode).

Quoted 5 lines

I'm not familiar at all with landlock so forgive this question: what is this key about exactly?
When a program loads a ruleset, paths referred in that ruleset are looked up by the kernel and the inodes involved kept around in some hash table for lookup on further accesses?

Yes, that is correct (but it uses rbtree, not hash table, currently).

Quoted 4 lines

I'm fuzzy on the details but I don't see how inode pointers would be stable for other filesystems as well, what prevents
e.g. vm.drop_caches=3 to drop these inodes on ext4?

Landlock holds a reference to the inode in the ruleset, so they shouldn't be dropped. On security_sb_delete Landlock will iput those inodes so they won't cause issue with unmounting. There is some special mechanism ("landlock objects") to decouple the ruleset themselves from the actual inodes, so that previously Landlocked things can keep running even after the inode has disappeared as a result of unmounting.

Quoted 6 lines

In general I'd see the file handle (as exposed to userspace by name_to_handle_at) as a stable key, that works for all filesystems supporting fhandles (... so, not 9p, right... But in general it's something like inode number + generation, and we could expose that as handle and "just" return ENOTSUP on open_by_handle_at if that helps)

Hmm, I think this would be a good way for 9pfs to expose the qid to Landlock, by exposing it as a handle, since that is standardized.

Quoted 10 lines

Although looking at the patches what 9p seems to need isn't a new stable handle, but "just" not allocating new inodes in iget5... This was attempted in 724a08450f74 ("fs/9p: simplify iget to remove unnecessary paths"), but later reverted in be2ca3825372 ("Revert "fs/9p:
simplify iget to remove unnecessary paths"") because it broke too many users, but if you're comfortable with a new mount option for the lookup by path I think we could make a new option saying "yes_my_server_has_unique_qids"... Which I assume would work for landlock/fsnotify?

I noticed that, but assumed that simply reverting them without additional work (such as tracking the string path) would be a no go given the reason why they are reverted.

Quoted 9 lines

If you'd like to try, you can re-revert these 4 patches:

Fixes be2ca3825372 ("Revert "fs/9p: simplify iget to remove unnecessary paths"")
Fixes 26f8dd2dde68 ("Revert "fs/9p: fix uaf in in v9fs_stat2inode_dotl"")
Fixes fedd06210b14 ("Revert "fs/9p: remove redundant pointer v9ses"")
Fixes f69999b5f9b4 ("Revert " fs/9p: mitigate inode collisions"")

If that works, and having this only work when a non-default option is set is acceptable, I think that's as good a way forward as we'll find.

Well, if you think there is no other possibility for a default solution (and tracking paths by default is not feasible) I think it might also be alright if we expose the qid as a handle to Landlock (without any need for mount options), and figure out a way for Landlock to keep a fid open.

Quoted 8 lines
Quoted 7 lines

1. The qid is 9pfs internal data, and we may need extra API for 9pfs to expose this to Landlock. On 64bit, this is easy as it's just the inode number (offset by 2), which we can already get from the struct inode. But perhaps on 32bit we need a way to expose the full 64bit server-sent qid to Landlock (or other kernel subsystems), if we're going to do this.

Quoted 7 lines

I'm not sure how much effort we want to spend on 32bit: as far as I know, if we have inode number collision on 32 bit we're already in trouble (tools like tar will consider such files to be hardlink of each other and happily skip reading data, producing corrupted archives); this is not a happy state but I don't know how to do better in any reasonable way, so we can probably keep a similar limitation for 32bit and use inode number directly...

I think if 9pfs export a handle it can be the full 64bit qid.path on any platform, right?

Quoted 11 lines
Quoted 5 lines

2. Even though qids are supposed to be unique across the lifetime of a filesystem (including deleted files), this is not the case even for QEMU in multidevs=remap mode, when running on ext4, as tested on QEMU 10.1.0.

I'm not familiar with the qid remap implementation in qemu, but I'm curious in what case you hit that.
Deleting and recreating files? Or as you seem to say below the 'qid' is "freed" when fd is closed qemu-side and re-used by later open of other files?

I tried mounting a qemu-exported 9pfs backed on ext4, with multidevs=remap, and created a file, used stat to note its inode number, deleted the file, created another file (of the same OR different name), and that new file will have the same inode number.

(If I don't delete the file, then a newly created file would of course have a different ext4 inode number, and in that case QEMU exposes a different qid)

Quoted 19 lines

If this is understood I think this can be improved, reusing the qid on different files could yield problems with caching as well so I think it's something that warrants investigations.

Quoted 6 lines

Unfortunately, holding a dentry in Landlock prevents the filesystem from being unmounted (causes WARNs), with no (proper) chance for Landlock to release those dentries. We might do it in security_sb_umount, but then at that point it is not guaranteed that the unmount will happen - perhaps we would need a new security_ hooks in the umount path?

Hmm yeah that is problematic, I don't see how to take "weak" refs that wouldn't cause a warning for the umount to free yet still prevent recycling the inode, so another hook to free up resources when really umounting sounds appropriate if we go that way... At least umount isn't usually performance sensitive so it would probably be acceptable?

#11 lore raw

(Thanks Christian, replying just once but your reply was helpful)

Tingmao Wang wrote on Mon, Sep 15, 2025 at 02:44:44PM +0100:

Quoted 10 lines
Quoted 3 lines

I'm fuzzy on the details but I don't see how inode pointers would be stable for other filesystems as well, what prevents
e.g. vm.drop_caches=3 to drop these inodes on ext4?

Landlock holds a reference to the inode in the ruleset, so they shouldn't be dropped. On security_sb_delete Landlock will iput those inodes so they won't cause issue with unmounting. There is some special mechanism ("landlock objects") to decouple the ruleset themselves from the actual inodes, so that previously Landlocked things can keep running even after the inode has disappeared as a result of unmounting.

Thank you for the explanation, that makes more sense.
iirc even in cacheless mode 9p should keep inode arounds if there's an open fd somewhere

Quoted 10 lines
Quoted 9 lines

Although looking at the patches what 9p seems to need isn't a new stable handle, but "just" not allocating new inodes in iget5... This was attempted in 724a08450f74 ("fs/9p: simplify iget to remove unnecessary paths"), but later reverted in be2ca3825372 ("Revert "fs/9p:
simplify iget to remove unnecessary paths"") because it broke too many users, but if you're comfortable with a new mount option for the lookup by path I think we could make a new option saying "yes_my_server_has_unique_qids"... Which I assume would work for landlock/fsnotify?

Quoted 3 lines

I noticed that, but assumed that simply reverting them without additional work (such as tracking the string path) would be a no go given the reason why they are reverted.

Yes, just reverting and using that as default broke too much things, so this is unfortunately not acceptable... And 9p has no "negotiation" phase on mount to say "okay this is qemu with remap mode so we can do that" to enable to disable the behaviour automatically; which has been annoying in the past too.

I understand you'd prefer something that works by default.

Quoted 10 lines
Quoted 7 lines

I'm not sure how much effort we want to spend on 32bit: as far as I know, if we have inode number collision on 32 bit we're already in trouble (tools like tar will consider such files to be hardlink of each other and happily skip reading data, producing corrupted archives); this is not a happy state but I don't know how to do better in any reasonable way, so we can probably keep a similar limitation for 32bit and use inode number directly...

I think if 9pfs export a handle it can be the full 64bit qid.path on any platform, right?

yes, file handle can be an arbitrary size.

Quoted 6 lines
Quoted 5 lines

I'm not familiar with the qid remap implementation in qemu, but I'm curious in what case you hit that.
Deleting and recreating files? Or as you seem to say below the 'qid' is "freed" when fd is closed qemu-side and re-used by later open of other files?

Quoted 8 lines

I tried mounting a qemu-exported 9pfs backed on ext4, with multidevs=remap, and created a file, used stat to note its inode number, deleted the file, created another file (of the same OR different name), and that new file will have the same inode number.

(If I don't delete the file, then a newly created file would of course have a different ext4 inode number, and in that case QEMU exposes a different qid)

Ok so from Christian's reply this is just ext4 reusing the same inode.. I briefly hinted at this above, but in this case ext4 will give the inode a different generation number (so the ext4 file handle will be different, and accessing the old one will get ESTALE); but that's not something qemu currently tracks and it'd be a bit of an overhaul... In theory qemu could hash mount_id + file handle to get a properly unique qid, if we need to improve that, but that'd be limited to root users (and to filesystems that support name_to_handle_at) so I don't think it's really appropriate either... hmm..

(I also thought of checking if nlink is 0 when getting a new inode, but that's technically legimitate from /proc/x/fd opens so I don't think we can do that either)

And then there's also all the servers that don't give unique qids at all, so we'll just get weird landlock/fsnotify behaviours for them if we go that way...

-----------------

Okay, you've convinced me something like path tracking seems more appropriate; I'll just struggle one last time first with a few more open questions:
- afaiu this (reusing inodes) work in cached mode because the dentry is
kept around; I don't understand the vfs well enough but could the inodes hold its dentry and dentries hold their parent dentry alive somehow? So in cacheless mode, if you have a tree like this:
a

└── b
    ├── c
    └── d

with c 'open' (or a reference held by landlock), then dentries for a/b/c would be kept, but d could be droppable?

My understanding is that in cacheless mode we're dropping dentries aggressively so that things like readdir() are refreshed, but I'm thinking this should work even if we keep some dentries alive when their inode is held up.

- if that doesn't work (or is too complicated), I'm thinking tracking
path is probably better than qid-based filtering based on what we discussed as it only affects uncached mode.. I'll need to spend some time testing but I think we can move forward with the current patchset rather than try something new.

Thanks!

Signature · 1 line
Dominique Martinet | Asmadeus
#12 lore raw
Quoted 14 lines · Dominique Martinet

(Thanks Christian, replying just once but your reply was helpful)

Tingmao Wang wrote on Mon, Sep 15, 2025 at 02:44:44PM +0100:

Quoted 10 lines
Quoted 3 lines

I'm fuzzy on the details but I don't see how inode pointers would be stable for other filesystems as well, what prevents
e.g. vm.drop_caches=3 to drop these inodes on ext4?

Landlock holds a reference to the inode in the ruleset, so they shouldn't be dropped. On security_sb_delete Landlock will iput those inodes so they won't cause issue with unmounting. There is some special mechanism ("landlock objects") to decouple the ruleset themselves from the actual inodes, so that previously Landlocked things can keep running even after the inode has disappeared as a result of unmounting.

Quoted 3 lines

Thank you for the explanation, that makes more sense.
iirc even in cacheless mode 9p should keep inode arounds if there's an open fd somewhere

Yes, because there is a dentry that has a reference to it. Similarly if there is a Landlock rule referencing it, the inode will also be kept around (but not the dentry, Landlock only references the inode). The problem is that when another application (that is being Landlocked) accesses the file, 9pfs will create a new inode in uncached mode, regardless of whether an existing inode exists.

Quoted 12 lines
Quoted 11 lines

[...]

I tried mounting a qemu-exported 9pfs backed on ext4, with multidevs=remap, and created a file, used stat to note its inode number, deleted the file, created another file (of the same OR different name), and that new file will have the same inode number.

(If I don't delete the file, then a newly created file would of course have a different ext4 inode number, and in that case QEMU exposes a different qid)

Quoted 9 lines

Ok so from Christian's reply this is just ext4 reusing the same inode.. I briefly hinted at this above, but in this case ext4 will give the inode a different generation number (so the ext4 file handle will be different, and accessing the old one will get ESTALE); but that's not something qemu currently tracks and it'd be a bit of an overhaul... In theory qemu could hash mount_id + file handle to get a properly unique qid, if we need to improve that, but that'd be limited to root users (and to filesystems that support name_to_handle_at) so I don't think it's really appropriate either... hmm..

Actually I think I forgot that there is also qid.version, which in the case of a QEMU-exported 9pfs might just be the file modification time? In 9pfs currently we do reject a inode match if that version changed server side in cached mode:

v9fs_test_inode_dotl:

	/* compare qid details */
	if (memcmp(&v9inode->qid.version,
		   &st->qid.version, sizeof(v9inode->qid.version)))
		return 0;

(not tested whether QEMU correctly sets this version yet)

Quoted 9 lines

(I also thought of checking if nlink is 0 when getting a new inode, but that's technically legimitate from /proc/x/fd opens so I don't think we can do that either)

And then there's also all the servers that don't give unique qids at all, so we'll just get weird landlock/fsnotify behaviours for them if we go that way...

Quoted 7 lines

-----------------

Okay, you've convinced me something like path tracking seems more appropriate; I'll just struggle one last time first with a few more open questions:
- afaiu this (reusing inodes) work in cached mode because the dentry is
kept around;

Based on my understanding, I think this isn't really to do with whether the dentry is around or not. In cached mode, 9pfs will use iget5_locked to look up an existing inode based on the qid, if one exists, and use that, even if no cached dentry points to it. However, in uncached mode, currently if vfs asks 9pfs to find an inode (e.g. because the dentry is no longer in cache), it always get a new one:

v9fs_vfs_lookup:

	...
	else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
		inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
	else
		inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
	...
v9fs_qid_iget_dotl:
	...
	if (new)
		test = v9fs_test_new_inode_dotl;
	else
		test = v9fs_test_inode_dotl;
	...
v9fs_test_new_inode_dotl:
	static int v9fs_test_new_inode_dotl(struct inode *inode, void *data)
	{
		return 0;
	}
Quoted 9 lines

I don't understand the vfs well enough but could the inodes hold its dentry and dentries hold their parent dentry alive somehow? So in cacheless mode, if you have a tree like this:
a

└── b
    ├── c
    └── d

with c 'open' (or a reference held by landlock), then dentries for a/b/c would be kept, but d could be droppable?

I think, based on my understanding, a child dentry does always have a reference to its parent, and so parent won't be dropped before child, if child dentry is alive. However holding a proper dentry reference in an inode might be tricky as dentry holds the reference to its inode.

Quoted 5 lines

My understanding is that in cacheless mode we're dropping dentries aggressively so that things like readdir() are refreshed, but I'm thinking this should work even if we keep some dentries alive when their inode is held up.

If we have some way of keeping the dentry alive (without introducing circular reference problems) then I guess that would work and we don't have to track paths ourselves.

Quoted 8 lines

- if that doesn't work (or is too complicated), I'm thinking tracking
path is probably better than qid-based filtering based on what we discussed as it only affects uncached mode.. I'll need to spend some time testing but I think we can move forward with the current patchset rather than try something new.

Thanks!

Note that in discussion with Mickaël (maintainer of Landlock) he indicated that he would be comfortable for Landlock to track a qid, instead of holding a inode, specifically for 9pfs.

(This doesn't solve the problem for fsnotify though)

Kind regards,
Tingmao

#13 lore raw

Tingmao Wang wrote on Tue, Sep 16, 2025 at 01:44:27PM +0100:

Quoted 18 lines
Quoted 2 lines

iirc even in cacheless mode 9p should keep inode arounds if there's an open fd somewhere

Yes, because there is a dentry that has a reference to it. Similarly if there is a Landlock rule referencing it, the inode will also be kept around (but not the dentry, Landlock only references the inode). The problem is that when another application (that is being Landlocked) accesses the file, 9pfs will create a new inode in uncached mode, regardless of whether an existing inode exists.
[...]
Based on my understanding, I think this isn't really to do with whether the dentry is around or not. In cached mode, 9pfs will use iget5_locked to look up an existing inode based on the qid, if one exists, and use that, even if no cached dentry points to it. However, in uncached mode, currently if vfs asks 9pfs to find an inode (e.g. because the dentry is no longer in cache), it always get a new one:
[...]
v9fs_qid_iget_dotl:

Quoted 5 lines
	...
	if (new)
		test = v9fs_test_new_inode_dotl;
	else
		test = v9fs_test_inode_dotl;

Right, if we get all the way to iget uncached mode will get a new inode, but if the file is opened (I tried cat > foo and tail -f foo) then re-opening cat will not issue a lookup at all -- v9fs_vfs_lookup() is not called in the first place.
Likewise, in cached mode, just having the file in cache makes new open not call v9fs_vfs_lookup for that file (even if it's not currently open), so that if (new) is not actually what matters here afaiu.

What's the condition to make it happen? Can we make that happen with landlock?

In practice that'd make landlock partially negate cacheless mode, as we'd "pin" landlocked paths, but as long as readdirs aren't cached and other metadata is refreshed on e.g. stat() calls I think that's fine if we can make it happen.

(That's a big if)

Quoted 11 lines
Quoted 7 lines

So in cacheless mode, if you have a tree like this:
a

└── b
    ├── c
    └── d

with c 'open' (or a reference held by landlock), then dentries for a/b/c would be kept, but d could be droppable?

I think, based on my understanding, a child dentry does always have a reference to its parent, and so parent won't be dropped before child, if child dentry is alive.

I'd be tempted to agree here

Quoted 2 lines

However holding a proper dentry reference in an
inode might be tricky as dentry holds the reference to its inode.

Hmm, yeah, that's problematic.
Could it be held in landlock and not by the inode? Just thinking out loud.

Quoted 8 lines
Quoted 4 lines

My understanding is that in cacheless mode we're dropping dentries aggressively so that things like readdir() are refreshed, but I'm thinking this should work even if we keep some dentries alive when their inode is held up.

If we have some way of keeping the dentry alive (without introducing circular reference problems) then I guess that would work and we don't have to track paths ourselves.

Yes, that's the idea - the dentry basically already contain the path, so we wouldn't be reinventing the wheel...

Quoted 9 lines
Quoted 5 lines

- if that doesn't work (or is too complicated), I'm thinking tracking
path is probably better than qid-based filtering based on what we discussed as it only affects uncached mode.. I'll need to spend some time testing but I think we can move forward with the current patchset rather than try something new.

Note that in discussion with Mickaël (maintainer of Landlock) he indicated that he would be comfortable for Landlock to track a qid, instead of holding a inode, specifically for 9pfs.

Yes, I saw that, but what you pointed out about qid reuse make me somewhat uncomfortable with that direction -- you could allow a directory, delete it, create a new one somewhere else and if the underlying fs reuse the same inode number the rule would allow an intended directory instead so I'd rather not rely on qid for this either.
But if you think that's not a problem in practice (because e.g. landlock would somehow detect the dir got deleted or another good reason it's not a problem) then I agree it's probably the simplest way forward implementation-wise.

Signature · 1 line
Dominique Martinet | Asmadeus
#14 lore raw
Quoted 7 lines · Dominique Martinet

Tingmao Wang wrote on Tue, Sep 16, 2025 at 01:44:27PM +0100:

Quoted 5 lines

[...]

Note that in discussion with Mickaël (maintainer of Landlock) he indicated that he would be comfortable for Landlock to track a qid, instead of holding a inode, specifically for 9pfs.

Quoted 11 lines

Yes, I saw that, but what you pointed out about qid reuse make me somewhat uncomfortable with that direction -- you could allow a directory, delete it, create a new one somewhere else and if the underlying fs reuse the same inode number the rule would allow an intended directory instead so I'd rather not rely on qid for this either.
But if you think that's not a problem in practice (because e.g. landlock would somehow detect the dir got deleted or another good reason it's not a problem) then I agree it's probably the simplest way forward implementation-wise.

Sorry, I forgot to add that this idea would also involve Landlock holding a reference to the fid (or dentry, but that's problematic due to breaking unmount unless we can have a new hook) to keep the file open on the host side so that the qid won't be reused (ignoring collisions caused by different filesystems mounted under one 9pfs export when multidev mapping is not enabled)

(There's the separate issue of QEMU not seemingly keeping a directory open on the host when the guest has a fid to it tho. I checked that if the dir is renamed on the host side, any process in the guest that has a fd to it (checked via cd in a shell) will not be able to use that fd to read it anymore. This also means that another directory might be created with the same qid.path)

(I've not looked at the code yet but Christian, feel free to point out if I missed anything or if you disagree :) didn't realize earlier you're also the recent author of the 9pfs server in QEMU)

#15 lore raw
Quoted 20 lines · CEST Tingmao Wang
Quoted 8 lines · Dominique Martinet

Tingmao Wang wrote on Tue, Sep 16, 2025 at 01:44:27PM +0100:

Quoted 6 lines

[...]

Note that in discussion with Mickaël (maintainer of Landlock) he indicated
that he would be comfortable for Landlock to track a qid, instead of holding a inode, specifically for 9pfs.

Quoted 10 lines

Yes, I saw that, but what you pointed out about qid reuse make me somewhat uncomfortable with that direction -- you could allow a directory, delete it, create a new one somewhere else and if the underlying fs reuse the same inode number the rule would allow an intended directory instead so I'd rather not rely on qid for this either.
But if you think that's not a problem in practice (because e.g. landlock would somehow detect the dir got deleted or another good reason it's not a problem) then I agree it's probably the simplest way forward implementation-wise.

Quoted 6 lines

Sorry, I forgot to add that this idea would also involve Landlock holding a reference to the fid (or dentry, but that's problematic due to breaking unmount unless we can have a new hook) to keep the file open on the host side so that the qid won't be reused (ignoring collisions caused by different filesystems mounted under one 9pfs export when multidev mapping is not enabled)

I see that you are proposing an option for your proposed qid based re-using of dentries. I don't think it should be on by default though, considering what we already discussed (e.g. inodes recycled by ext4, but also not all 9p servers handling inode collisions).

Quoted 6 lines

(There's the separate issue of QEMU not seemingly keeping a directory open on the host when the guest has a fid to it tho. I checked that if the dir is renamed on the host side, any process in the guest that has a fd to it (checked via cd in a shell) will not be able to use that fd to read it anymore. This also means that another directory might be created with the same qid.path)

For all open FIDs QEMU retains a descriptor to the file/directory.

Which 9p message do you see sent to server, Trename or Trenameat?

Does this always happen to you or just sometimes, i.e. under heavy load? Because even though QEMU retains descriptors of open FIDs; when the QEMU process approaches host system's max. allowed number of open file descriptors then v9fs_reclaim_fd() [hw/9pfs/9p.c] is called, which closes some descriptors of older FIDs to (at least) keep the QEMU process alive.

BTW: to prevent these descriptor reclaims to happen too often, I plan to do what many other files servers do: asking the host system on process start to increase the max. number of file descriptors.

/Christian

#16 lore raw
Quoted 28 lines · Christian Schoenebeck
Quoted 20 lines · CEST Tingmao Wang
Quoted 8 lines · Dominique Martinet

Tingmao Wang wrote on Tue, Sep 16, 2025 at 01:44:27PM +0100:

Quoted 6 lines

[...]

Note that in discussion with Mickaël (maintainer of Landlock) he indicated
that he would be comfortable for Landlock to track a qid, instead of holding a inode, specifically for 9pfs.

Quoted 10 lines

Yes, I saw that, but what you pointed out about qid reuse make me somewhat uncomfortable with that direction -- you could allow a directory, delete it, create a new one somewhere else and if the underlying fs reuse the same inode number the rule would allow an intended directory instead so I'd rather not rely on qid for this either.
But if you think that's not a problem in practice (because e.g. landlock would somehow detect the dir got deleted or another good reason it's not a problem) then I agree it's probably the simplest way forward implementation-wise.

Quoted 6 lines

Sorry, I forgot to add that this idea would also involve Landlock holding a reference to the fid (or dentry, but that's problematic due to breaking unmount unless we can have a new hook) to keep the file open on the host side so that the qid won't be reused (ignoring collisions caused by different filesystems mounted under one 9pfs export when multidev mapping is not enabled)

Quoted 4 lines

I see that you are proposing an option for your proposed qid based re-using of dentries. I don't think it should be on by default though, considering what we already discussed (e.g. inodes recycled by ext4, but also not all 9p servers handling inode collisions).

Just to be clear, this approach (Landlock holding a fid reference, then using the qid as a key to search for rules when a Landlocked process accesses the previously remembered file, possibly after the file has been moved on the server) would only be in Landlock, and would only affect Landlock, not 9pfs (so not sure what you meant by "re-using of dentries").

The idea behind holding a fid reference within Landlock is that, because we have the file open, the inode would not get recycled in ext4, and thus no other file will reuse the qid, until we close that reference (when the Landlock domain terminates, or when the 9p filesystem is unmounted)

Quoted 8 lines
Quoted 7 lines

(There's the separate issue of QEMU not seemingly keeping a directory open on the host when the guest has a fid to it tho. I checked that if the dir is renamed on the host side, any process in the guest that has a fd to it (checked via cd in a shell) will not be able to use that fd to read it anymore. This also means that another directory might be created with the same qid.path)

Quoted 5 lines

For all open FIDs QEMU retains a descriptor to the file/directory.

Which 9p message do you see sent to server, Trename or Trenameat?

Does this always happen to you or just sometimes, i.e. under heavy load?

Always happen, see log: (no Trename since the rename is done on the host)

    qemu flags: -virtfs "local,path=/tmp/test,mount_tag=test,security_model=passthrough,readonly=off,multidevs=remap"
    qemu version: QEMU emulator version 10.1.0 (Debian 1:10.1.0+ds-5)
    guest kernel version: 6.17.0-rc5 (for the avoidance of doubt, this is clean 6.17-rc5 with no patches)
    qemu pid: 511476

    root@host # ls -la /proc/511476/fd | grep test
    lr-x------ 1 root root 64 Sep 17 00:35 41 -> /tmp/test

    root@guest # mount --mkdir -t 9p -o trans=virtio,cache=none,inodeident=qid,debug=13 test /tmp/test
    root@guest # mkdir /tmp/test/dir1
    root@guest # cd /tmp/test/dir1
     9pnet: -- v9fs_vfs_getattr_dotl (183): dentry: ffff888102ed4d38
     9pnet: -- v9fs_fid_find (183):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000183) >>> TGETATTR fid 1, request_mask 16383
     9pnet: (00000183) >>> size=19 type: 24 tag: 0
     9pnet: (00000183) <<< size=160 type: 25 tag: 0
     9pnet: (00000183) <<< RGETATTR st_result_mask=6143
     <<< qid=80.3.68c9c8a3
     <<< st_mode=000043ff st_nlink=3
     <<< st_uid=0 st_gid=0
     <<< st_rdev=0 st_size=3c st_blksize=131072 st_blocks=0
     <<< st_atime_sec=1758065706 st_atime_nsec=857221735
     <<< st_mtime_sec=1758065827 st_mtime_nsec=745359877
     <<< st_ctime_sec=1758065827 st_ctime_nsec=745359877
     <<< st_btime_sec=0 st_btime_nsec=0
     <<< st_gen=0 st_data_version=0
     9pnet: -- v9fs_vfs_lookup (183): dir: ffff8881090e0000 dentry: (dir1) ffff888102e458f8 flags: 1
     9pnet: -- v9fs_fid_find (183):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000183) >>> TWALK fids 1,2 nwname 1d wname[0] dir1
     9pnet: (00000183) >>> size=23 type: 110 tag: 0
     9pnet: (00000183) <<< size=22 type: 111 tag: 0
     9pnet: (00000183) <<< RWALK nwqid 1:
     9pnet: (00000183) <<<     [0] 80.5.68c9dca3
     9pnet: (00000183) >>> TGETATTR fid 2, request_mask 6143
     9pnet: (00000183) >>> size=19 type: 24 tag: 0
     9pnet: (00000183) <<< size=160 type: 25 tag: 0
     9pnet: (00000183) <<< RGETATTR st_result_mask=6143
     <<< qid=80.5.68c9dca3
     <<< st_mode=000041ed st_nlink=2
     <<< st_uid=0 st_gid=0
     <<< st_rdev=0 st_size=28 st_blksize=131072 st_blocks=0
     <<< st_atime_sec=1758065827 st_atime_nsec=745359877
     <<< st_mtime_sec=1758065827 st_mtime_nsec=745359877
     <<< st_ctime_sec=1758065827 st_ctime_nsec=749830521
     <<< st_btime_sec=0 st_btime_nsec=0
     <<< st_gen=0 st_data_version=0
     9pnet: -- v9fs_vfs_getattr_dotl (183): dentry: ffff888102e458f8
     9pnet: -- v9fs_fid_find (183):  dentry: dir1 (ffff888102e458f8) uid 0 any 0
     9pnet: (00000183) >>> TGETATTR fid 2, request_mask 16383
     9pnet: (00000183) >>> size=19 type: 24 tag: 0
     9pnet: (00000183) <<< size=160 type: 25 tag: 0
     9pnet: (00000183) <<< RGETATTR st_result_mask=6143
     <<< qid=80.5.68c9dca3
     <<< st_mode=000041ed st_nlink=2
     <<< st_uid=0 st_gid=0
     <<< st_rdev=0 st_size=28 st_blksize=131072 st_blocks=0
     <<< st_atime_sec=1758065827 st_atime_nsec=745359877
     <<< st_mtime_sec=1758065827 st_mtime_nsec=745359877
     <<< st_ctime_sec=1758065827 st_ctime_nsec=749830521
     <<< st_btime_sec=0 st_btime_nsec=0
     <<< st_gen=0 st_data_version=0
     9pnet: -- v9fs_dentry_release (183):  dentry: dir1 (ffff888102e458f8)
     9pnet: (00000183) >>> TCLUNK fid 2 (try 0)
     9pnet: (00000183) >>> size=11 type: 120 tag: 0
     9pnet: (00000183) <<< size=7 type: 121 tag: 0
     9pnet: (00000183) <<< RCLUNK fid 2
     9pnet: -- v9fs_vfs_lookup (183): dir: ffff8881090e0000 dentry: (dir1) ffff888102e45a70 flags: 3
     9pnet: -- v9fs_fid_find (183):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000183) >>> TWALK fids 1,2 nwname 1d wname[0] dir1
     9pnet: (00000183) >>> size=23 type: 110 tag: 0
     9pnet: (00000183) <<< size=22 type: 111 tag: 0
     9pnet: (00000183) <<< RWALK nwqid 1:
     9pnet: (00000183) <<<     [0] 80.5.68c9dca3
     9pnet: (00000183) >>> TGETATTR fid 2, request_mask 6143
     9pnet: (00000183) >>> size=19 type: 24 tag: 0
     9pnet: (00000183) <<< size=160 type: 25 tag: 0
     9pnet: (00000183) <<< RGETATTR st_result_mask=6143
     <<< qid=80.5.68c9dca3
     <<< st_mode=000041ed st_nlink=2
     <<< st_uid=0 st_gid=0
     <<< st_rdev=0 st_size=28 st_blksize=131072 st_blocks=0
     <<< st_atime_sec=1758065827 st_atime_nsec=745359877
     <<< st_mtime_sec=1758065827 st_mtime_nsec=745359877
     <<< st_ctime_sec=1758065827 st_ctime_nsec=749830521
     <<< st_btime_sec=0 st_btime_nsec=0
     <<< st_gen=0 st_data_version=0

     (fid 2 is now a persistent handle pointing to /dir1, not sure why the
     walk was done twice)

    root@host # ls -la /proc/511476/fd | grep test
    lr-x------ 1 root root 64 Sep 17 00:35 41 -> /tmp/test
    (no fd points to dir1)

    root@host # mv -v /tmp/test/dir1 /tmp/test/dir2
    renamed '/tmp/test/dir1' -> '/tmp/test/dir2'

    root@guest:/tmp/test/dir1# ls
     9pnet: -- v9fs_vfs_getattr_dotl (183): dentry: ffff888102e45a70
     9pnet: -- v9fs_fid_find (183):  dentry: dir1 (ffff888102e45a70) uid 0 any 0
     9pnet: (00000183) >>> TGETATTR fid 2, request_mask 16383
     9pnet: (00000183) >>> size=19 type: 24 tag: 0
     9pnet: (00000183) <<< size=11 type: 7 tag: 0
     9pnet: (00000183) <<< RLERROR (-2)
     9pnet: -- v9fs_file_open (188): inode: ffff888102e80640 file: ffff88810af45340
     9pnet: -- v9fs_fid_find (188):  dentry: dir1 (ffff888102e45a70) uid 0 any 0
     9pnet: (00000188) >>> TWALK fids 2,3 nwname 0d wname[0] (null)
     9pnet: (00000188) >>> size=17 type: 110 tag: 0
     9pnet: (00000188) <<< size=11 type: 7 tag: 0
     9pnet: (00000188) <<< RLERROR (-2)
    ls: cannot open directory '.': No such file or directory

It looks like as soon as the directory was moved on the host, TGETATTR on the guest-opened fid 2 fails, even though I would expect that if QEMU opens a fd to the dir and use that fd whenever fid 2 is used, that TGETATTR should succeed. The fact that I can't see anything pointing to dir1 in /proc/511476/fd was also suspicious.

Also, if I remove the dir on the host, then repoen it in the guest, ls starts working again:

    root@host # mv -v /tmp/test/dir2 /tmp/test/dir1
    renamed '/tmp/test/dir2' -> '/tmp/test/dir1'

    root@guest:/tmp/test/dir1# ls
     9pnet: -- v9fs_file_open (189): inode: ffff888102e80640 file: ffff88810af47100
     9pnet: -- v9fs_fid_find (189):  dentry: dir1 (ffff888102e45a70) uid 0 any 0
     9pnet: (00000189) >>> TWALK fids 2,3 nwname 0d wname[0] (null)
     9pnet: (00000189) >>> size=17 type: 110 tag: 0
     9pnet: (00000189) <<< size=9 type: 111 tag: 0
     9pnet: (00000189) <<< RWALK nwqid 0:
     9pnet: (00000189) >>> TLOPEN fid 3 mode 100352
     9pnet: (00000189) >>> size=15 type: 12 tag: 0
     9pnet: (00000189) <<< size=24 type: 13 tag: 0
     9pnet: (00000189) <<< RLOPEN qid 80.5.68c9dca3 iounit 0
     9pnet: -- v9fs_vfs_getattr_dotl (189): dentry: ffff888102e45a70
     9pnet: -- v9fs_fid_find (189):  dentry: dir1 (ffff888102e45a70) uid 0 any 0
     9pnet: (00000189) >>> TGETATTR fid 2, request_mask 16383
     9pnet: (00000189) >>> size=19 type: 24 tag: 0
     9pnet: (00000189) <<< size=160 type: 25 tag: 0
     9pnet: (00000189) <<< RGETATTR st_result_mask=6143
     <<< qid=80.5.68c9dca3
     <<< st_mode=000041ed st_nlink=2
     <<< st_uid=0 st_gid=0
     <<< st_rdev=0 st_size=28 st_blksize=131072 st_blocks=0
     <<< st_atime_sec=1758065827 st_atime_nsec=745359877
     <<< st_mtime_sec=1758065827 st_mtime_nsec=745359877
     <<< st_ctime_sec=1758066075 st_ctime_nsec=497687251
     <<< st_btime_sec=0 st_btime_nsec=0
     <<< st_gen=0 st_data_version=0
     9pnet: -- v9fs_dir_readdir_dotl (189): name dir1
     9pnet: (00000189) >>> TREADDIR fid 3 offset 0 count 131072
     9pnet: (00000189) >>> size=23 type: 40 tag: 0
     9pnet: (00000189) <<< size=62 type: 41 tag: 0
     9pnet: (00000189) <<< RREADDIR count 51
     9pnet: (00000189) >>> TREADDIR fid 3 offset 2147483647 count 131072
     9pnet: (00000189) >>> size=23 type: 40 tag: 0
     9pnet: (00000189) <<< size=11 type: 41 tag: 0
     9pnet: (00000189) <<< RREADDIR count 0
     9pnet: -- v9fs_dir_readdir_dotl (189): name dir1
     9pnet: (00000189) >>> TREADDIR fid 3 offset 2147483647 count 131072
     9pnet: (00000189) >>> size=23 type: 40 tag: 0
     9pnet: (00000189) <<< size=11 type: 41 tag: 0
     9pnet: (00000189) <<< RREADDIR count 0
     9pnet: -- v9fs_dir_release (189): inode: ffff888102e80640 filp: ffff88810af47100 fid: 3
     9pnet: (00000189) >>> TCLUNK fid 3 (try 0)
     9pnet: (00000189) >>> size=11 type: 120 tag: 0
     9pnet: (00000189) <<< size=7 type: 121 tag
    root@guest:/tmp/test/dir1# echo $?
    0

Somehow if I rename in the guest, it all works, even though it's using the same fid 2 (and it didn't ask QEMU to walk the new path)

    root@guest:/tmp/test/dir1# mv /tmp/test/dir1 /tmp/test/dir2
     9pnet: -- v9fs_vfs_getattr_dotl (183): dentry: ffff888102e45a70
     9pnet: -- v9fs_fid_find (183):  dentry: dir1 (ffff888102e45a70) uid 0 any 0
     9pnet: (00000183) >>> TGETATTR fid 2, request_mask 16383
     9pnet: (00000183) >>> size=19 type: 24 tag: 0
     9pnet: (00000183) <<< size=160 type: 25 tag: 0
     9pnet: (00000183) <<< RGETATTR st_result_mask=6143
     <<< qid=80.5.68c9dca3
     <<< st_mode=000041ed st_nlink=2
     <<< st_uid=0 st_gid=0
     <<< st_rdev=0 st_size=28 st_blksize=131072 st_blocks=0
     <<< st_atime_sec=1758066561 st_atime_nsec=442431580
     <<< st_mtime_sec=1758065827 st_mtime_nsec=745359877
     <<< st_ctime_sec=1758066559 st_ctime_nsec=570428555
     <<< st_btime_sec=0 st_btime_nsec=0
     <<< st_gen=0 st_data_version=0
     9pnet: -- v9fs_vfs_lookup (194): dir: ffff8881090e0000 dentry: (dir2) ffff888102edca48 flags: e0000
     9pnet: -- v9fs_fid_find (194):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000194) >>> TWALK fids 1,3 nwname 1d wname[0] dir2
     9pnet: (00000194) >>> size=23 type: 110 tag: 0
     9pnet: (00000194) <<< size=11 type: 7 tag: 0
     9pnet: (00000194) <<< RLERROR (-2)
     9pnet: -- v9fs_dentry_release (194):  dentry: dir2 (ffff888102edca48)
     9pnet: -- v9fs_vfs_lookup (194): dir: ffff8881090e0000 dentry: (dir2) ffff888102edcbc0 flags: 0
     9pnet: -- v9fs_fid_find (194):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000194) >>> TWALK fids 1,3 nwname 1d wname[0] dir2
     9pnet: (00000194) >>> size=23 type: 110 tag: 0
     9pnet: (00000194) <<< size=11 type: 7 tag: 0
     9pnet: (00000194) <<< RLERROR (-2)
     9pnet: -- v9fs_dentry_release (194):  dentry: dir2 (ffff888102edcbc0)
     9pnet: -- v9fs_vfs_lookup (194): dir: ffff8881090e0000 dentry: (dir2) ffff888102edcd38 flags: a0000
     9pnet: -- v9fs_fid_find (194):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000194) >>> TWALK fids 1,3 nwname 1d wname[0] dir2
     9pnet: (00000194) >>> size=23 type: 110 tag: 0
     9pnet: (00000194) <<< size=11 type: 7 tag: 0
     9pnet: (00000194) <<< RLERROR (-2)
     9pnet: -- v9fs_vfs_rename (194): 
     9pnet: -- v9fs_fid_find (194):  dentry: dir1 (ffff888102e45a70) uid 0 any 0
     9pnet: -- v9fs_fid_find (194):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000194) >>> TWALK fids 1,3 nwname 0d wname[0] (null)
     9pnet: (00000194) >>> size=17 type: 110 tag: 0
     9pnet: (00000194) <<< size=9 type: 111 tag: 0
     9pnet: (00000194) <<< RWALK nwqid 0:
     9pnet: -- v9fs_fid_find (194):  dentry: / (ffff888102ed4d38) uid 0 any 0
     9pnet: (00000194) >>> TWALK fids 1,4 nwname 0d wname[0] (null)
     9pnet: (00000194) >>> size=17 type: 110 tag: 0
     9pnet: (00000194) <<< size=9 type: 111 tag: 0
     9pnet: (00000194) <<< RWALK nwqid 0:
     9pnet: (00000194) >>> TRENAMEAT olddirfid 3 old name dir1 newdirfid 4 new name dir2
     9pnet: (00000194) >>> size=27 type: 74 tag: 0
     9pnet: (00000194) <<< size=7 type: 75 tag: 0
     9pnet: (00000194) <<< RRENAMEAT newdirfid 4 new name dir2
     9pnet: (00000194) >>> TCLUNK fid 4 (try 0)
     9pnet: (00000194) >>> size=11 type: 120 tag: 0
     9pnet: (00000194) <<< size=7 type: 121 tag: 0
     9pnet: (00000194) <<< RCLUNK fid 4
     9pnet: (00000194) >>> TCLUNK fid 3 (try 0)
     9pnet: (00000194) >>> size=11 type: 120 tag: 0
     9pnet: (00000194) <<< size=7 type: 121 tag: 0
     9pnet: (00000194) <<< RCLUNK fid 3
     9pnet: -- v9fs_dentry_release (194):  dentry: dir2 (ffff888102edcd38)
    root@guest:/tmp/test/dir1# ls
     9pnet: -- v9fs_file_open (195): inode: ffff888102e80640 file: ffff88810b2b1500
     9pnet: -- v9fs_fid_find (195):  dentry: dir2 (ffff888102e45a70) uid 0 any 0
     9pnet: (00000195) >>> TWALK fids 2,3 nwname 0d wname[0] (null)
     9pnet: (00000195) >>> size=17 type: 110 tag: 0
     9pnet: (00000195) <<< size=9 type: 111 tag: 0
     9pnet: (00000195) <<< RWALK nwqid 0:
     9pnet: (00000195) >>> TLOPEN fid 3 mode 100352
     9pnet: (00000195) >>> size=15 type: 12 tag: 0
     9pnet: (00000195) <<< size=24 type: 13 tag: 0
     9pnet: (00000195) <<< RLOPEN qid 80.5.68c9dca3 iounit 0
     9pnet: -- v9fs_vfs_getattr_dotl (195): dentry: ffff888102e45a70
     9pnet: -- v9fs_fid_find (195):  dentry: dir2 (ffff888102e45a70) uid 0 any 0
     9pnet: (00000195) >>> TGETATTR fid 2, request_mask 16383
     9pnet: (00000195) >>> size=19 type: 24 tag: 0
     9pnet: (00000195) <<< size=160 type: 25 tag: 0
     9pnet: (00000195) <<< RGETATTR st_result_mask=6143
     <<< qid=80.5.68c9dca3
     <<< st_mode=000041ed st_nlink=2
     <<< st_uid=0 st_gid=0
     <<< st_rdev=0 st_size=28 st_blksize=131072 st_blocks=0
     <<< st_atime_sec=1758066561 st_atime_nsec=442431580
     <<< st_mtime_sec=1758065827 st_mtime_nsec=745359877
     <<< st_ctime_sec=1758066568 st_ctime_nsec=562443096
     <<< st_btime_sec=0 st_btime_nsec=0
     <<< st_gen=0 st_data_version=0
     9pnet: -- v9fs_dir_readdir_dotl (195): name dir2
     9pnet: (00000195) >>> TREADDIR fid 3 offset 0 count 131072
     9pnet: (00000195) >>> size=23 type: 40 tag: 0
     9pnet: (00000195) <<< size=62 type: 41 tag: 0
     9pnet: (00000195) <<< RREADDIR count 51
     9pnet: (00000195) >>> TREADDIR fid 3 offset 2147483647 count 131072
     9pnet: (00000195) >>> size=23 type: 40 tag: 0
     9pnet: (00000195) <<< size=11 type: 41 tag: 0
     9pnet: (00000195) <<< RREADDIR count 0
     9pnet: -- v9fs_dir_readdir_dotl (195): name dir2
     9pnet: (00000195) >>> TREADDIR fid 3 offset 2147483647 count 131072
     9pnet: (00000195) >>> size=23 type: 40 tag: 0
     9pnet: (00000195) <<< size=11 type: 41 tag: 0
     9pnet: (00000195) <<< RREADDIR count 0
     9pnet: -- v9fs_dir_release (195): inode: ffff888102e80640 filp: ffff88810b2b1500 fid: 3
     9pnet: (00000195) >>> TCLUNK fid 3 (try 0)
     9pnet: (00000195) >>> size=11 type: 120 tag: 0
     9pnet: (00000195) <<< size=7 type: 121 tag: 0
     9pnet: (00000195) <<< RCLUNK fid 3

If this is surprising, I'm happy to take a deeper look over the weekend (but I've never tried to debug QEMU itself :D)

Quoted 8 lines

Because even though QEMU retains descriptors of open FIDs; when the QEMU process approaches host system's max. allowed number of open file descriptors then v9fs_reclaim_fd() [hw/9pfs/9p.c] is called, which closes some descriptors of older FIDs to (at least) keep the QEMU process alive.

BTW: to prevent these descriptor reclaims to happen too often, I plan to do what many other files servers do: asking the host system on process start to increase the max. number of file descriptors.

Note that the above is reproduced with only 1 file open (the dir being renamed around)

Kind regards,
Tingmao

Quoted 4 lines

/Christian

#17 lore raw
Quoted 2 lines · CEST Tingmao Wang
Quoted 1 line · Christian Schoenebeck

On Tuesday, September 16, 2025 4:01:40 PM CEST Tingmao Wang wrote:

[...]

Quoted 11 lines
Quoted 4 lines

I see that you are proposing an option for your proposed qid based re-using of dentries. I don't think it should be on by default though, considering what we already discussed (e.g. inodes recycled by ext4, but also not all 9p servers handling inode collisions).

Just to be clear, this approach (Landlock holding a fid reference, then using the qid as a key to search for rules when a Landlocked process accesses the previously remembered file, possibly after the file has been moved on the server) would only be in Landlock, and would only affect Landlock, not 9pfs (so not sure what you meant by "re-using of dentries").

Quoted 4 lines

The idea behind holding a fid reference within Landlock is that, because we have the file open, the inode would not get recycled in ext4, and thus no other file will reuse the qid, until we close that reference (when the Landlock domain terminates, or when the 9p filesystem is unmounted)

So far I only had a glimpse on your kernel patches and had the impression that they are changing behaviour for all users, since you are touching dentry lookup.

Quoted 7 lines
Quoted 5 lines

For all open FIDs QEMU retains a descriptor to the file/directory.

Which 9p message do you see sent to server, Trename or Trenameat?

Does this always happen to you or just sometimes, i.e. under heavy load?

Always happen, see log: (no Trename since the rename is done on the host)

[...]

Quoted 2 lines

Somehow if I rename in the guest, it all works, even though it's using the same fid 2 (and it didn't ask QEMU to walk the new path)

Got it. Even though QEMU *should* hold a file descriptor (or a DIR* stream, which should imply a file descriptor), there is still a path string stored at V9fsFidState and that path being processed at some places, probably because there are path based and FID based variants (e.g Trename vs. Trenameat). Maybe that clashes somewhere, not sure. So I fear you would need to debug this.

/Christian

#18 lore raw
Quoted 15 lines · Christian Schoenebeck
Quoted 2 lines · CEST Tingmao Wang
Quoted 1 line · Christian Schoenebeck

On Tuesday, September 16, 2025 4:01:40 PM CEST Tingmao Wang wrote:

[...]

Quoted 11 lines
Quoted 4 lines

I see that you are proposing an option for your proposed qid based re-using of dentries. I don't think it should be on by default though, considering what we already discussed (e.g. inodes recycled by ext4, but also not all 9p servers handling inode collisions).

Just to be clear, this approach (Landlock holding a fid reference, then using the qid as a key to search for rules when a Landlocked process accesses the previously remembered file, possibly after the file has been moved on the server) would only be in Landlock, and would only affect Landlock, not 9pfs (so not sure what you meant by "re-using of dentries").

Quoted 8 lines
Quoted 4 lines

The idea behind holding a fid reference within Landlock is that, because we have the file open, the inode would not get recycled in ext4, and thus no other file will reuse the qid, until we close that reference (when the Landlock domain terminates, or when the 9p filesystem is unmounted)

So far I only had a glimpse on your kernel patches and had the impression that they are changing behaviour for all users, since you are touching dentry lookup.

I think we should not hold dentries because:
- they reference other dentries (i.e. a file hierarchy),
- they block umount and I'm convinced the VFS (and users) are not going to like long-lived dentries,
- Landlock and inotify don't need dentries, just inodes.

I'm wondering why fid are referenced by dentries instead of inodes.

The need for Landlock is to be able to match an inode with a previously seen one. Not all LSM hooks (nor VFS internals) always have access to dentries, but they do have access to inodes.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

For all open FIDs QEMU retains a descriptor to the file/directory.

Which 9p message do you see sent to server, Trename or Trenameat?

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

Does this always happen to you or just sometimes, i.e. under heavy load?

Always happen, see log: (no Trename since the rename is done on the host)

[...]

Quoted 2 lines

Somehow if I rename in the guest, it all works, even though it's using the same fid 2 (and it didn't ask QEMU to walk the new path)

Got it. Even though QEMU *should* hold a file descriptor (or a DIR* stream,

It's reasonable to assume that QEMU and other should hold opened fid In practice, this might not always be the case, but let's move on and consider that a 9p server bug.

Landlock and fanotify need some guarantees on opened files, and we cannot consider every server bug. For Landlock, inode may get an "ephemeral tag" (with the Landlock object mechanism) to match previously seen inodes. In a perfect world, Landlock could keep a reference on 9p inodes (as for other filesystems) and these inodes would always match the same file. In practice this is not the case, but the 9p client requirements and the Landlock requirements are not exactly the same.

A 9p client (the kernel) wants to safely deal with duplicated qid, which should not happen but still happen in practice as explained before. On the other side, Landlock wants to not deny access to allowed files (currently identified by their inodes), but I think it would be reasonable to allow access theoretically denied (i.e. not allowed to be precise, because of the denied by default mechanism) files because of a 9p server bug mishandling qid (e.g. mapping them to recycled ext4 inodes).

All that to say that it looks reasonable for Landlock to trust the filesystem, and by that I mean all its dependencies, including the 9p server, to not have bugs.

Another advantage to rely on qid and server-side opened files is that we get (in theory) the same semantic as when Landlock is used with local filesystems (e.g. files moved on the server should still be correctly identified by Landlock on the client).

Quoted 4 lines

which should imply a file descriptor), there is still a path string stored at V9fsFidState and that path being processed at some places, probably because there are path based and FID based variants (e.g Trename vs. Trenameat). Maybe that clashes somewhere, not sure. So I fear you would need to debug this.

Good to know that it is not a legitimate behavior for a 9p client.

#19 lore raw
Quoted 81 lines · Mickaël Salaün
Quoted 15 lines · Christian Schoenebeck
Quoted 2 lines · CEST Tingmao Wang
Quoted 1 line · Christian Schoenebeck

On Tuesday, September 16, 2025 4:01:40 PM CEST Tingmao Wang wrote:

[...]

Quoted 11 lines
Quoted 4 lines

I see that you are proposing an option for your proposed qid based re-using of dentries. I don't think it should be on by default though, considering what we already discussed (e.g. inodes recycled by ext4, but also not all 9p servers handling inode collisions).

Just to be clear, this approach (Landlock holding a fid reference, then using the qid as a key to search for rules when a Landlocked process accesses the previously remembered file, possibly after the file has been moved on the server) would only be in Landlock, and would only affect Landlock, not 9pfs (so not sure what you meant by "re-using of dentries").

Quoted 8 lines
Quoted 4 lines

The idea behind holding a fid reference within Landlock is that, because we have the file open, the inode would not get recycled in ext4, and thus no other file will reuse the qid, until we close that reference (when the Landlock domain terminates, or when the 9p filesystem is unmounted)

So far I only had a glimpse on your kernel patches and had the impression that they are changing behaviour for all users, since you are touching dentry lookup.

I think we should not hold dentries because:
- they reference other dentries (i.e. a file hierarchy),
- they block umount and I'm convinced the VFS (and users) are not going to like long-lived dentries,
- Landlock and inotify don't need dentries, just inodes.

I'm wondering why fid are referenced by dentries instead of inodes.

The need for Landlock is to be able to match an inode with a previously seen one. Not all LSM hooks (nor VFS internals) always have access to dentries, but they do have access to inodes.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

For all open FIDs QEMU retains a descriptor to the file/directory.

Which 9p message do you see sent to server, Trename or Trenameat?

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

Does this always happen to you or just sometimes, i.e. under heavy load?

Always happen, see log: (no Trename since the rename is done on the host)

[...]

Quoted 2 lines

Somehow if I rename in the guest, it all works, even though it's using the same fid 2 (and it didn't ask QEMU to walk the new path)

Got it. Even though QEMU *should* hold a file descriptor (or a DIR* stream,

It's reasonable to assume that QEMU and other should hold opened fid In practice, this might not always be the case, but let's move on and consider that a 9p server bug.

Landlock and fanotify need some guarantees on opened files, and we cannot consider every server bug. For Landlock, inode may get an "ephemeral tag" (with the Landlock object mechanism) to match previously seen inodes. In a perfect world, Landlock could keep a reference on 9p inodes (as for other filesystems) and these inodes would always match the same file. In practice this is not the case, but the 9p client requirements and the Landlock requirements are not exactly the same.

A 9p client (the kernel) wants to safely deal with duplicated qid, which should not happen but still happen in practice as explained before. On the other side, Landlock wants to not deny access to allowed files (currently identified by their inodes), but I think it would be reasonable to allow access theoretically denied (i.e. not allowed to be precise, because of the denied by default mechanism) files because of a 9p server bug mishandling qid (e.g. mapping them to recycled ext4 inodes).

All that to say that it looks reasonable for Landlock to trust the filesystem, and by that I mean all its dependencies, including the 9p server, to not have bugs.

Another advantage to rely on qid and server-side opened files is that we get (in theory) the same semantic as when Landlock is used with local filesystems (e.g. files moved on the server should still be correctly identified by Landlock on the client).

Quoted 6 lines
Quoted 4 lines

which should imply a file descriptor), there is still a path string stored at V9fsFidState and that path being processed at some places, probably because there are path based and FID based variants (e.g Trename vs. Trenameat). Maybe that clashes somewhere, not sure. So I fear you would need to debug this.

Good to know that it is not a legitimate behavior for a 9p client.

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].

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'm not sure if this was is intentional in QEMU - it would seem to me that a fid should translate to a fd (maybe opened with just O_PATH) on the host, and path walks based on that fid should be done via openat with this fd, which will also "automatically" handle renames without QEMU needing to fixup the string paths?

In any case, this probably means that even if Landlock were to hold a fid reference, and QEMU does qid remapping, that's still not enough to guarantees that we won't have a different, unrelated file ending up with the same qid, at least under ext4.

I'm not sure what's the way forward - would Landlock need to actually "open" the files (or do something that will cause a Topen to be issued by v9fs)? Alternatively if we believe this to be a QEMU issue, maybe Landlock don't need to work around it and should just hold fids (and use QIDs to key the rules) anyway despite server quirks like these. This can perhaps then be fixed in QEMU?

(I guess the fact that QEMU is doing path tracking in the first place does gives more precedent for justifying doing path tracking in v9fs as well, but maybe that's the wrong way to think about it)

Test programs: openat.c [3], open_procselffd.c [4]

[1]: https://gitlab.com/qemu-project/qemu/-/blob/44f51c1a3cf435daa82eb757740b59b1fd4fe71c/hw/9pfs/9p.c#L3403
[2]: https://gitlab.com/qemu-project/qemu/-/blob/371a269ff8ce561c28e4fa03bb49e4940f990637/hw/9pfs/9p-local.c#L1243
[3]: https://fileshare.maowtm.org/9pfs-landlock-fix/20250921/openat.c
[4]: https://fileshare.maowtm.org/9pfs-landlock-fix/20250921/open_procselffd.c

#20 lore raw

Adding Greg Kurz too.

Quoted 110 lines · Tingmao Wang
Quoted 81 lines · Mickaël Salaün
Quoted 15 lines · Christian Schoenebeck
Quoted 2 lines · CEST Tingmao Wang
Quoted 1 line · Christian Schoenebeck

On Tuesday, September 16, 2025 4:01:40 PM CEST Tingmao Wang wrote:

[...]

Quoted 11 lines
Quoted 4 lines

I see that you are proposing an option for your proposed qid based re-using of dentries. I don't think it should be on by default though, considering what we already discussed (e.g. inodes recycled by ext4, but also not all 9p servers handling inode collisions).

Just to be clear, this approach (Landlock holding a fid reference, then using the qid as a key to search for rules when a Landlocked process accesses the previously remembered file, possibly after the file has been moved on the server) would only be in Landlock, and would only affect Landlock, not 9pfs (so not sure what you meant by "re-using of dentries").

Quoted 8 lines
Quoted 4 lines

The idea behind holding a fid reference within Landlock is that, because we have the file open, the inode would not get recycled in ext4, and thus no other file will reuse the qid, until we close that reference (when the Landlock domain terminates, or when the 9p filesystem is unmounted)

So far I only had a glimpse on your kernel patches and had the impression that they are changing behaviour for all users, since you are touching dentry lookup.

I think we should not hold dentries because:
- they reference other dentries (i.e. a file hierarchy),
- they block umount and I'm convinced the VFS (and users) are not going to like long-lived dentries,
- Landlock and inotify don't need dentries, just inodes.

I'm wondering why fid are referenced by dentries instead of inodes.

The need for Landlock is to be able to match an inode with a previously seen one. Not all LSM hooks (nor VFS internals) always have access to dentries, but they do have access to inodes.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

For all open FIDs QEMU retains a descriptor to the file/directory.

Which 9p message do you see sent to server, Trename or Trenameat?

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

Does this always happen to you or just sometimes, i.e. under heavy load?

Always happen, see log: (no Trename since the rename is done on the host)

[...]

Quoted 2 lines

Somehow if I rename in the guest, it all works, even though it's using the same fid 2 (and it didn't ask QEMU to walk the new path)

Got it. Even though QEMU *should* hold a file descriptor (or a DIR* stream,

It's reasonable to assume that QEMU and other should hold opened fid In practice, this might not always be the case, but let's move on and consider that a 9p server bug.

Landlock and fanotify need some guarantees on opened files, and we cannot consider every server bug. For Landlock, inode may get an "ephemeral tag" (with the Landlock object mechanism) to match previously seen inodes. In a perfect world, Landlock could keep a reference on 9p inodes (as for other filesystems) and these inodes would always match the same file. In practice this is not the case, but the 9p client requirements and the Landlock requirements are not exactly the same.

A 9p client (the kernel) wants to safely deal with duplicated qid, which should not happen but still happen in practice as explained before. On the other side, Landlock wants to not deny access to allowed files (currently identified by their inodes), but I think it would be reasonable to allow access theoretically denied (i.e. not allowed to be precise, because of the denied by default mechanism) files because of a 9p server bug mishandling qid (e.g. mapping them to recycled ext4 inodes).

All that to say that it looks reasonable for Landlock to trust the filesystem, and by that I mean all its dependencies, including the 9p server, to not have bugs.

Another advantage to rely on qid and server-side opened files is that we get (in theory) the same semantic as when Landlock is used with local filesystems (e.g. files moved on the server should still be correctly identified by Landlock on the client).

Quoted 6 lines
Quoted 4 lines

which should imply a file descriptor), there is still a path string stored at V9fsFidState and that path being processed at some places, probably because there are path based and FID based variants (e.g Trename vs. Trenameat). Maybe that clashes somewhere, not sure. So I fear you would need to debug this.

Good to know that it is not a legitimate behavior for a 9p client.

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].

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].

Quoted 5 lines

I'm not sure if this was is intentional in QEMU - it would seem to me that a fid should translate to a fd (maybe opened with just O_PATH) on the host, and path walks based on that fid should be done via openat with this fd, which will also "automatically" handle renames without QEMU needing to fixup the string paths?

I agree, it would make sense for QEMU to map fid to FD+O_PATH. That would avoid the kind of issues you mentioned.

Christian, Greg, what do you think?

Quoted 6 lines

In any case, this probably means that even if Landlock were to hold a fid reference, and QEMU does qid remapping, that's still not enough to guarantees that we won't have a different, unrelated file ending up with the same qid, at least under ext4.

Quoted 8 lines

I'm not sure what's the way forward - would Landlock need to actually "open" the files (or do something that will cause a Topen to be issued by v9fs)?

Alternatively if we believe this to be a QEMU issue, maybe Landlock don't need to work around it and should just hold fids (and use QIDs to key the rules) anyway despite server quirks like these. This can perhaps then be fixed in QEMU?

Yes, I think it would make sense for Landlock to open and keep open a fid (and hopefully the related remote file). However, the v9fs umount should be handled gracefully the same way Landlock tag inodes are handled. This should come with a QEMU patch to fix the consistency issue.

Quoted 4 lines

(I guess the fact that QEMU is doing path tracking in the first place does gives more precedent for justifying doing path tracking in v9fs as well, but maybe that's the wrong way to think about it)

Anyway, if QEMU does it, wouldn't it be the same for Landlock to just rely on fid? If QEMU uses FD+O_PATH, then Landlock would work even for server-moved files.

Quoted 9 lines

Test programs: openat.c [3], open_procselffd.c [4]

[1]: https://gitlab.com/qemu-project/qemu/-/blob/44f51c1a3cf435daa82eb757740b59b1fd4fe71c/hw/9pfs/9p.c#L3403
[2]: https://gitlab.com/qemu-project/qemu/-/blob/371a269ff8ce561c28e4fa03bb49e4940f990637/hw/9pfs/9p-local.c#L1243
[3]: https://fileshare.maowtm.org/9pfs-landlock-fix/20250921/openat.c
[4]: https://fileshare.maowtm.org/9pfs-landlock-fix/20250921/open_procselffd.c

#21 lore raw
Quoted 18 lines · Mickaël Salaün

Adding Greg Kurz too.

Quoted 7 lines · Tingmao Wang
Quoted 1 line · Mickaël Salaün

[...]

Alternatively if we believe this to be a QEMU issue, maybe Landlock don't need to work around it and should just hold fids (and use QIDs to key the rules) anyway despite server quirks like these. This can perhaps then be fixed in QEMU?

Yes, I think it would make sense for Landlock to open and keep open a fid (and hopefully the related remote file). However, the v9fs umount should be handled gracefully the same way Landlock tag inodes are handled. This should come with a QEMU patch to fix the consistency issue.

Quoted 1 line
Quoted 6 lines
Quoted 3 lines

(I guess the fact that QEMU is doing path tracking in the first place does gives more precedent for justifying doing path tracking in v9fs as well, but maybe that's the wrong way to think about it)

Anyway, if QEMU does it, wouldn't it be the same for Landlock to just rely on fid?

The fid can't be relied on because it's just a handle. The client can open multiple fids pointing to the same file (and in fact this is what v9fs does - new fid for each open())

Quoted 2 lines

If QEMU uses FD+O_PATH, then Landlock would work even for server-moved files.

(With this new approach, Landlock would have to key the rules based on qid, but it also needs to hold an open fid to prevent that qid from being reused (due to ext4 inode number reuse, etc))

#22 lore raw
In reply to Tingmao Wang
Quoted 1 line · CEST Tingmao Wang

On 9/17/25 16:00, Mickaël Salaün wrote:

[...]

Hi Greg,

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.

Quoted 10 lines

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.

Quoted 9 lines

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?

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.

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.

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?

/Christian

#23 lore raw
Quoted 6 lines · On Mon, 29 Sep 2025 15:06:59 +0200 Christian Schoenebeck…
Quoted 1 line · CEST Tingmao Wang

On 9/17/25 16:00, Mickaël Salaün wrote:

[...]

Hi Greg,

Hi Christian,

Quoted 7 lines

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 ;-)

Quoted 31 lines
Quoted 10 lines

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.

Quoted 9 lines

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].

Quoted 9 lines

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.

Quoted 5 lines

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 ?

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

/Christian

Cheers,

Signature · 1 line
Greg
#24 lore raw

Hi Christian, Greg, Dominique,

Trying to revive this discussion, some answers to Greg's questions below, then more thoughts on this series and alternatives.

Quoted 69 lines · Greg Kurz
Quoted 6 lines · On Mon, 29 Sep 2025 15:06:59 +0200 Christian Schoenebeck…
Quoted 1 line · CEST Tingmao Wang

On 9/17/25 16:00, Mickaël Salaün wrote:

[...]

Hi Greg,

Hi Christian,

Quoted 7 lines

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 ;-)

Quoted 31 lines
Quoted 10 lines

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.

Quoted 9 lines

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].

Quoted 9 lines

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.

Quoted 8 lines
Quoted 5 lines

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 ?

There are two cases where things break: openat() with a previously opened dir fd, and re-open()ing an O_PATH fd as readable or writable with /proc/self/fd.

For the first case,
https://fileshare.maowtm.org/9pfs-landlock-fix/20250921/openat.c
is an example program that demonstrates that in QEMU, path walk always starts from the export root. The breakage is when the dir is moved on the host, even if the file still exists within the dir with the same name, the openat() with the opened dir fd (which translates to a fid in v9fs) breaks:

    (Host tmpfs exported as 9p to guest /tmp/9p)
    root@7-2-0-rc2-dev:/# ./openat /tmp/9p/dir file
    [   12.264880][  T183] 9pnet: (00000183) >>> TWALK fids 1,2 nwname 1d wname[0] dir
    [   12.265156][  T183] 9pnet: (00000183) >>> size=22 type: 110 tag: 0
    [   12.265466][  T183] 9pnet: (00000183) <<< size=22 type: 111 tag: 0
    [   12.265590][  T183] 9pnet: (00000183) <<< RWALK nwqid 1:
    [   12.265675][  T183] 9pnet: (00000183) <<<     [0] 80.6d10.6a53fe73
    [   12.265781][  T183] 9pnet: (00000183) >>> TGETATTR fid 2, request_mask 6143
    [   12.265903][  T183] 9pnet: (00000183) >>> size=19 type: 24 tag: 0
    [   12.266062][  T183] 9pnet: (00000183) <<< size=160 type: 25 tag: 0
    [   12.266148][  T183] 9pnet: (00000183) <<< RGETATTR st_result_mask=6143
    [   12.266148][  T183] <<< qid=80.6d10.6a53fe73
    [   12.266148][  T183] <<< st_mode=000041e8 st_nlink=2
    [   12.266148][  T183] <<< st_uid=1000 st_gid=1000
    [   12.266148][  T183] <<< st_rdev=0 st_size=3c st_blksize=131072 st_blocks=0
    [   12.266148][  T183] <<< st_atime_sec=1783874216 st_atime_nsec=302475718
    [   12.266148][  T183] <<< st_mtime_sec=1783874163 st_mtime_nsec=254338030
    [   12.266148][  T183] <<< st_ctime_sec=1783874163 st_ctime_nsec=254338030
    [   12.266148][  T183] <<< st_btime_sec=0 st_btime_nsec=0
    [   12.266148][  T183] <<< st_gen=0 st_data_version=0
    [   12.266932][  T183] 9pnet: (00000183) >>> TWALK fids 2,3 nwname 0d wname[0] (null)
    [   12.267038][  T183] 9pnet: (00000183) >>> size=17 type: 110 tag: 0
    [   12.267189][  T183] 9pnet: (00000183) <<< size=9 type: 111 tag: 0
    [   12.267300][  T183] 9pnet: (00000183) <<< RWALK nwqid 0:
    [   12.267368][  T183] 9pnet: (00000183) >>> TLOPEN fid 3 mode 98304
    [   12.267530][  T183] 9pnet: (00000183) >>> size=15 type: 12 tag: 0
    [   12.267779][  T183] 9pnet: (00000183) <<< size=24 type: 13 tag: 0
    [   12.267867][  T183] 9pnet: (00000183) <<< RLOPEN qid 80.6d10.6a53fe73 iounit 0
    Opened /tmp/9p/dir with fd 3
    [   12.268713][  T183] 9pnet: (00000183) >>> TWALK fids 2,4 nwname 1d wname[0] file
    [   12.268816][  T183] 9pnet: (00000183) >>> size=23 type: 110 tag: 0
    [   12.268961][  T183] 9pnet: (00000183) <<< size=22 type: 111 tag: 0
    [   12.269051][  T183] 9pnet: (00000183) <<< RWALK nwqid 1:
    [   12.269116][  T183] 9pnet: (00000183) <<<     [0] 0.6d11.6a53c273
    [   12.269198][  T183] 9pnet: (00000183) >>> TGETATTR fid 4, request_mask 6143
    [   12.269305][  T183] 9pnet: (00000183) >>> size=19 type: 24 tag: 0
    [   12.269454][  T183] 9pnet: (00000183) <<< size=160 type: 25 tag: 0
    [   12.269538][  T183] 9pnet: (00000183) <<< RGETATTR st_result_mask=6143
    [   12.269538][  T183] <<< qid=0.6d11.6a53c273
    [   12.269538][  T183] <<< st_mode=000081a0 st_nlink=1
    [   12.269538][  T183] <<< st_uid=1000 st_gid=1000
    [   12.269538][  T183] <<< st_rdev=0 st_size=0 st_blksize=131072 st_blocks=0
    [   12.269538][  T183] <<< st_atime_sec=1783874265 st_atime_nsec=602603686
    [   12.269538][  T183] <<< st_mtime_sec=1783874163 st_mtime_nsec=254338030
    [   12.269538][  T183] <<< st_ctime_sec=1783874163 st_ctime_nsec=254338030
    [   12.269538][  T183] <<< st_btime_sec=0 st_btime_nsec=0
    [   12.269538][  T183] <<< st_gen=0 st_data_version=0
    [   12.270149][  T183] 9pnet: (00000183) >>> TWALK fids 4,5 nwname 0d wname[0] (null)
    [   12.270249][  T183] 9pnet: (00000183) >>> size=17 type: 110 tag: 0
    [   12.270406][  T183] 9pnet: (00000183) <<< size=9 type: 111 tag: 0
    [   12.270497][  T183] 9pnet: (00000183) <<< RWALK nwqid 0:
    [   12.270561][  T183] 9pnet: (00000183) >>> TLOPEN fid 5 mode 32768
    [   12.270696][  T183] 9pnet: (00000183) >>> size=15 type: 12 tag: 0
    [   12.270879][  T183] 9pnet: (00000183) <<< size=24 type: 13 tag: 0
    [   12.270962][  T183] 9pnet: (00000183) <<< RLOPEN qid 0.6d11.6a53c273 iounit 20000
    Opened file with fd 4
    [   12.271278][  T183] 9pnet: (00000183) >>> TREAD fid 5 offset 0 255
    [   12.271393][  T183] 9pnet: (00000183) >>> size=23 type: 116 tag: 0
    [   12.271577][  T183] 9pnet: (00000183) <<< size=11 type: 117 tag: 0
    [   12.271667][  T183] 9pnet: (00000183) <<< RREAD count 0
    Read 0 bytes:

    [   12.271843][  T183] 9pnet: (00000183) >>> TCLUNK fid 5 (try 0)
    [   12.271978][  T183] 9pnet: (00000183) >>> size=11 type: 120 tag: 0
    [   12.272128][  T183] 9pnet: (00000183) <<< size=7 type: 121 tag: 0
    [   12.272212][  T183] 9pnet: (00000183) <<< RCLUNK fid 5
    [   12.272309][  T183] 9pnet: (00000183) >>> TCLUNK fid 4 (try 0)
    [   12.272407][  T183] 9pnet: (00000183) >>> size=11 type: 120 tag: 0
    [   12.272535][  T183] 9pnet: (00000183) <<< size=7 type: 121 tag: 0
    [   12.272618][  T183] 9pnet: (00000183) <<< RCLUNK fid 4
    Press Enter to openat again or Ctrl-C to exit

    (at this point dir is moved to dir1 on the host, then Enter is pressed here)

    [   32.441881][  T183] 9pnet: (00000183) >>> TWALK fids 2,4 nwname 1d wname[0] file
    [   32.442038][  T183] 9pnet: (00000183) >>> size=23 type: 110 tag: 0
    [   32.442330][  T183] 9pnet: (00000183) <<< size=11 type: 7 tag: 0
    [   32.442438][  T183] 9pnet: (00000183) <<< RLERROR (-2)
    openat: No such file or directory
    [   32.443619][  T183] 9pnet: (00000183) >>> TCLUNK fid 3 (try 0)
    [   32.443790][  T183] 9pnet: (00000183) >>> size=11 type: 120 tag: 0
    [   32.443942][  T183] 9pnet: (00000183) <<< size=7 type: 121 tag: 0
    [   32.444037][  T183] 9pnet: (00000183) <<< RCLUNK fid 3
    [   32.444127][  T183] 9pnet: (00000183) >>> TCLUNK fid 2 (try 0)
    [   32.444235][  T183] 9pnet: (00000183) >>> size=11 type: 120 tag: 0
    [   32.444364][  T183] 9pnet: (00000183) <<< size=7 type: 121 tag: 0
    [   32.444450][  T183] 9pnet: (00000183) <<< RCLUNK fid 2

In this case QEMU does hold a fd for the dir (since the dir is opened with O_RDONLY, causing a TLOPEN), but as Christian mentioned, it always starts the path walk from root, rather than using that fd.

QEMU git commit a759542a2c (today's master)

For the second case,
https://fileshare.maowtm.org/9pfs-landlock-fix/20250921/open_procselffd.c
reproduces it:

    (Host tmpfs exported as 9p to guest /tmp/9p)
    root@7-2-0-rc2-dev:/# ./open_procselffd /tmp/9p/dir/file
    [   24.243001][  T188] 9pnet: (00000188) >>> TWALK fids 1,2 nwname 1d wname[0] dir
    [   24.243305][  T188] 9pnet: (00000188) >>> size=22 type: 110 tag: 0
    [   24.243659][  T188] 9pnet: (00000188) <<< size=22 type: 111 tag: 0
    [   24.243855][  T188] 9pnet: (00000188) <<< RWALK nwqid 1:
    [   24.243983][  T188] 9pnet: (00000188) <<<     [0] 80.6d10.6a53fe73
    [   24.244124][  T188] 9pnet: (00000188) >>> TGETATTR fid 2, request_mask 6143
    [   24.244270][  T188] 9pnet: (00000188) >>> size=19 type: 24 tag: 0
    [   24.244460][  T188] 9pnet: (00000188) <<< size=160 type: 25 tag: 0
    [   24.244599][  T188] 9pnet: (00000188) <<< RGETATTR st_result_mask=6143
    [   24.244599][  T188] <<< qid=80.6d10.6a53fe73
    [   24.244599][  T188] <<< st_mode=000041e8 st_nlink=2
    [   24.244599][  T188] <<< st_uid=1000 st_gid=1000
    [   24.244599][  T188] <<< st_rdev=0 st_size=3c st_blksize=131072 st_blocks=0
    [   24.244599][  T188] <<< st_atime_sec=1783874216 st_atime_nsec=302475718
    [   24.244599][  T188] <<< st_mtime_sec=1783874163 st_mtime_nsec=254338030
    [   24.244599][  T188] <<< st_ctime_sec=1783875959 st_ctime_nsec=665487467
    [   24.244599][  T188] <<< st_btime_sec=0 st_btime_nsec=0
    [   24.244599][  T188] <<< st_gen=0 st_data_version=0
    [   24.245601][  T188] 9pnet: (00000188) >>> TWALK fids 2,3 nwname 1d wname[0] file
    [   24.245754][  T188] 9pnet: (00000188) >>> size=23 type: 110 tag: 0
    [   24.245936][  T188] 9pnet: (00000188) <<< size=22 type: 111 tag: 0
    [   24.246070][  T188] 9pnet: (00000188) <<< RWALK nwqid 1:
    [   24.246165][  T188] 9pnet: (00000188) <<<     [0] 0.6d11.6a53c273
    [   24.246287][  T188] 9pnet: (00000188) >>> TGETATTR fid 3, request_mask 6143
    [   24.246484][  T188] 9pnet: (00000188) >>> size=19 type: 24 tag: 0
    [   24.246661][  T188] 9pnet: (00000188) <<< size=160 type: 25 tag: 0
    [   24.246780][  T188] 9pnet: (00000188) <<< RGETATTR st_result_mask=6143
    [   24.246780][  T188] <<< qid=0.6d11.6a53c273
    [   24.246780][  T188] <<< st_mode=000081a0 st_nlink=1
    [   24.246780][  T188] <<< st_uid=1000 st_gid=1000
    [   24.246780][  T188] <<< st_rdev=0 st_size=0 st_blksize=131072 st_blocks=0
    [   24.246780][  T188] <<< st_atime_sec=1783875507 st_atime_nsec=502397563
    [   24.246780][  T188] <<< st_mtime_sec=1783874163 st_mtime_nsec=254338030
    [   24.246780][  T188] <<< st_ctime_sec=1783874163 st_ctime_nsec=254338030
    [   24.246780][  T188] <<< st_btime_sec=0 st_btime_nsec=0
    [   24.246780][  T188] <<< st_gen=0 st_data_version=0
    Opened /tmp/9p/dir/file with fd 3 (O_PATH)
    Press Enter to reopen

    (at this point dir/file is moved to dir/file2 on the host, then Enter is pressed here)
    (result is the same if instead dir is renamed)

    [   34.725233][  T188] 9pnet: (00000188) >>> TWALK fids 3,4 nwname 0d wname[0] (null)
    [   34.725619][  T188] 9pnet: (00000188) >>> size=17 type: 110 tag: 0
    [   34.726067][  T188] 9pnet: (00000188) <<< size=11 type: 7 tag: 0
    [   34.726241][  T188] 9pnet: (00000188) <<< RLERROR (-2)
    Failed to open /proc/self/fd/3: No such file or directory
    Press Enter to exit

With this case, there is no TLOPEN, only TWALK, so QEMU does not hold a fd for the file. (This means that, for Landlock, merely holding a fid is not enough to prevent inode number reuse on QEMU)

Quoted 8 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 4 lines

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.

I think there are two separate problems here - the server using inotify to "fixup" its own tracked path of each fids, and the client being notified of changes. With what Christian suggested, the above two cases will not break even without any client notification, since the client (in this case the kernel) already re-uses previously obtained fids to avoid redoing the path walk from root.

Having client notification might be useful for other cases tho. Aside from supporting things like inotify triggered from server-side notification, it also looks like if a fd is passed to another user, on access=user or access=client 9pfs will in fact re-do the path walk from root (v9fs_fid_lookup_with_uid), and so having notification could let us know what the expected path has changed to in order to correctly re-open a (stale) dentry that may have been moved on the host.

Quoted 7 lines
Quoted 4 lines

/Christian

Cheers,

I think overall there are still largely two directions, and I'm unclear which way is best, but some recent testing has made me realize that the second approach might be simpler than I thought:

- One is to continue with this path-based approach implemented in this RFC series. The main issue I can see of doing this is the somewhat unwieldy path tracking, and handling of parent renames (with the current implementation it would need to recursively update all child ino_paths). However, from a correctness point of view, inodeident=path proposed here really means that both the path, qid.path, and inode type has to match, and so the risk of "false inode matches" like the kind observed earlier, leading to the revert of 724a08450f74 ("fs/9p: simplify iget to remove unnecessary paths") and various related commits, should be low.

This also has the benefit of making i/fanotify on a cache=none 9pfs work again (for local changes), aside from Landlock.

- The other is to have special handling in Landlock: Landlock would key its rules on 9pfs based on some 9p-exposed identifier, such as the fhandle as proposed by Dominique. Through testing I realized that we could use st_gen to distinguish inodes with the same ino, and QEMU does expose the st_gen to the guest when the backing fs is something like ext4, so in fact this approach might be simplified (although I've only realized this today and probably need to try this out on more filesystems):

                                     v qid.path
                                       vvvvvvvv qid.version aka mtime
    [   78.617306][  T183] <<< qid=0.e.6a558647
    [   78.617306][  T183] <<< st_mode=000081a0 st_nlink=1
    [   78.617306][  T183] <<< st_uid=1000 st_gid=1000
    [   78.617306][  T183] <<< st_rdev=0 st_size=0 st_blksize=131072 st_blocks=0
    [   78.617306][  T183] <<< st_atime_sec=1783989831 st_atime_nsec=576593826
    [   78.617306][  T183] <<< st_mtime_sec=1783989831 st_mtime_nsec=576593826
    [   78.617306][  T183] <<< st_ctime_sec=1783989831 st_ctime_nsec=576593826
    [   78.617306][  T183] <<< st_btime_sec=0 st_btime_nsec=0
    [   78.617306][  T183] <<< st_gen=3634407333 st_data_version=0

After rm + touch:

                                     v qid.path
                                       vvvvvvvv qid.version aka mtime
    [  100.234598][  T184] <<< qid=0.e.6a558659
    [  100.234598][  T184] <<< st_mode=000081a0 st_nlink=1
    [  100.234598][  T184] <<< st_uid=1000 st_gid=1000
    [  100.234598][  T184] <<< st_rdev=0 st_size=0 st_blksize=131072 st_blocks=0
    [  100.234598][  T184] <<< st_atime_sec=1783989849 st_atime_nsec=5600077
    [  100.234598][  T184] <<< st_mtime_sec=1783989849 st_mtime_nsec=5600077
    [  100.234598][  T184] <<< st_ctime_sec=1783989849 st_ctime_nsec=5600077
    [  100.234598][  T184] <<< st_btime_sec=0 st_btime_nsec=0
    [  100.234598][  T184] <<< st_gen=4062134779 st_data_version=0
                                      ^ this is different

Previously I proposed that Landlock would need to hold a fid to keep the inode alive on the server side, to prevent inode number reuse when using qid.path as the rule key, but if the 9pfs-exposed fhandle includes this inode generation number, and Landlock uses that instead, then inode reuse is no longer an issue, without Landlock having to do anything special.

(we can't use qid.version since that is mtime, which will change on modification / files added or removed in a directory)

Does anyone have preferences / suggestions on whether to keep going with the path-tracking based approach (perhaps subject to figuring out a way to handle parent renames), or an approach where Landlock simply stores the fhandle (or (i_ino, i_generation (but this is u32 whereas st_gen within 9p is u64))) as the rule key?

If I find some time over the next couple of weeks I will try out the second approach since it might be turning out to be the simpler option, but the drawback is that it doesn't enable i/fanotify on 9pfs, which I originally hoped to achieve together with fixing Landlock on 9pfs.

Kind regards,
Tingmao

#25 lore raw

Tingmao Wang wrote on Tue, Jul 14, 2026 at 02:14:00AM +0100:

Quoted 5 lines

Does anyone have preferences / suggestions on whether to keep going with the path-tracking based approach (perhaps subject to figuring out a way to handle parent renames), or an approach where Landlock simply stores the fhandle (or (i_ino, i_generation (but this is u32 whereas st_gen within 9p is u64))) as the rule key?

st_gen comes from FS_IOC_GETVERSION which is exactly i_generation on at least ext4/btrfs/xfs, so it's not like that brings you anything (and it's also stored in the 9p inode's i_generation afaics?)

My understanding is that if the underlying filesystem supports it then yes mountpoint + i_ino + i_generation ought to be unique when combined together, but that leaves you with what to do with filesystems that don't support it (at least tmpfs?) and 9p mounts sharing multiple filesystems (even if qemu has been warning about it for a while and that brings its own share of bugs around cache)

If you're fine not supporting them, I think that definitely looks more promising, but otherwise you don't have much choice

Either way I'm afraid I don't have time to look much into it (I'm afraid I have to admit I didn't read your full mail, and I have to get up in 4 hours so I should probably stop processing this 9p backlog...), and it doesn't look like Greg is much more reactive, so you're on your own and I'm really sorry about it :/
I wish you best though!

Quoted 4 lines

If I find some time over the next couple of weeks I will try out the second approach since it might be turning out to be the simpler option, but the drawback is that it doesn't enable i/fanotify on 9pfs, which I originally hoped to achieve together with fixing Landlock on 9pfs.

(inotify/fanotify would need a protocol overhaul to get "right" and is a much bigger can of works, because if you get it to work locally then you'll get people to expect the server to notify clients of remote changes (some network filesystems can do it with leases, but 9p doesn't have any such mechanism);
I can understand it is useful even if it only works within the client local changes but I'm not sure I want to push much here either.)

Signature · 1 line
Domnique
#26 lore raw
In reply to Tingmao Wang
Quoted 1 line · CEST Tingmao Wang

On 9/16/25 00:31, Dominique Martinet wrote:

[...]

Quoted 24 lines
Quoted 9 lines
Quoted 8 lines

I tried mounting a qemu-exported 9pfs backed on ext4, with multidevs=remap, and created a file, used stat to note its inode number, deleted the file, created another file (of the same OR different name), and that new file will have the same inode number.

(If I don't delete the file, then a newly created file would of course have a different ext4 inode number, and in that case QEMU exposes a different qid)

Quoted 9 lines

Ok so from Christian's reply this is just ext4 reusing the same inode.. I briefly hinted at this above, but in this case ext4 will give the inode a different generation number (so the ext4 file handle will be different, and accessing the old one will get ESTALE); but that's not something qemu currently tracks and it'd be a bit of an overhaul... In theory qemu could hash mount_id + file handle to get a properly unique qid, if we need to improve that, but that'd be limited to root users (and to filesystems that support name_to_handle_at) so I don't think it's really appropriate either... hmm..

Actually I think I forgot that there is also qid.version, which in the case of a QEMU-exported 9pfs might just be the file modification time? In 9pfs currently we do reject a inode match if that version changed server side in cached mode:

Quoted 7 lines

v9fs_test_inode_dotl:

	/* compare qid details */
	if (memcmp(&v9inode->qid.version,
		   &st->qid.version, sizeof(v9inode->qid.version)))
		return 0;

(not tested whether QEMU correctly sets this version yet)

Define "correctly". ;-) QEMU sets it like this since 2010:

  qidp->version = stbuf->st_mtime ^ (stbuf->st_size << 8);

https://github.com/qemu/qemu/blob/190d5d7fd725ff754f94e8e0cbfb69f279c82b5d/hw/9pfs/9p.c#L1020

/Christian

#27 lore raw
In reply to Dominique Martinet

On Monday, September 15, 2025 2:53:14 PM CEST Dominique Martinet wrote:
[...]

Quoted 8 lines
Quoted 7 lines

1. The qid is 9pfs internal data, and we may need extra API for 9pfs to

expose this to Landlock. On 64bit, this is easy as it's just the inode number (offset by 2), which we can already get from the struct inode. But perhaps on 32bit we need a way to expose the full 64bit server-sent qid to Landlock (or other kernel subsystems), if we're going to do this.

Quoted 7 lines

I'm not sure how much effort we want to spend on 32bit: as far as I know, if we have inode number collision on 32 bit we're already in trouble (tools like tar will consider such files to be hardlink of each other and happily skip reading data, producing corrupted archives); this is not a happy state but I don't know how to do better in any reasonable way, so we can probably keep a similar limitation for 32bit and use inode number directly...

I agree, on 32-bit the game is lost.

One way that would come to my mind though: exposing the full qid path as xattr on 32-bit, e.g. via "system.9pfs_qid" or something like that.

Quoted 11 lines
Quoted 5 lines

2. Even though qids are supposed to be unique across the lifetime of a

filesystem (including deleted files), this is not the case even for QEMU in multidevs=remap mode, when running on ext4, as tested on QEMU 10.1.0.

I'm not familiar with the qid remap implementation in qemu, but I'm curious in what case you hit that.
Deleting and recreating files? Or as you seem to say below the 'qid' is "freed" when fd is closed qemu-side and re-used by later open of other files?

The inode remap algorithm in QEMU's 9p server was designed to prevent inode number collisions of equally numbered inodes of *different* *devices* on host, exposed to guest via the same 9p mount (which appears as only one 9pfs device on guest). Basis for this however is still the underlying filesystem's inode number on host.

So yes, ext4 re-uses inode numbers of deleted files, and when that happens, a new file appears with the same qid path as the previously deleted file with QEMU.

/Christian

#28 lore raw
Quoted 18 lines · Christian Schoenebeck

On Monday, September 15, 2025 2:53:14 PM CEST Dominique Martinet wrote:
[...]

Quoted 8 lines
Quoted 7 lines

1. The qid is 9pfs internal data, and we may need extra API for 9pfs to

expose this to Landlock. On 64bit, this is easy as it's just the inode number (offset by 2), which we can already get from the struct inode. But perhaps on 32bit we need a way to expose the full 64bit server-sent qid to Landlock (or other kernel subsystems), if we're going to do this.

Quoted 7 lines

I'm not sure how much effort we want to spend on 32bit: as far as I know, if we have inode number collision on 32 bit we're already in trouble (tools like tar will consider such files to be hardlink of each other and happily skip reading data, producing corrupted archives); this is not a happy state but I don't know how to do better in any reasonable way, so we can probably keep a similar limitation for 32bit and use inode number directly...

Quoted 4 lines

I agree, on 32-bit the game is lost.

One way that would come to my mind though: exposing the full qid path as xattr on 32-bit, e.g. via "system.9pfs_qid" or something like that.

Another way to always deal with 64-bit values, even on 32-bit architectures, would be to implement inode->i_op->getattr(), but that could have side effects for 9p users expecting the current behavior.

Quoted 27 lines
Quoted 7 lines
Quoted 6 lines

2. Even though qids are supposed to be unique across the lifetime of a

filesystem (including deleted files), this is not the case even for QEMU in multidevs=remap mode, when running on ext4, as tested on QEMU 10.1.0.

Quoted 5 lines

I'm not familiar with the qid remap implementation in qemu, but I'm curious in what case you hit that.
Deleting and recreating files? Or as you seem to say below the 'qid' is "freed" when fd is closed qemu-side and re-used by later open of other files?

The inode remap algorithm in QEMU's 9p server was designed to prevent inode number collisions of equally numbered inodes of *different* *devices* on host, exposed to guest via the same 9p mount (which appears as only one 9pfs device on guest). Basis for this however is still the underlying filesystem's inode number on host.

So yes, ext4 re-uses inode numbers of deleted files, and when that happens, a new file appears with the same qid path as the previously deleted file with QEMU.

/Christian