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
This commit is contained in:
Alan Somers 2019-04-05 18:37:48 +00:00
parent efa23d9784
commit caf5f57d2d
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/projects/fuse2/; revision=345962
5 changed files with 44 additions and 64 deletions

View File

@ -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 (!(dataflags & FSESS_DAEMON_CAN_SPY)) {
int denied = fuse_match_cred(data->daemoncred,
cred);
if (denied) {
/* Unless explicitly permitted, deny everyone except the fs owner. */
if (!(facp->facc_flags)) {
if (!(dataflags & FSESS_DAEMON_CAN_SPY)) {
int denied = fuse_match_cred(data->daemoncred, cred);
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);

View File

@ -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)

View File

@ -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);

View File

@ -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);

View File

@ -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();
}