fusefs: correctly handle an inode that changes file types
Correctly handle the situation where a FUSE server unlinks a file, then creates a new file of a different type but with the same inode number. Previously fuse_vnop_lookup in this situation would return EAGAIN. But since it didn't call vgone(), the vnode couldn't be reused right away. Fix this by immediately calling vgone() and reallocating a new vnode. This problem can occur in three code paths, during VOP_LOOKUP, VOP_SETATTR, or following FUSE_GETATTR, which usually happens during VOP_GETATTR but can occur during other vops, too. Note that the correct response actually doesn't depend on whether the entry cache has expired. In fact, during VOP_LOOKUP, we can't even tell. Either it has expired already, or else the vnode got reclaimed by vnlru. Also, correct the error code during the VOP_SETATTR path. PR: 258022 Reported by: chogata@moosefs.pro MFC after: 2 weeks Reviewed by: pfg Differential Revision: https://reviews.freebsd.org/D33283
This commit is contained in:
parent
169b368a62
commit
25927e068f
@ -1255,12 +1255,15 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
|
|||||||
* STALE vnode, ditch
|
* STALE vnode, ditch
|
||||||
*
|
*
|
||||||
* The vnode has changed its type "behind our back".
|
* The vnode has changed its type "behind our back".
|
||||||
|
* This probably means that the file got deleted and
|
||||||
|
* recreated on the server, with the same inode.
|
||||||
* There's nothing really we can do, so let us just
|
* There's nothing really we can do, so let us just
|
||||||
* force an internal revocation and tell the caller to
|
* return ENOENT. After all, the entry must not have
|
||||||
* try again, if interested.
|
* existed in the recent past. If the user tries
|
||||||
|
* again, it will work.
|
||||||
*/
|
*/
|
||||||
fuse_internal_vnode_disappear(vp);
|
fuse_internal_vnode_disappear(vp);
|
||||||
err = EAGAIN;
|
err = ENOENT;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (err == 0) {
|
if (err == 0) {
|
||||||
|
@ -213,24 +213,27 @@ fuse_vnode_alloc(struct mount *mp,
|
|||||||
return (err);
|
return (err);
|
||||||
|
|
||||||
if (*vpp) {
|
if (*vpp) {
|
||||||
if ((*vpp)->v_type != vtyp) {
|
if ((*vpp)->v_type == vtyp) {
|
||||||
|
/* Reuse a vnode that hasn't yet been reclaimed */
|
||||||
|
MPASS((*vpp)->v_data != NULL);
|
||||||
|
MPASS(VTOFUD(*vpp)->nid == nodeid);
|
||||||
|
SDT_PROBE2(fusefs, , node, trace, 1,
|
||||||
|
"vnode taken from hash");
|
||||||
|
return (0);
|
||||||
|
} else {
|
||||||
/*
|
/*
|
||||||
* STALE vnode! This probably indicates a buggy
|
* The inode changed types! If we get here, we can't
|
||||||
* server, but it could also be the result of a race
|
* tell whether the inode's entry cache had expired
|
||||||
* between FUSE_LOOKUP and another client's
|
* yet. So this could be the result of a buggy server,
|
||||||
* FUSE_UNLINK/FUSE_CREATE
|
* but more likely the server just reused an inode
|
||||||
|
* number following an entry cache expiration.
|
||||||
*/
|
*/
|
||||||
SDT_PROBE3(fusefs, , node, stale_vnode, *vpp, vtyp,
|
SDT_PROBE3(fusefs, , node, stale_vnode, *vpp, vtyp,
|
||||||
nodeid);
|
nodeid);
|
||||||
fuse_internal_vnode_disappear(*vpp);
|
fuse_internal_vnode_disappear(*vpp);
|
||||||
|
vgone(*vpp);
|
||||||
lockmgr((*vpp)->v_vnlock, LK_RELEASE, NULL);
|
lockmgr((*vpp)->v_vnlock, LK_RELEASE, NULL);
|
||||||
*vpp = NULL;
|
|
||||||
return (EAGAIN);
|
|
||||||
}
|
}
|
||||||
MPASS((*vpp)->v_data != NULL);
|
|
||||||
MPASS(VTOFUD(*vpp)->nid == nodeid);
|
|
||||||
SDT_PROBE2(fusefs, , node, trace, 1, "vnode taken from hash");
|
|
||||||
return (0);
|
|
||||||
}
|
}
|
||||||
fvdat = malloc(sizeof(*fvdat), M_FUSEVN, M_WAITOK | M_ZERO);
|
fvdat = malloc(sizeof(*fvdat), M_FUSEVN, M_WAITOK | M_ZERO);
|
||||||
switch (vtyp) {
|
switch (vtyp) {
|
||||||
|
@ -256,6 +256,56 @@ TEST_F(Getattr, ok)
|
|||||||
//FUSE can't set st_blksize until protocol 7.9
|
//FUSE can't set st_blksize until protocol 7.9
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* FUSE_GETATTR returns a different file type, even though the entry cache
|
||||||
|
* hasn't expired. This is a server bug! It probably means that the server
|
||||||
|
* removed the file and recreated it with the same inode but a different vtyp.
|
||||||
|
* The best thing fusefs can do is return ENOENT to the caller. After all, the
|
||||||
|
* entry must not have existed recently.
|
||||||
|
*/
|
||||||
|
TEST_F(Getattr, vtyp_conflict)
|
||||||
|
{
|
||||||
|
const char FULLPATH[] = "mountpoint/some_file.txt";
|
||||||
|
const char RELPATH[] = "some_file.txt";
|
||||||
|
const uint64_t ino = 42;
|
||||||
|
struct stat sb;
|
||||||
|
sem_t sem;
|
||||||
|
|
||||||
|
ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno);
|
||||||
|
|
||||||
|
EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
|
||||||
|
.WillOnce(Invoke(
|
||||||
|
ReturnImmediate([=](auto in __unused, auto& out) {
|
||||||
|
SET_OUT_HEADER_LEN(out, entry);
|
||||||
|
out.body.entry.attr.mode = S_IFREG | 0644;
|
||||||
|
out.body.entry.nodeid = ino;
|
||||||
|
out.body.entry.attr.nlink = 1;
|
||||||
|
out.body.entry.attr_valid = 0;
|
||||||
|
out.body.entry.entry_valid = UINT64_MAX;
|
||||||
|
})));
|
||||||
|
EXPECT_CALL(*m_mock, process(
|
||||||
|
ResultOf([](auto in) {
|
||||||
|
return (in.header.opcode == FUSE_GETATTR &&
|
||||||
|
in.body.getattr.getattr_flags == 0 &&
|
||||||
|
in.header.nodeid == ino);
|
||||||
|
}, Eq(true)),
|
||||||
|
_)
|
||||||
|
).WillOnce(Invoke(ReturnImmediate([](auto i __unused, auto& out) {
|
||||||
|
SET_OUT_HEADER_LEN(out, attr);
|
||||||
|
out.body.attr.attr.ino = ino; // Must match nodeid
|
||||||
|
out.body.attr.attr.mode = S_IFDIR | 0755; // Changed!
|
||||||
|
out.body.attr.attr.nlink = 2;
|
||||||
|
})));
|
||||||
|
// We should reclaim stale vnodes
|
||||||
|
expect_forget(ino, 1, &sem);
|
||||||
|
|
||||||
|
ASSERT_NE(0, stat(FULLPATH, &sb));
|
||||||
|
EXPECT_EQ(errno, ENOENT);
|
||||||
|
|
||||||
|
sem_wait(&sem);
|
||||||
|
sem_destroy(&sem);
|
||||||
|
}
|
||||||
|
|
||||||
TEST_F(Getattr_7_8, ok)
|
TEST_F(Getattr_7_8, ok)
|
||||||
{
|
{
|
||||||
const char FULLPATH[] = "mountpoint/some_file.txt";
|
const char FULLPATH[] = "mountpoint/some_file.txt";
|
||||||
|
@ -343,8 +343,9 @@ TEST_F(Lookup, subdir)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* The server returns two different vtypes for the same nodeid. This is a bad
|
* The server returns two different vtypes for the same nodeid. This is
|
||||||
* server! But we shouldn't crash.
|
* technically allowed if the entry's cache has already expired.
|
||||||
|
* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=258022
|
||||||
*/
|
*/
|
||||||
TEST_F(Lookup, vtype_conflict)
|
TEST_F(Lookup, vtype_conflict)
|
||||||
{
|
{
|
||||||
@ -354,12 +355,29 @@ TEST_F(Lookup, vtype_conflict)
|
|||||||
const char SECONDRELPATH[] = "bar";
|
const char SECONDRELPATH[] = "bar";
|
||||||
uint64_t ino = 42;
|
uint64_t ino = 42;
|
||||||
|
|
||||||
expect_lookup(FIRSTRELPATH, ino, S_IFREG | 0644, 0, 1, UINT64_MAX);
|
EXPECT_LOOKUP(FUSE_ROOT_ID, FIRSTRELPATH)
|
||||||
expect_lookup(SECONDRELPATH, ino, S_IFDIR | 0755, 0, 1, UINT64_MAX);
|
.WillOnce(Invoke(
|
||||||
|
ReturnImmediate([=](auto in __unused, auto& out) {
|
||||||
|
SET_OUT_HEADER_LEN(out, entry);
|
||||||
|
out.body.entry.attr.mode = S_IFDIR | 0644;
|
||||||
|
out.body.entry.nodeid = ino;
|
||||||
|
out.body.entry.attr.nlink = 1;
|
||||||
|
})));
|
||||||
|
expect_lookup(SECONDRELPATH, ino, S_IFREG | 0755, 0, 1, UINT64_MAX);
|
||||||
|
// VOP_FORGET happens asynchronously, so it may or may not arrive
|
||||||
|
// before the test completes.
|
||||||
|
EXPECT_CALL(*m_mock, process(
|
||||||
|
ResultOf([=](auto in) {
|
||||||
|
return (in.header.opcode == FUSE_FORGET &&
|
||||||
|
in.header.nodeid == ino &&
|
||||||
|
in.body.forget.nlookup == 1);
|
||||||
|
}, Eq(true)),
|
||||||
|
_)
|
||||||
|
).Times(AtMost(1))
|
||||||
|
.WillOnce(Invoke([=](auto in __unused, auto &out __unused) { }));
|
||||||
|
|
||||||
ASSERT_EQ(0, access(FIRSTFULLPATH, F_OK)) << strerror(errno);
|
ASSERT_EQ(0, access(FIRSTFULLPATH, F_OK)) << strerror(errno);
|
||||||
ASSERT_EQ(-1, access(SECONDFULLPATH, F_OK));
|
EXPECT_EQ(0, access(SECONDFULLPATH, F_OK)) << strerror(errno);
|
||||||
ASSERT_EQ(EAGAIN, errno);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST_F(Lookup_7_8, ok)
|
TEST_F(Lookup_7_8, ok)
|
||||||
|
@ -724,6 +724,53 @@ TEST_F(Setattr, utimensat_utime_now) {
|
|||||||
EXPECT_EQ(now[1].tv_nsec, sb.st_mtim.tv_nsec);
|
EXPECT_EQ(now[1].tv_nsec, sb.st_mtim.tv_nsec);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* FUSE_SETATTR returns a different file type, even though the entry cache
|
||||||
|
* hasn't expired. This is a server bug! It probably means that the server
|
||||||
|
* removed the file and recreated it with the same inode but a different vtyp.
|
||||||
|
* The best thing fusefs can do is return ENOENT to the caller. After all, the
|
||||||
|
* entry must not have existed recently.
|
||||||
|
*/
|
||||||
|
TEST_F(Setattr, vtyp_conflict)
|
||||||
|
{
|
||||||
|
const char FULLPATH[] = "mountpoint/some_file.txt";
|
||||||
|
const char RELPATH[] = "some_file.txt";
|
||||||
|
const uint64_t ino = 42;
|
||||||
|
uid_t newuser = 12345;
|
||||||
|
sem_t sem;
|
||||||
|
|
||||||
|
ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno);
|
||||||
|
|
||||||
|
EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
|
||||||
|
.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
|
||||||
|
SET_OUT_HEADER_LEN(out, entry);
|
||||||
|
out.body.entry.attr.mode = S_IFREG | 0777;
|
||||||
|
out.body.entry.nodeid = ino;
|
||||||
|
out.body.entry.entry_valid = UINT64_MAX;
|
||||||
|
})));
|
||||||
|
|
||||||
|
EXPECT_CALL(*m_mock, process(
|
||||||
|
ResultOf([](auto in) {
|
||||||
|
return (in.header.opcode == FUSE_SETATTR &&
|
||||||
|
in.header.nodeid == ino);
|
||||||
|
}, Eq(true)),
|
||||||
|
_)
|
||||||
|
).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
|
||||||
|
SET_OUT_HEADER_LEN(out, attr);
|
||||||
|
out.body.attr.attr.ino = ino;
|
||||||
|
out.body.attr.attr.mode = S_IFDIR | 0777; // Changed!
|
||||||
|
out.body.attr.attr.uid = newuser;
|
||||||
|
})));
|
||||||
|
// We should reclaim stale vnodes
|
||||||
|
expect_forget(ino, 1, &sem);
|
||||||
|
|
||||||
|
EXPECT_NE(0, chown(FULLPATH, newuser, -1));
|
||||||
|
EXPECT_EQ(ENOENT, errno);
|
||||||
|
|
||||||
|
sem_wait(&sem);
|
||||||
|
sem_destroy(&sem);
|
||||||
|
}
|
||||||
|
|
||||||
/* On a read-only mount, no attributes may be changed */
|
/* On a read-only mount, no attributes may be changed */
|
||||||
TEST_F(RofsSetattr, erofs)
|
TEST_F(RofsSetattr, erofs)
|
||||||
{
|
{
|
||||||
|
Loading…
Reference in New Issue
Block a user