fusefs: remove "early permission check hack"

fuse_vnop_lookup contained an awkward hack meant to reduce daemon activity
during long lookup chains.  However, the hack is no longer necessary now
that we properly cache file attributes.  Also, I'm 99% certain that it
could've bypassed permission checks when using openat to open a file
relative to a directory that lacks execute permission.

Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Alan Somers 2019-04-10 21:46:59 +00:00
parent 666f8543bb
commit 73825da397
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/projects/fuse2/; revision=346106

View File

@ -745,25 +745,15 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
*vpp = NULL;
return ENXIO;
}
if (!vnode_isdir(dvp)) {
if (!vnode_isdir(dvp))
return ENOTDIR;
}
if (islastcn && vfs_isrdonly(mp) && (nameiop != LOOKUP)) {
return EROFS;
}
/*
* We do access check prior to doing anything else only in the case
* when we are at fs root (we'd like to say, "we are at the first
* component", but that's not exactly the same... nevermind).
* See further comments at further access checks.
*/
/* TODO: consider eliminating this. Is there any good reason for it? */
if (vnode_isvroot(dvp)) { /* early permission check hack */
if ((err = fuse_internal_access(dvp, VEXEC, td, cred))) {
return err;
}
}
if (islastcn && vfs_isrdonly(mp) && (nameiop != LOOKUP))
return EROFS;
if ((err = fuse_internal_access(dvp, VEXEC, td, cred)))
return err;
if (flags & ISDOTDOT) {
nid = VTOFUD(dvp)->parent_nid;
if (nid == 0) {
@ -1071,58 +1061,6 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
fdisp_destroy(&fdi);
return err;
} else {
#ifndef NO_EARLY_PERM_CHECK_HACK
if (!islastcn) {
/*
* We have the attributes of the next item
* *now*, and it's a fact, and we do not
* have to do extra work for it (ie, beg the
* daemon), and it neither depends on such
* accidental things like attr caching. So
* the big idea: check credentials *now*,
* not at the beginning of the next call to
* lookup.
*
* The first item of the lookup chain (fs root)
* won't be checked then here, of course, as
* its never "the next". But go and see that
* the root is taken care about at the very
* beginning of this function.
*
* Now, given we want to do the access check
* this way, one might ask: so then why not
* do the access check just after fetching
* the inode and its attributes from the
* daemon? Why bother with producing the
* corresponding vnode at all if something
* is not OK? We know what's the deal as
* soon as we get those attrs... There is
* one bit of info though not given us by
* the daemon: whether his response is
* authoritative or not... His response should
* be ignored if something is mounted over
* the dir in question. But that can be
* known only by having the vnode...
*/
int tmpvtype = vnode_vtype(*vpp);
if ((tmpvtype != VDIR) && (tmpvtype != VLNK)) {
err = ENOTDIR;
}
if (!err && !vnode_mountedhere(*vpp)) {
err = fuse_internal_access(*vpp, VEXEC,
td, cred);
}
if (err) {
if (tmpvtype == VLNK)
SDT_PROBE2(fuse, , vnops, trace,
1, "weird, permission "
"error with a symlink?");
vput(*vpp);
*vpp = NULL;
}
}
#endif
}
}
fdisp_destroy(&fdi);