d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fusefs: Finish supporting -o default_permissions

Browse Source 
I got most of -o default_permissions working in r346088.  This commit adds
sticky bit checks.  One downside is that sometimes there will be an extra
FUSE_GETATTR call for the parent directory during unlink or rename.  But in
actual use I think those attributes will almost always be cached.

PR:		216391
Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Alan Somers 2019-04-11 21:00:40 +00:00
parent dc14d593a6
commit 6124fd7106
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/projects/fuse2/; revision=346135
7 changed files with 171 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sys/fs/fuse/fuse_node.h
View File

@ -90,6 +90,7 @@ struct fuse_vnode_data {
/* The monotonic time after which the attr cache is invalid */
struct bintime attr_cache_timeout;
struct vattr cached_attrs;
/* TODO: use cached_attrs.size instead */
off_t filesize;
uint64_t nlookup;
enum vtype vtype;

174
sys/fs/fuse/fuse_vnops.c
View File

@ -797,6 +797,11 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
/* Cache timeout */
atomic_add_acq_long(&fuse_lookup_cache_misses,
1);
/*
* XXX is fuse_internal_vnode_disappear ok to
* call if another process is still using the
* vnode?
*/
fuse_internal_vnode_disappear(*vpp);
if (dvp != *vpp)
vput(*vpp);
@ -864,99 +869,21 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
/* Entry not found */
if ((nameiop == CREATE || nameiop == RENAME) && islastcn) {
err = fuse_internal_access(dvp, VWRITE, td, cred);
if (err)
goto out;
if (!err) {
/*
* Set the SAVENAME flag to hold onto the
* pathname for use later in VOP_CREATE or
* VOP_RENAME.
*/
cnp->cn_flags |= SAVENAME;
/*
* Set the SAVENAME flag to hold onto the
* pathname for use later in VOP_CREATE or VOP_RENAME.
*/
cnp->cn_flags |= SAVENAME;
err = EJUSTRETURN;
goto out;
err = EJUSTRETURN;
}
} else {
err = ENOENT;
}
err = ENOENT;
goto out;
} else {
/* Entry was found */
if ((nameiop == DELETE || nameiop == RENAME) && islastcn) {
err = fuse_internal_access(dvp, VWRITE, td, cred);
if (err != 0)
goto out;
/*
* TODO: if the parent's sticky bit is set, check
* whether we're allowed to remove the file.
* Need to figure out the vnode locking to make this
* work.
*/
#if defined(NOT_YET)
struct vattr dvattr;
fuse_internal_getattr(dvp, &dvattr, cred, td);
if ((dvattr.va_mode & S_ISTXT) &&
fuse_internal_access(dvp, VADMIN, td, cred) &&
fuse_internal_access(*vpp, VADMIN, td, cred)) {
err = EPERM;
goto out;
}
#endif
}
/*
* If deleting, and at end of pathname, return parameters
* which can be used to remove file. If the wantparent flag
* isn't set, we return only the directory, otherwise we go on
* and lock the inode, being careful with ".".
*/
if (nameiop == DELETE && islastcn) {
if (nid == VTOI(dvp)) {
vref(dvp);
*vpp = dvp;
} else {
err = fuse_vnode_get(dvp->v_mount, feo, nid,
dvp, &vp, cnp, vtyp);
if (err)
goto out;
*vpp = vp;
}
/*
* Save the name for use in VOP_RMDIR and VOP_REMOVE
* later.
*/
cnp->cn_flags |= SAVENAME;
goto out;
}
/*
* If rewriting (RENAME), return the inode and the
* information required to rewrite the present directory
* Must get inode of directory entry to verify it's a
* regular file, or empty directory.
*/
if (nameiop == RENAME && wantparent && islastcn) {
/*
* Check for "."
*/
if (nid == VTOI(dvp)) {
err = EISDIR;
goto out;
}
err = fuse_vnode_get(vnode_mount(dvp), feo, nid, dvp,
&vp, cnp, vtyp);
if (err) {
goto out;
}
*vpp = vp;
/*
* Save the name for use in VOP_RENAME later.
*/
cnp->cn_flags |= SAVENAME;
goto out;
}
if (flags & ISDOTDOT) {
struct fuse_lookup_alloc_arg flaa;
@ -966,8 +893,6 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
flaa.vtyp = vtyp;
err = vn_vget_ino_gen(dvp, fuse_lookup_alloc, &flaa, 0,
&vp);
if (err)
goto out;
*vpp = vp;
} else if (nid == VTOI(dvp)) {
vref(dvp);
@ -977,9 +902,10 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
err = fuse_vnode_get(vnode_mount(dvp), feo, nid, dvp,
&vp, cnp, vtyp);
if (err) {
if (err)
goto out;
}
*vpp = vp;
fuse_vnode_setparent(vp, dvp);
/*
@ -989,8 +915,10 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
* the vnode's cached size, fix the vnode cache to
* match the real object size.
*
* This can occur via FUSE distributed filesystems,
* irregular files, etc.
* We can get here:
* * following attribute cache expiration, or
* * due a bug in the daemon, or
* * the first time that we looked up the file.
*/
fvdat = VTOFUD(vp);
if (vnode_isreg(vp) &&
@ -1012,28 +940,52 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
(void)fuse_vnode_setsize(vp, cred, filesize);
fvdat->flag &= ~FN_SIZECHANGE;
}
*vpp = vp;
}
if (feo != NULL) {
MPASS(feo != NULL);
fuse_internal_cache_attrs(*vpp, &feo->attr,
feo->attr_valid, feo->attr_valid_nsec, NULL);
if ((nameiop == DELETE || nameiop == RENAME) &&
islastcn)
{
struct vattr dvattr;
err = fuse_internal_access(dvp, VWRITE, td,
cred);
if (err != 0)
goto out;
/*
* if the parent's sticky bit is set, check
* whether we're allowed to remove the file.
* Need to figure out the vnode locking to make
* this work.
*/
fuse_internal_getattr(dvp, &dvattr, cred, td);
if ((dvattr.va_mode & S_ISTXT) &&
fuse_internal_access(dvp, VADMIN, td,
cred) &&
fuse_internal_access(*vpp, VADMIN, td,
cred)) {
err = EPERM;
goto out;
}
}
if (islastcn && (
(nameiop == DELETE) ||
(nameiop == RENAME && wantparent))) {
cnp->cn_flags |= SAVENAME;
}
}
}
out:
if (!lookup_err) {
/* No lookup error; need to clean up. */
if (err) { /* Found inode; exit with no vnode. */
if (feo != NULL) {
fuse_internal_forget_send(vnode_mount(dvp), td,
cred, nid, 1);
}
if (did_lookup)
fdisp_destroy(&fdi);
return err;
}
if (err) {
if (vp != NULL && dvp != vp)
vput(vp);
else if (vp != NULL)
vrele(vp);
*vpp = NULL;
}
if (did_lookup)
fdisp_destroy(&fdi);

60
tests/sys/fs/fusefs/default_permissions.cc
View File

@ -71,7 +71,6 @@ public:
void expect_getattr(uint64_t ino, mode_t mode, uint64_t attr_valid, int times,
uid_t uid = 0)
{
/* Until the attr cache is working, we may send an additional GETATTR */
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
return (in->header.opcode == FUSE_GETATTR &&
@ -453,7 +452,6 @@ TEST_F(Lookup, eacces)
{
const char FULLPATH[] = "mountpoint/some_dir/some_file.txt";
const char RELDIRPATH[] = "some_dir";
//const char FINALPATH[] = "some_file.txt";
uint64_t dir_ino = 42;
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
@ -548,32 +546,25 @@ TEST_F(Rename, eacces_on_dstdir_for_removing)
ASSERT_EQ(EACCES, errno);
}
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216391 */
TEST_F(Rename, DISABLED_eperm_on_sticky_srcdir)
TEST_F(Rename, eperm_on_sticky_srcdir)
{
const char FULLDST[] = "mountpoint/d/dst";
const char RELDSTDIR[] = "d";
const char RELDST[] = "dst";
const char FULLSRC[] = "mountpoint/src";
const char RELSRC[] = "src";
uint64_t ino = 42;
uint64_t dstdir_ino = 43;
expect_getattr(1, S_IFDIR | 01777, UINT64_MAX, 1, 0);
expect_lookup(RELSRC, ino, S_IFREG | 0644, UINT64_MAX);
expect_lookup(RELDSTDIR, dstdir_ino, S_IFDIR | 0777, UINT64_MAX);
EXPECT_LOOKUP(dstdir_ino, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT)));
ASSERT_EQ(-1, rename(FULLSRC, FULLDST));
ASSERT_EQ(EPERM, errno);
}
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216391 */
TEST_F(Rename, DISABLED_eperm_on_sticky_dstdir)
TEST_F(Rename, eperm_on_sticky_dstdir)
{
const char FULLDST[] = "mountpoint/d/dst";
const char RELDSTDIR[] = "d";
const char RELDST[] = "d/dst";
const char RELDST[] = "dst";
const char FULLSRC[] = "mountpoint/src";
const char RELSRC[] = "src";
uint64_t src_ino = 42;
@ -583,7 +574,15 @@ TEST_F(Rename, DISABLED_eperm_on_sticky_dstdir)
expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1, 0);
expect_lookup(RELSRC, src_ino, S_IFREG | 0644, UINT64_MAX);
expect_lookup(RELDSTDIR, dstdir_ino, S_IFDIR | 01777, UINT64_MAX);
expect_lookup(RELDST, dst_ino, S_IFREG | 0644, UINT64_MAX, 0);
EXPECT_LOOKUP(dstdir_ino, RELDST)
.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 = dst_ino;
out->body.entry.attr_valid = UINT64_MAX;
out->body.entry.entry_valid = UINT64_MAX;
out->body.entry.attr.uid = 0;
})));
ASSERT_EQ(-1, rename(FULLSRC, FULLDST));
ASSERT_EQ(EPERM, errno);
@ -755,6 +754,38 @@ TEST_F(Unlink, ok)
ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno);
}
/*
* Ensure that a cached name doesn't cause unlink to bypass permission checks
* in VOP_LOOKUP.
*
* This test should pass because lookup(9) purges the namecache entry by doing
* a vfs_cache_lookup with ~MAKEENTRY when nameiop == DELETE.
*/
TEST_F(Unlink, cached_unwritable_directory)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
EXPECT_LOOKUP(1, RELPATH)
.Times(AnyNumber())
.WillRepeatedly(Invoke(
ReturnImmediate([=](auto i __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.entry_valid = UINT64_MAX;
}))
);
/* Fill name cache */
ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
/* Despite cached name , unlink should fail */
ASSERT_EQ(-1, unlink(FULLPATH));
ASSERT_EQ(EACCES, errno);
}
TEST_F(Unlink, unwritable_directory)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
@ -768,8 +799,7 @@ TEST_F(Unlink, unwritable_directory)
ASSERT_EQ(EACCES, errno);
}
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216391 */
TEST_F(Unlink, DISABLED_sticky_directory)
TEST_F(Unlink, sticky_directory)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";

2
tests/sys/fs/fusefs/destroy.cc
View File

@ -50,7 +50,7 @@ void expect_destroy(int error)
/*
* On unmount the kernel should send a FUSE_DESTROY operation. It should also
* send FUSE_FORGET operations for all inodes with lookup_count > 0. It's hard
* to trigger FUSE_FORGET in way except by unmounting, so this is the only
* to trigger FUSE_FORGET in any way except by unmounting, so this is the only
* testing that FUSE_FORGET gets.
*/
TEST_F(Destroy, ok)

24
tests/sys/fs/fusefs/rename.cc
View File

@ -51,6 +51,24 @@ class Rename: public FuseTest {
FuseTest::TearDown();
}
void expect_getattr(uint64_t ino, mode_t mode)
{
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
return (in->header.opcode == FUSE_GETATTR &&
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 = mode;
out->body.attr.attr_valid = UINT64_MAX;
})));
}
};
// EINVAL, dst is subdir of src
@ -62,6 +80,7 @@ TEST_F(Rename, einval)
const char RELSRC[] = "src";
uint64_t src_ino = 42;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELSRC, src_ino, S_IFDIR | 0755, 0, 2);
EXPECT_LOOKUP(src_ino, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT)));
@ -102,6 +121,7 @@ TEST_F(Rename, entry_cache_negative)
*/
struct timespec entry_valid = {.tv_sec = 0, .tv_nsec = 0};
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELSRC, ino, S_IFREG | 0644, 0, 1);
/* LOOKUP returns a negative cache entry for dst */
EXPECT_LOOKUP(1, RELDST).WillOnce(ReturnNegativeCache(&entry_valid));
@ -136,6 +156,7 @@ TEST_F(Rename, entry_cache_negative_purge)
uint64_t ino = 42;
struct timespec entry_valid = {.tv_sec = TIME_T_MAX, .tv_nsec = 0};
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELSRC, ino, S_IFREG | 0644, 0, 1);
/* LOOKUP returns a negative cache entry for dst */
EXPECT_LOOKUP(1, RELDST).WillOnce(ReturnNegativeCache(&entry_valid))
@ -172,6 +193,7 @@ TEST_F(Rename, exdev)
tmpfd = mkstemp(tmpfile);
ASSERT_LE(0, tmpfd) << strerror(errno);
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELB, b_ino, S_IFREG | 0644, 0, 2);
ASSERT_NE(0, rename(tmpfile, FULLB));
@ -191,6 +213,7 @@ TEST_F(Rename, ok)
uint64_t dst_dir_ino = 1;
uint64_t ino = 42;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELSRC, ino, S_IFREG | 0644, 0, 1);
EXPECT_LOOKUP(1, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT)));
@ -223,6 +246,7 @@ TEST_F(Rename, overwrite)
uint64_t dst_dir_ino = 1;
uint64_t ino = 42;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELSRC, ino, S_IFREG | 0644, 0, 1);
expect_lookup(RELDST, dst_ino, S_IFREG | 0644, 0, 1);
EXPECT_CALL(*m_mock, process(

18
tests/sys/fs/fusefs/rmdir.cc
View File

@ -39,6 +39,22 @@ using namespace testing;
class Rmdir: public FuseTest {
public:
void expect_getattr(uint64_t ino, mode_t mode)
{
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
return (in->header.opcode == FUSE_GETATTR &&
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 = mode;
out->body.attr.attr_valid = UINT64_MAX;
})));
}
void expect_lookup(const char *relpath, uint64_t ino)
{
EXPECT_LOOKUP(1, relpath)
@ -57,6 +73,7 @@ TEST_F(Rmdir, enotempty)
const char RELPATH[] = "some_dir";
uint64_t ino = 42;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELPATH, ino);
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
@ -77,6 +94,7 @@ TEST_F(Rmdir, ok)
const char RELPATH[] = "some_dir";
uint64_t ino = 42;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELPATH, ino);
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {

19
tests/sys/fs/fusefs/unlink.cc
View File

@ -38,6 +38,22 @@ using namespace testing;
class Unlink: public FuseTest {
public:
void expect_getattr(uint64_t ino, mode_t mode)
{
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
return (in->header.opcode == FUSE_GETATTR &&
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 = mode;
out->body.attr.attr_valid = UINT64_MAX;
})));
}
void expect_lookup(const char *relpath, uint64_t ino, int times)
{
FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, times);
@ -51,6 +67,7 @@ TEST_F(Unlink, eperm)
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELPATH, ino, 1);
expect_unlink(1, RELPATH, EPERM);
@ -64,6 +81,7 @@ TEST_F(Unlink, ok)
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELPATH, ino, 1);
expect_unlink(1, RELPATH, 0);
@ -78,6 +96,7 @@ TEST_F(Unlink, open_but_deleted)
uint64_t ino = 42;
int fd;
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELPATH, ino, 2);
expect_open(ino, 0, 1);
expect_unlink(1, RELPATH, 0);