git: 32273253667b - main - fusefs: fix two bugs regarding VOP_RECLAIM of the root inode

From: Alan Somers <asomers_at_FreeBSD.org>
Date: Wed, 06 Apr 2022 22:17:22 UTC
The branch main has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=32273253667b941c376cf08383006b3a0cbc5ca2

commit 32273253667b941c376cf08383006b3a0cbc5ca2
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-04-02 19:31:24 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-04-06 22:16:52 +0000

    fusefs: fix two bugs regarding VOP_RECLAIM of the root inode
    
    * We never send FUSE_LOOKUP for the root inode, since its inode number
      is hard-coded to 1.  Therefore, we should not send FUSE_FORGET for it,
      lest the server see its lookup count fall below 0.
    
    * During VOP_RECLAIM, if we are reclaiming the root inode, we must clear
      the file system's vroot pointer.  Otherwise it will be left pointing
      at a reclaimed vnode, which will cause future VOP_LOOKUP operations to
      fail.  Previously we only cleared that pointer during VFS_UMOUNT.  I
      don't know of any real-world way to trigger this bug.
    
    MFC after:      2 weeks
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D34753
---
 sys/fs/fuse/fuse_vnops.c             | 19 ++++++++++++++++++-
 tests/sys/fs/fusefs/destroy.cc       |  1 -
 tests/sys/fs/fusefs/dev_fuse_poll.cc |  1 -
 tests/sys/fs/fusefs/forget.cc        | 33 +++++++++++++++++++++++++++++++++
 tests/sys/fs/fusefs/lookup.cc        |  2 --
 5 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index acded47f75a3..4374c854a5be 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -1980,7 +1980,24 @@ fuse_vnop_reclaim(struct vop_reclaim_args *ap)
 		fuse_filehandle_close(vp, fufh, td, NULL);
 	}
 
-	if (!fuse_isdeadfs(vp) && fvdat->nlookup > 0) {
+	if (VTOI(vp) == 1) {
+		/*
+		 * Don't send FUSE_FORGET for the root inode, because
+		 * we never send FUSE_LOOKUP for it (see
+		 * fuse_vfsop_root) and we don't want the server to see
+		 * mismatched lookup counts.
+		 */
+		struct fuse_data *data;
+		struct vnode *vroot;
+
+		data = fuse_get_mpdata(vnode_mount(vp));
+		FUSE_LOCK();
+		vroot = data->vroot;
+		data->vroot = NULL;
+		FUSE_UNLOCK();
+		if (vroot)
+			vrele(vroot);
+	} else if (!fuse_isdeadfs(vp) && fvdat->nlookup > 0) {
 		fuse_internal_forget_send(vnode_mount(vp), td, NULL, VTOI(vp),
 		    fvdat->nlookup);
 	}
diff --git a/tests/sys/fs/fusefs/destroy.cc b/tests/sys/fs/fusefs/destroy.cc
index e72464382d4e..91c22ba6975e 100644
--- a/tests/sys/fs/fusefs/destroy.cc
+++ b/tests/sys/fs/fusefs/destroy.cc
@@ -141,7 +141,6 @@ TEST_F(Destroy, ok)
 	uint64_t ino = 42;
 
 	expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 2);
-	expect_forget(FUSE_ROOT_ID, 1);
 	expect_forget(ino, 2);
 	expect_destroy(0);
 
diff --git a/tests/sys/fs/fusefs/dev_fuse_poll.cc b/tests/sys/fs/fusefs/dev_fuse_poll.cc
index 951777701e1e..5ac66a915ea5 100644
--- a/tests/sys/fs/fusefs/dev_fuse_poll.cc
+++ b/tests/sys/fs/fusefs/dev_fuse_poll.cc
@@ -93,7 +93,6 @@ TEST_P(DevFusePoll, access)
 /* Ensure that we wake up pollers during unmount */
 TEST_P(DevFusePoll, destroy)
 {
-	expect_forget(FUSE_ROOT_ID, 1);
 	expect_destroy(0);
 
 	m_mock->unmount();
diff --git a/tests/sys/fs/fusefs/forget.cc b/tests/sys/fs/fusefs/forget.cc
index 84fc271df57c..d520ba84dbdd 100644
--- a/tests/sys/fs/fusefs/forget.cc
+++ b/tests/sys/fs/fusefs/forget.cc
@@ -32,6 +32,7 @@
 
 extern "C" {
 #include <sys/types.h>
+#include <sys/mount.h>
 #include <sys/sysctl.h>
 
 #include <fcntl.h>
@@ -145,3 +146,35 @@ TEST_F(Forget, invalidate_names)
 	/* Access the file again, causing another lookup */
 	ASSERT_EQ(0, access(FULLFPATH, F_OK)) << strerror(errno);
 }
+
+/*
+ * Reclaiming the root inode should not send a FUSE_FORGET request, nor should
+ * it interfere with further lookup operations.
+ */
+TEST_F(Forget, root)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	mode_t mode = S_IFREG | 0755;
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.WillRepeatedly(Invoke(
+		ReturnImmediate([=](auto in __unused, auto& out) {
+		SET_OUT_HEADER_LEN(out, entry);
+		out.body.entry.attr.mode = mode;
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr.nlink = 1;
+		out.body.entry.attr_valid = UINT64_MAX;
+		out.body.entry.entry_valid = UINT64_MAX;
+	})));
+
+	/* access(2) the file to force a lookup. */
+	ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
+
+	reclaim_vnode("mountpoint");
+	nap();
+
+	/* Access it again, to make sure it's still possible. */
+	ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
+}
diff --git a/tests/sys/fs/fusefs/lookup.cc b/tests/sys/fs/fusefs/lookup.cc
index 2dfa10730ec8..32e2a08eb949 100644
--- a/tests/sys/fs/fusefs/lookup.cc
+++ b/tests/sys/fs/fusefs/lookup.cc
@@ -263,7 +263,6 @@ TEST_F(Lookup, dotdot_no_parent_nid)
 	).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
 		SET_OUT_HEADER_LEN(out, open);
 	})));
-	expect_forget(FUSE_ROOT_ID, 1, NULL);
 	expect_forget(foo_ino, 1, NULL);
 
 	fd = open("mountpoint/foo/bar", O_EXEC| O_DIRECTORY);
@@ -574,7 +573,6 @@ TEST_F(LookupExportable, dotdot_no_parent_nid)
 	).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
 		SET_OUT_HEADER_LEN(out, open);
 	})));
-	expect_forget(FUSE_ROOT_ID, 1, NULL);
 	expect_forget(foo_ino, 1, NULL);
 	EXPECT_LOOKUP(bar_ino, "..")
 	.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {