On Thu, 17 Aug 2000, Boris Popov wrote:
> On Wed, 16 Aug 2000, Poul-Henning Kamp wrote:
>
> > Please test and review this patch:
> >
> > http://phk.freebsd.dk/patch/vop_stdaccess.patch
>
> Looks fine to me except vop_stdaccess() itself.
vop_stdaccess() is badly named. It is not a vop function (one suitable
for putting in vop tables). In NetBSD, the corresponding function is:
int vaccess __P((enum vtype type, mode_t file_mode, uid_t uid, gid_t gid,
mode_t acc_mode, struct ucred *cred));
This is declared in <sys/vnode.h> and implemented in kern/vfs_subr.c.
It has the following interesting differences:
- it checks the X bits for root. Most or all of our filesystems omit this
check, so access(2) by root bogusly returns success for X_OK of files
with no X bits set, and VOP_ACCESS() can't actually be used to check for
X access by root. The POSIX spec for access(2) may permit this braindamage.
It permits access(2) for X_OK to succeed for processes with appropriate
privilege although no X bits are set. I'm not sure if it requires this
success to have anything to do with executability. execve() in the kernel
knows not to trust VOP_ACCESS() and does an explicit test of the X bits
(hard-coded as 0111 :-()) in check_permissions(). Shells know not to
trust access() and do similar explicit tests.
- it doesn't bother inlining groupmember().
- it is missing some style bugs.
The function could reasonably handle MNT_RDONLY, MNT_NOEXEC and
immutability, especially the first two since they are necessary for
most filesystems (cd9660 handles MNT_RDONLY although it doesn't support
writing). MNT_NOEXEC bogotifies access() in the oppisite direction
even for non-root. execve() in the kernel does an explicit check for
it. I guess shells and execlp() are confused by it.
> Since VREAD,
> VWRITE and VEXEC bits are carefully layed this function can be rewritten
> to use single shift operation instead of 3 'or's:
>
> if (cred->cr_uid != uid) {
> amode >>= 3;
> if (!groupmember(gid, cred))
> amode >>= 3;
> }
> return (fmode & amode) == amode ? 0 : EACCES;
This optimization seems to be intentionally left out. I don't think it
is worth doing. Hopefully the compiler will do it. gcc doesn't seem
to do it, but I've seen it generate good code for repacking less well
laid out bits in device drivers.
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
