[PATCH] fuse: set dentry epoch before making the dentry visible

[PATCH] fuse: set dentry epoch before making the dentry visible

[Leon Ming]

fuse_lookup(), create_new_entry(), fuse_create_open() and
fuse_direntplus_link() all record the connection epoch in the new dentry
only after d_splice_alias() or d_instantiate() has published it.  A new
dentry starts with an epoch of zero (fuse_dentry_init()), and the
connection epoch starts at one, so in that window the dentry is hashed
but fails the epoch check in fuse_dentry_revalidate().

d_splice_alias() hashes the dentry and wakes the lookups waiting on it
in d_alloc_parallel().  A waiter, or a concurrent lookup_fast() that
finds the dentry in the hash, calls ->d_revalidate(), gets 0 and
d_invalidate()s the dentry the original lookup is about to return.  The
caller still gets a valid dentry, but it is now unhashed.  One visible
result is that readlink() of /proc/self/fd/N for an O_PATH fd opened on
that path returns "<path> (deleted)" for a file that exists.

This shows up when many threads look up the same uncached path at once.
TypeScript's native compiler (tsgo) resolves real paths with
open(O_PATH) + readlink(/proc/self/fd/N), and when it type-checks a
project in a FUSE-backed checkout it intermittently caches the
"(deleted)" path and fails to resolve a package that is installed.

Before commit 5a6baf204610 ("fuse: fix uninit-value in
fuse_dentry_revalidate()") initialised the epoch to zero, it held
whatever the slab left there, so the check usually passed by accident.

Set the epoch on the new dentry before it is published.  If
d_splice_alias() returns an existing alias, keep setting the epoch on
that dentry afterwards, as before.

Reproducer: N processes start together and each run open(O_PATH) +
readlink(/proc/self/fd/N) over the same list of files in a freshly
mounted FUSE filesystem, counting results that end in " (deleted)".
On this tree under UML (16 CPUs, squashfuse_ll, 6000 files, 32
processes, a fresh mount per round):

  before: 3519 of 10.75M lookups "(deleted)", in 51 of 56 rounds
  after:  0 of 10.75M lookups

On v7.1-based production kernels with EdenFS the same test returned
"(deleted)" for 5.5% of lookups.

Fixes: 2396356a945b ("fuse: add more control over cache invalidation behaviour")
Cc: stable@vger.kernel.org
Signed-off-by: Leon Ming <leonming@meta.com>
---
 fs/fuse/dir.c     | 16 ++++++++++++----
 fs/fuse/readdir.c |  1 +
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 28516a1760e..b7af3f5a3bb 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -644,13 +644,21 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
 	if (inode && get_node_id(inode) == FUSE_ROOT_ID)
 		goto out_iput;
 
+	/*
+	 * Set the epoch before d_splice_alias() makes the dentry visible.
+	 * Otherwise a concurrent lookup can find it with an epoch of zero, fail
+	 * fuse_dentry_revalidate() and invalidate it under us.
+	 */
+	fuse_dentry_set_epoch(entry, epoch);
 	newent = d_splice_alias(inode, entry);
 	err = PTR_ERR(newent);
 	if (IS_ERR(newent))
 		goto out_err;
 
-	entry = newent ? newent : entry;
-	fuse_dentry_set_epoch(entry, epoch);
+	if (newent) {
+		entry = newent;
+		fuse_dentry_set_epoch(entry, epoch);
+	}
 	if (outarg_valid)
 		fuse_change_entry_timeout(entry, &outarg);
 	else
@@ -907,8 +915,8 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
 		goto out_err;
 	}
 	kfree(forget);
-	d_instantiate(entry, inode);
 	fuse_dentry_set_epoch(entry, epoch);
+	d_instantiate(entry, inode);
 	fuse_change_entry_timeout(entry, &outentry);
 	fuse_dir_changed(dir);
 	err = generic_file_open(inode, file);
@@ -1030,6 +1038,7 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
 		fuse_chan_queue_forget(fm->fc->chan, forget, outarg.nodeid, 1);
 		return ERR_PTR(-ENOMEM);
 	}
+	fuse_dentry_set_epoch(entry, epoch);
 	kfree(forget);
 
 	d = d_splice_alias(inode, entry);
@@ -1040,7 +1049,6 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
 		fuse_dentry_set_epoch(d, epoch);
 		fuse_change_entry_timeout(d, &outarg);
 	} else {
-		fuse_dentry_set_epoch(entry, epoch);
 		fuse_change_entry_timeout(entry, &outarg);
 	}
 	fuse_dir_changed(dir);
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index 83a1cc46fe4..ffce4031892 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -239,6 +239,7 @@ static int fuse_direntplus_link(struct file *file,
 		if (!inode)
 			inode = ERR_PTR(-ENOMEM);
 
+		fuse_dentry_set_epoch(dentry, epoch);
 		alias = d_splice_alias(inode, dentry);
 		d_lookup_done(dentry);
 		if (alias) {

base-commit: 416fd13cb4b789b78304ef1b8579125106ba3ffc
-- 
2.53.0-Meta