The branch main has been updated by asomers:
URL:
https://cgit.FreeBSD.org/src/commit/?id=32273253667b941c376cf08383006b3a0cbc5ca2
commit 32273253667b941c376cf08383006b3a0cbc5ca2
Author: Alan Somers <asom...@freebsd.org>
AuthorDate: 2022-04-02 19:31:24 +0000
Commit: Alan Somers <asom...@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) {
