From owner-p4-projects@FreeBSD.ORG Thu Sep 24 21:29:29 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 69B381065670; Thu, 24 Sep 2009 21:29:29 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2E06B106566B for ; Thu, 24 Sep 2009 21:29:29 +0000 (UTC) (envelope-from trasz@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 128D08FC22 for ; Thu, 24 Sep 2009 21:29:29 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n8OLTS4G094684 for ; Thu, 24 Sep 2009 21:29:28 GMT (envelope-from trasz@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n8OLTSOv094682 for perforce@freebsd.org; Thu, 24 Sep 2009 21:29:28 GMT (envelope-from trasz@freebsd.org) Date: Thu, 24 Sep 2009 21:29:28 GMT Message-Id: <200909242129.n8OLTSOv094682@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to trasz@freebsd.org using -f From: Edward Tomasz Napierala To: Perforce Change Reviews Cc: Subject: PERFORCE change 168866 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Sep 2009 21:29:29 -0000 http://perforce.freebsd.org/chv.cgi?CH=168866 Change 168866 by trasz@trasz_victim on 2009/09/24 21:28:41 Instead of calling vfs_unixify_accmode() in vaccess(9) and vaccess_acl_posix1e(9), just KASSERT that there are no NFSv4 bits passed to them and do the 'unixification' in ufs_accessx(). Affected files ... .. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_posix1e.c#16 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_subr.c#40 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#34 edit Differences ... ==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_posix1e.c#16 (text+ko) ==== @@ -59,7 +59,10 @@ accmode_t dac_granted; accmode_t priv_granted; accmode_t acl_mask_granted; - int group_matched, i, error; + int group_matched, i; + + KASSERT((accmode & ~(VEXEC | VWRITE | VREAD | VADMIN | VAPPEND)) == 0, + ("invalid bit in accmode")); /* * Look for a normal, non-privileged way to access the file/directory @@ -71,10 +74,6 @@ if (privused != NULL) *privused = 0; - error = vfs_unixify_accmode(&accmode); - if (error) - return (error); - /* * Determine privileges now, but don't apply until we've found a DAC * entry that matches but has failed to allow access. ==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_subr.c#40 (text+ko) ==== @@ -3519,7 +3519,9 @@ { accmode_t dac_granted; accmode_t priv_granted; - int error; + + KASSERT((accmode & ~(VEXEC | VWRITE | VREAD | VADMIN | VAPPEND)) == 0, + ("invalid bit in accmode")); /* * Look for a normal, non-privileged way to access the file/directory @@ -3529,10 +3531,6 @@ if (privused != NULL) *privused = 0; - error = vfs_unixify_accmode(&accmode); - if (error) - return (error); - dac_granted = 0; /* Check the owner. */ @@ -3645,13 +3643,6 @@ /* Potentially should be: return (EPERM); */ return (priv_check_cred(cred, PRIV_VFS_EXTATTR_SYSTEM, 0)); case EXTATTR_NAMESPACE_USER: -#ifdef SunOS_doesnt_do_that - if (accmode == VREAD) - return (VOP_ACCESSX(vp, VREAD_NAMED_ATTRS, cred, td)); - if (accmode == VWRITE) - return (VOP_ACCESSX(vp, VWRITE_NAMED_ATTRS, cred, td)); -#endif - /* XXX: Is this possible for "accmode" to not be any of the two above? */ return (VOP_ACCESS(vp, accmode, cred, td)); default: return (EPERM); ==== //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#34 (text+ko) ==== @@ -88,7 +88,6 @@ #include -static vop_access_t ufs_access; static vop_accessx_t ufs_accessx; static int ufs_chmod(struct vnode *, int, struct ucred *, struct thread *); static int ufs_chown(struct vnode *, uid_t, gid_t, struct ucred *, struct thread *); @@ -299,19 +298,6 @@ } static int -ufs_access(ap) - struct vop_access_args /* { - struct vnode *a_vp; - accmode_t a_accmode; - struct ucred *a_cred; - struct thread *a_td; - } */ *ap; -{ - - return (VOP_ACCESSX(ap->a_vp, ap->a_accmode, ap->a_cred, ap->a_td)); -} - -static int ufs_accessx(ap) struct vop_accessx_args /* { struct vnode *a_vp; @@ -323,6 +309,7 @@ struct vnode *vp = ap->a_vp; struct inode *ip = VTOI(vp); int error; + accmode_t accmode = ap->a_accmode; #ifdef QUOTA int relocked; #endif @@ -336,7 +323,7 @@ * unless the file is a socket, fifo, or a block or * character device resident on the filesystem. */ - if (ap->a_accmode & VMODIFY_PERMS) { + if (accmode & VMODIFY_PERMS) { switch (vp->v_type) { case VDIR: case VLNK: @@ -382,11 +369,11 @@ } /* - * If immutable bit set, nobody gets to write it. - * "& ~VADMIN_PERMS" is here, because without it, - * it would be impossible to remove the IMMUTABLE flag. + * If immutable bit set, nobody gets to write it. "& ~VADMIN_PERMS" + * is here, because without it, * it would be impossible for the owner + * to remove the IMMUTABLE flag. */ - if ((ap->a_accmode & (VMODIFY_PERMS & ~VADMIN_PERMS)) && + if ((accmode & (VMODIFY_PERMS & ~VADMIN_PERMS)) && (ip->i_flags & (IMMUTABLE | SF_SNAPSHOT))) return (EPERM); @@ -398,39 +385,43 @@ type = ACL_TYPE_ACCESS; acl = acl_alloc(M_WAITOK); - error = VOP_GETACL(vp, type, acl, ap->a_cred, - ap->a_td); + error = VOP_GETACL(vp, type, acl, ap->a_cred, ap->a_td); switch (error) { - case EOPNOTSUPP: - error = vaccess(vp->v_type, ip->i_mode, ip->i_uid, - ip->i_gid, ap->a_accmode, ap->a_cred, NULL); - break; case 0: if (type == ACL_TYPE_NFS4) { error = vaccess_acl_nfs4(vp->v_type, ip->i_uid, - ip->i_gid, acl, ap->a_accmode, ap->a_cred, NULL); + ip->i_gid, acl, accmode, ap->a_cred, NULL); } else { - error = vaccess_acl_posix1e(vp->v_type, ip->i_uid, - ip->i_gid, acl, ap->a_accmode, ap->a_cred, NULL); + error = vfs_unixify_accmode(&accmode); + if (error == 0) + error = vaccess_acl_posix1e(vp->v_type, ip->i_uid, + ip->i_gid, acl, accmode, ap->a_cred, NULL); } break; default: - printf( + if (error != EOPNOTSUPP) + printf( "ufs_accessx(): Error retrieving ACL on object (%d).\n", - error); + error); /* * XXX: Fall back until debugged. Should * eventually possibly log an error, and return * EPERM for safety. */ - error = vaccess(vp->v_type, ip->i_mode, ip->i_uid, - ip->i_gid, ap->a_accmode, ap->a_cred, NULL); + error = vfs_unixify_accmode(&accmode); + if (error == 0) + error = vaccess(vp->v_type, ip->i_mode, ip->i_uid, + ip->i_gid, accmode, ap->a_cred, NULL); } acl_free(acl); - } else + + return (error); + } #endif /* !UFS_ACL */ + error = vfs_unixify_accmode(&accmode); + if (error == 0) error = vaccess(vp->v_type, ip->i_mode, ip->i_uid, ip->i_gid, - ap->a_accmode, ap->a_cred, NULL); + accmode, ap->a_cred, NULL); return (error); } @@ -2634,7 +2625,6 @@ .vop_read = VOP_PANIC, .vop_reallocblks = VOP_PANIC, .vop_write = VOP_PANIC, - .vop_access = ufs_access, .vop_accessx = ufs_accessx, .vop_bmap = ufs_bmap, .vop_cachedlookup = ufs_lookup, @@ -2679,7 +2669,6 @@ struct vop_vector ufs_fifoops = { .vop_default = &fifo_specops, .vop_fsync = VOP_PANIC, - .vop_access = ufs_access, .vop_accessx = ufs_accessx, .vop_close = ufsfifo_close, .vop_getattr = ufs_getattr,