Date: Mon, 30 Aug 2010 16:30:18 +0000 (UTC) From: Jaakko Heinonen <jh@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r212002 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs kern Message-ID: <201008301630.o7UGUIK9000756@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jh Date: Mon Aug 30 16:30:18 2010 New Revision: 212002 URL: http://svn.freebsd.org/changeset/base/212002 Log: execve(2) has a special check for file permissions: a file must have at least one execute bit set, otherwise execve(2) will return EACCES even for an user with PRIV_VFS_EXEC privilege. Add the check also to vaccess(9), vaccess_acl_nfs4(9) and vaccess_acl_posix1e(9). This makes access(2) to better agree with execve(2). Because ZFS doesn't use vaccess(9) for VEXEC, add the check to zfs_freebsd_access() too. There may be other file systems which are not using vaccess*() functions and need to be handled separately. PR: kern/125009 Reviewed by: bde, trasz Approved by: pjd (ZFS part) Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c head/sys/kern/kern_exec.c head/sys/kern/subr_acl_nfs4.c head/sys/kern/subr_acl_posix1e.c head/sys/kern/vfs_subr.c Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Mon Aug 30 15:06:55 2010 (r212001) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Mon Aug 30 16:30:18 2010 (r212002) @@ -4247,6 +4247,9 @@ zfs_freebsd_access(ap) struct thread *a_td; } */ *ap; { + vnode_t *vp = ap->a_vp; + znode_t *zp = VTOZ(vp); + znode_phys_t *zphys = zp->z_phys; accmode_t accmode; int error = 0; @@ -4263,16 +4266,20 @@ zfs_freebsd_access(ap) if (error == 0) { accmode = ap->a_accmode & ~(VREAD|VWRITE|VEXEC|VAPPEND); if (accmode != 0) { - vnode_t *vp = ap->a_vp; - znode_t *zp = VTOZ(vp); - znode_phys_t *zphys = zp->z_phys; - error = vaccess(vp->v_type, zphys->zp_mode, zphys->zp_uid, zphys->zp_gid, accmode, ap->a_cred, NULL); } } + /* + * For VEXEC, ensure that at least one execute bit is set for + * non-directories. + */ + if (error == 0 && (ap->a_accmode & VEXEC) != 0 && vp->v_type != VDIR && + (zphys->zp_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0) + error = EACCES; + return (error); } Modified: head/sys/kern/kern_exec.c ============================================================================== --- head/sys/kern/kern_exec.c Mon Aug 30 15:06:55 2010 (r212001) +++ head/sys/kern/kern_exec.c Mon Aug 30 16:30:18 2010 (r212002) @@ -1363,17 +1363,17 @@ exec_check_permissions(imgp) if (error) return (error); #endif - + /* - * 1) Check if file execution is disabled for the filesystem that this - * file resides on. - * 2) Insure that at least one execute bit is on - otherwise root - * will always succeed, and we don't want to happen unless the - * file really is executable. - * 3) Insure that the file is a regular file. + * 1) Check if file execution is disabled for the filesystem that + * this file resides on. + * 2) Ensure that at least one execute bit is on. Otherwise, a + * privileged user will always succeed, and we don't want this + * to happen unless the file really is executable. + * 3) Ensure that the file is a regular file. */ if ((vp->v_mount->mnt_flag & MNT_NOEXEC) || - ((attr->va_mode & 0111) == 0) || + (attr->va_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0 || (attr->va_type != VREG)) return (EACCES); Modified: head/sys/kern/subr_acl_nfs4.c ============================================================================== --- head/sys/kern/subr_acl_nfs4.c Mon Aug 30 15:06:55 2010 (r212001) +++ head/sys/kern/subr_acl_nfs4.c Mon Aug 30 16:30:18 2010 (r212002) @@ -162,6 +162,7 @@ vaccess_acl_nfs4(enum vtype type, uid_t accmode_t priv_granted = 0; int denied, explicitly_denied, access_mask, is_directory, must_be_owner = 0; + mode_t file_mode; KASSERT((accmode & ~(VEXEC | VWRITE | VREAD | VADMIN | VAPPEND | VEXPLICIT_DENY | VREAD_NAMED_ATTRS | VWRITE_NAMED_ATTRS | @@ -216,6 +217,17 @@ vaccess_acl_nfs4(enum vtype type, uid_t denied = EPERM; } + /* + * For VEXEC, ensure that at least one execute bit is set for + * non-directories. We have to check the mode here to stay + * consistent with execve(2). See the test in + * exec_check_permissions(). + */ + acl_nfs4_sync_mode_from_acl(&file_mode, aclp); + if (!denied && !is_directory && (accmode & VEXEC) && + (file_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0) + denied = EACCES; + if (!denied) return (0); @@ -236,8 +248,14 @@ vaccess_acl_nfs4(enum vtype type, uid_t PRIV_VFS_LOOKUP, 0)) priv_granted |= VEXEC; } else { - if ((accmode & VEXEC) && !priv_check_cred(cred, - PRIV_VFS_EXEC, 0)) + /* + * Ensure that at least one execute bit is on. Otherwise, + * a privileged user will always succeed, and we don't want + * this to happen unless the file really is executable. + */ + if ((accmode & VEXEC) && (file_mode & + (S_IXUSR | S_IXGRP | S_IXOTH)) != 0 && + !priv_check_cred(cred, PRIV_VFS_EXEC, 0)) priv_granted |= VEXEC; } Modified: head/sys/kern/subr_acl_posix1e.c ============================================================================== --- head/sys/kern/subr_acl_posix1e.c Mon Aug 30 15:06:55 2010 (r212001) +++ head/sys/kern/subr_acl_posix1e.c Mon Aug 30 16:30:18 2010 (r212002) @@ -90,8 +90,14 @@ vaccess_acl_posix1e(enum vtype type, uid PRIV_VFS_LOOKUP, 0)) priv_granted |= VEXEC; } else { - if ((accmode & VEXEC) && !priv_check_cred(cred, - PRIV_VFS_EXEC, 0)) + /* + * Ensure that at least one execute bit is on. Otherwise, + * a privileged user will always succeed, and we don't want + * this to happen unless the file really is executable. + */ + if ((accmode & VEXEC) && (acl_posix1e_acl_to_mode(acl) & + (S_IXUSR | S_IXGRP | S_IXOTH)) != 0 && + !priv_check_cred(cred, PRIV_VFS_EXEC, 0)) priv_granted |= VEXEC; } Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Mon Aug 30 15:06:55 2010 (r212001) +++ head/sys/kern/vfs_subr.c Mon Aug 30 16:30:18 2010 (r212002) @@ -3619,7 +3619,13 @@ privcheck: !priv_check_cred(cred, PRIV_VFS_LOOKUP, 0)) priv_granted |= VEXEC; } else { + /* + * Ensure that at least one execute bit is on. Otherwise, + * a privileged user will always succeed, and we don't want + * this to happen unless the file really is executable. + */ if ((accmode & VEXEC) && ((dac_granted & VEXEC) == 0) && + (file_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0 && !priv_check_cred(cred, PRIV_VFS_EXEC, 0)) priv_granted |= VEXEC; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008301630.o7UGUIK9000756>