Date: Tue, 03 Sep 2019 14:06:18 -0000 From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r345962 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs Message-ID: <201904051837.x35Ibm4X059559@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Fri Apr 5 18:37:48 2019 New Revision: 345962 URL: https://svnweb.freebsd.org/changeset/base/345962 Log: fusefs: implement VOP_ACCESS VOP_ACCESS was never fully implemented in fusefs. This change: * Removes the FACCESS_DO_ACCESS flag, which pretty much disabled the whole vop. * Removes a quixotic special case for VEXEC on regular files. I don't know why that was in there. * Removes another confusing special case for VADMIN. * Removes the FACCESS_NOCHECKSPY flag. It seemed to be a performance optimization, but I'm unconvinced that it was a net positive. * Updates test cases. This change does NOT implement -o default_permissions. That will be handled separately. PR: 236291 Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c projects/fuse2/sys/fs/fuse/fuse_internal.h projects/fuse2/tests/sys/fs/fusefs/access.cc projects/fuse2/tests/sys/fs/fusefs/mockfs.cc projects/fuse2/tests/sys/fs/fusefs/utils.cc Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_internal.c Fri Apr 5 18:17:11 2019 (r345961) +++ projects/fuse2/sys/fs/fuse/fuse_internal.c Fri Apr 5 18:37:48 2019 (r345962) @@ -115,7 +115,7 @@ fuse_internal_access(struct vnode *vp, struct ucred *cred) { int err = 0; - uint32_t mask = 0; + uint32_t mask = F_OK; int dataflags; int vtype; struct mount *mp; @@ -123,13 +123,6 @@ fuse_internal_access(struct vnode *vp, struct fuse_access_in *fai; struct fuse_data *data; - /* NOT YET DONE */ - /* - * If this vnop gives you trouble, just return 0 here for a lazy - * kludge. - */ - /* return 0;*/ - mp = vnode_mount(vp); vtype = vnode_vtype(vp); @@ -139,65 +132,37 @@ fuse_internal_access(struct vnode *vp, if ((mode & VWRITE) && vfs_isrdonly(mp)) { return EROFS; } + /* Unless explicitly permitted, deny everyone except the fs owner. */ - if (!(facp->facc_flags & FACCESS_NOCHECKSPY)) { + if (!(facp->facc_flags)) { if (!(dataflags & FSESS_DAEMON_CAN_SPY)) { - int denied = fuse_match_cred(data->daemoncred, - cred); + int denied = fuse_match_cred(data->daemoncred, cred); - if (denied) { + if (denied) return EPERM; - } } - /* - * Set the "skip cred check" flag so future callers that share - * facp can skip fuse_match_cred. - */ - facp->facc_flags |= FACCESS_NOCHECKSPY; } - if (!(facp->facc_flags & FACCESS_DO_ACCESS)) { + + if (dataflags & FSESS_DEFAULT_PERMISSIONS) { + /* TODO: Implement me! Bug 216391 */ return 0; } - if (((vtype == VREG) && (mode & VEXEC))) { -#ifdef NEED_MOUNT_ARGUMENT_FOR_THIS - /* Let the kernel handle this through open / close heuristics.*/ - return ENOTSUP; -#else - /* Let the kernel handle this. */ - return 0; -#endif - } - if (!fsess_isimpl(mp, FUSE_ACCESS)) { - /* Let the kernel handle this. */ - return 0; - } - if (dataflags & FSESS_DEFAULT_PERMISSIONS) { - /* Let the kernel handle this. */ - return 0; - } - if ((mode & VADMIN) != 0) { - err = priv_check_cred(cred, PRIV_VFS_ADMIN); - if (err) { - return err; - } - } - if ((mode & (VWRITE | VAPPEND | VADMIN)) != 0) { + + if (!fsess_isimpl(mp, FUSE_ACCESS)) + return 0; + + if ((mode & (VWRITE | VAPPEND | VADMIN)) != 0) mask |= W_OK; - } - if ((mode & VREAD) != 0) { + if ((mode & VREAD) != 0) mask |= R_OK; - } - if ((mode & VEXEC) != 0) { + if ((mode & VEXEC) != 0) mask |= X_OK; - } - bzero(&fdi, sizeof(fdi)); fdisp_init(&fdi, sizeof(*fai)); fdisp_make_vp(&fdi, FUSE_ACCESS, vp, td, cred); fai = fdi.indata; - fai->mask = F_OK; - fai->mask |= mask; + fai->mask = mask; err = fdisp_wait_answ(&fdi); fdisp_destroy(&fdi); Modified: projects/fuse2/sys/fs/fuse/fuse_internal.h ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_internal.h Fri Apr 5 18:17:11 2019 (r345961) +++ projects/fuse2/sys/fs/fuse/fuse_internal.h Fri Apr 5 18:37:48 2019 (r345962) @@ -155,7 +155,6 @@ fuse_iosize(struct vnode *vp) #define FVP_ACCESS_NOOP 0x01 #define FACCESS_VA_VALID 0x01 -#define FACCESS_DO_ACCESS 0x02 /* * Caller must be the directory's owner, or the superuser, or the sticky bit * must not be set @@ -163,7 +162,6 @@ fuse_iosize(struct vnode *vp) #define FACCESS_STICKY 0x04 /* Caller requires access to change file's owner */ #define FACCESS_CHOWN 0x08 -#define FACCESS_NOCHECKSPY 0x10 #define FACCESS_SETGID 0x12 #define FACCESS_XQUERIES (FACCESS_STICKY | FACCESS_CHOWN | FACCESS_SETGID) Modified: projects/fuse2/tests/sys/fs/fusefs/access.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/access.cc Fri Apr 5 18:17:11 2019 (r345961) +++ projects/fuse2/tests/sys/fs/fusefs/access.cc Fri Apr 5 18:37:48 2019 (r345962) @@ -55,14 +55,14 @@ virtual void SetUp() { }; /* The error case of FUSE_ACCESS. */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236291 */ -TEST_F(Access, DISABLED_eaccess) +TEST_F(Access, eaccess) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; uint64_t ino = 42; mode_t access_mode = X_OK; + expect_access(1, X_OK, 0); expect_lookup(RELPATH, ino); expect_access(ino, access_mode, EACCES); @@ -75,17 +75,15 @@ TEST_F(Access, DISABLED_eaccess) * and subsequent VOP_ACCESS calls will succeed automatically without querying * the daemon. */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236557 */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236291 */ -TEST_F(Access, DISABLED_enosys) +TEST_F(Access, enosys) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; uint64_t ino = 42; mode_t access_mode = R_OK; - expect_lookup(RELPATH, ino); - expect_access(ino, access_mode, ENOSYS); + expect_access(1, X_OK, ENOSYS); + FuseTest::expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 2); ASSERT_EQ(0, access(FULLPATH, access_mode)) << strerror(errno); ASSERT_EQ(0, access(FULLPATH, access_mode)) << strerror(errno); @@ -98,6 +96,7 @@ TEST_F(RofsAccess, erofs) uint64_t ino = 42; mode_t access_mode = W_OK; + expect_access(1, X_OK, 0); expect_lookup(RELPATH, ino); ASSERT_NE(0, access(FULLPATH, access_mode)); @@ -105,14 +104,14 @@ TEST_F(RofsAccess, erofs) } /* The successful case of FUSE_ACCESS. */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236291 */ -TEST_F(Access, DISABLED_ok) +TEST_F(Access, ok) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; uint64_t ino = 42; mode_t access_mode = R_OK; + expect_access(1, X_OK, 0); expect_lookup(RELPATH, ino); expect_access(ino, access_mode, 0); Modified: projects/fuse2/tests/sys/fs/fusefs/mockfs.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/mockfs.cc Fri Apr 5 18:17:11 2019 (r345961) +++ projects/fuse2/tests/sys/fs/fusefs/mockfs.cc Fri Apr 5 18:37:48 2019 (r345962) @@ -162,6 +162,9 @@ void debug_fuseop(const mockfs_buf_in *in) switch (in->header.opcode) { const char *name, *value; + case FUSE_ACCESS: + printf(" mask=%#x", in->body.access.mask); + break; case FUSE_CREATE: name = (const char*)in->body.bytes + sizeof(fuse_open_in); Modified: projects/fuse2/tests/sys/fs/fusefs/utils.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/utils.cc Fri Apr 5 18:17:11 2019 (r345961) +++ projects/fuse2/tests/sys/fs/fusefs/utils.cc Fri Apr 5 18:37:48 2019 (r345962) @@ -96,6 +96,21 @@ void FuseTest::SetUp() { m_mock = new MockFS(m_maxreadahead, m_allow_other, m_default_permissions, m_push_symlinks_in, m_ro, m_init_flags); + /* + * FUSE_ACCESS is called almost universally. Expecting it in + * each test case would be super-annoying. Instead, set a + * default expectation for FUSE_ACCESS and return ENOSYS. + * + * Individual test cases can override this expectation since + * googlemock evaluates expectations in LIFO order. + */ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_ACCESS); + }, Eq(true)), + _) + ).Times(AnyNumber()) + .WillRepeatedly(Invoke(ReturnErrno(ENOSYS))); } catch (std::system_error err) { FAIL() << err.what(); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201904051837.x35Ibm4X059559>