Skip site navigation (1)Skip section navigation (2)
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>