From 3fa127896b72f2b1e9855979ce4b0300cb6d13e9 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Mon, 6 May 2019 20:46:58 +0000 Subject: [PATCH] fusefs: allow ftruncate on files without write permission ftruncate should succeed as long as the file descriptor is writable, even if the file doesn't have write permission. This is important when combined with O_CREAT. Reported by: pjdfstest Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_internal.c | 3 + sys/fs/fuse/fuse_vnops.c | 2 +- tests/sys/fs/fusefs/default_permissions.cc | 79 +++++++++++++++------- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index a5a55ef4eb49..8676d94455d7 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -128,6 +128,9 @@ fuse_internal_access(struct vnode *vp, data = fuse_get_mpdata(mp); dataflags = data->dataflags; + if (mode == 0) + return 0; + if (mode & VMODIFY_PERMS && vfs_isrdonly(mp)) { switch (vp->v_type) { case VDIR: diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 554167b42d2f..07059ba23555 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -1574,7 +1574,7 @@ fuse_vnop_setattr(struct vop_setattr_args *ap) */ return (0); } - accmode |= VWRITE; + /* Don't set accmode. Permission to trunc is checked upstack */ } /* * TODO: for atime and mtime, only require VWRITE if UTIMENS_NULL is diff --git a/tests/sys/fs/fusefs/default_permissions.cc b/tests/sys/fs/fusefs/default_permissions.cc index 78809af24da7..a538bae237ea 100644 --- a/tests/sys/fs/fusefs/default_permissions.cc +++ b/tests/sys/fs/fusefs/default_permissions.cc @@ -86,6 +86,25 @@ void expect_chmod(uint64_t ino, mode_t mode) }))); } +void expect_create(const char *relpath, uint64_t ino) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + const char *name = (const char*)in->body.bytes + + sizeof(fuse_open_in); + return (in->header.opcode == FUSE_CREATE && + (0 == strcmp(relpath, name))); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, create); + out->body.create.entry.attr.mode = S_IFREG | 0644; + out->body.create.entry.nodeid = ino; + out->body.create.entry.entry_valid = UINT64_MAX; + out->body.create.entry.attr_valid = UINT64_MAX; + }))); +} + void expect_getattr(uint64_t ino, mode_t mode, uint64_t attr_valid, int times, uid_t uid = 0, gid_t gid = 0) { @@ -129,29 +148,7 @@ class Write: public DefaultPermissions {}; * rename vops (they all share a common path for permission checks in * VOP_LOOKUP) */ -class Create: public DefaultPermissions { -public: - -void expect_create(const char *relpath, uint64_t ino) -{ - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - const char *name = (const char*)in->body.bytes + - sizeof(fuse_open_in); - return (in->header.opcode == FUSE_CREATE && - (0 == strcmp(relpath, name))); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { - SET_OUT_HEADER_LEN(out, create); - out->body.create.entry.attr.mode = S_IFREG | 0644; - out->body.create.entry.nodeid = ino; - out->body.create.entry.entry_valid = UINT64_MAX; - out->body.create.entry.attr_valid = UINT64_MAX; - }))); -} - -}; +class Create: public DefaultPermissions {}; class Deleteextattr: public DefaultPermissions { public: @@ -797,6 +794,42 @@ TEST_F(Setattr, eacces) EXPECT_EQ(EPERM, errno); } +/* + * ftruncate() of a file without writable permissions should succeed as long as + * the file descriptor is writable. This is important when combined with + * O_CREAT + */ +TEST_F(Setattr, ftruncate_of_newly_created_file) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const uint64_t ino = 42; + const mode_t mode = 0000; + int fd; + + expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1); + EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT))); + expect_create(RELPATH, ino); + EXPECT_CALL(*m_mock, process( + ResultOf([](auto in) { + return (in->header.opcode == FUSE_SETATTR && + in->header.nodeid == ino && + (in->body.setattr.valid & FATTR_SIZE)); + }, 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_IFREG | mode; + out->body.attr.attr_valid = UINT64_MAX; + }))); + + fd = open(FULLPATH, O_CREAT | O_RDWR, 0); + ASSERT_LE(0, fd) << strerror(errno); + ASSERT_EQ(0, ftruncate(fd, 100)) << strerror(errno); + /* Deliberately leak fd */ +} + /* * Setting the sgid bit should fail for an unprivileged user who doesn't belong * to the file's group