Rectify some issues with the noperm mount flag; the root vnode was not

protected properly and files without any x bit set were accidentaly considered
executable when checked with access(2).

Issues found and reported by deraadt, halex, reyk, tb
ok deraadt

6 files changed, 43 insertions, 32 deletions

diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index 7b8643f011a..4e888b98f27 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_subr.c,v 1.283 2018/12/07 16:21:19 mpi Exp $	*/
+/*	$OpenBSD: vfs_subr.c,v 1.284 2018/12/23 10:46:51 natano Exp $	*/
 /*	$NetBSD: vfs_subr.c,v 1.53 1996/04/22 01:39:13 christos Exp $	*/
 
 /*
@@ -1611,6 +1611,15 @@ vaccess(enum vtype type, mode_t file_mode, uid_t uid, gid_t gid,
 	return (file_mode & mask) == mask ? 0 : EACCES;
 }
 
+int
+vnoperm(struct vnode *vp)
+{
+	if (vp->v_flag & VROOT || vp->v_mount == NULL)
+		return 0;
+
+	return (vp->v_mount->mnt_flag & MNT_NOPERM);
+}
+
 struct rwlock vfs_stall_lock = RWLOCK_INITIALIZER("vfs_stall");
 
 int
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index 0993918abc9..101bd3aca24 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.308 2018/10/28 22:42:33 beck Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.309 2018/12/23 10:46:51 natano Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -1394,8 +1394,7 @@ domknodat(struct proc *p, int fd, const char *path, mode_t mode, dev_t dev)
 		return (error);
 	vp = nd.ni_vp;
 	if (!S_ISFIFO(mode) || dev != 0) {
-		if ((nd.ni_dvp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
-		    (error = suser(p)) != 0)
+		if (!vnoperm(nd.ni_dvp) && (error = suser(p)) != 0)
 			goto out;
 		if (p->p_fd->fd_rdir) {
 			error = EINVAL;
@@ -2265,7 +2264,7 @@ dofchownat(struct proc *p, int fd, const char *path, uid_t uid, gid_t gid,
 		if ((error = pledge_chown(p, uid, gid)))
 			goto out;
 		if ((uid != -1 || gid != -1) &&
-		    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+		    !vnoperm(vp) &&
 		    (suser(p) || suid_clear)) {
 			error = VOP_GETATTR(vp, &vattr, p->p_ucred, p);
 			if (error)
@@ -2318,7 +2317,7 @@ sys_lchown(struct proc *p, void *v, register_t *retval)
 		if ((error = pledge_chown(p, uid, gid)))
 			goto out;
 		if ((uid != -1 || gid != -1) &&
-		    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+		    !vnoperm(vp) &&
 		    (suser(p) || suid_clear)) {
 			error = VOP_GETATTR(vp, &vattr, p->p_ucred, p);
 			if (error)
@@ -2368,8 +2367,7 @@ sys_fchown(struct proc *p, void *v, register_t *retval)
 		if ((error = pledge_chown(p, uid, gid)))
 			goto out;
 		if ((uid != -1 || gid != -1) &&
-		    (vp->v_mount &&
-		     (vp->v_mount->mnt_flag & MNT_NOPERM) == 0) &&
+		    !vnoperm(vp) &&
 		    (suser(p) || suid_clear)) {
 			error = VOP_GETATTR(vp, &vattr, p->p_ucred, p);
 			if (error)
diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
index 44ed35962f8..f060698367d 100644
--- a/sys/sys/vnode.h
+++ b/sys/sys/vnode.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vnode.h,v 1.148 2018/08/13 15:26:17 visa Exp $	*/
+/*	$OpenBSD: vnode.h,v 1.149 2018/12/23 10:46:51 natano Exp $	*/
 /*	$NetBSD: vnode.h,v 1.38 1996/02/29 20:59:05 cgd Exp $	*/
 
 /*
@@ -586,6 +586,7 @@ struct vnode *checkalias(struct vnode *, dev_t, struct mount *);
 int	getnewvnode(enum vtagtype, struct mount *, struct vops *,
 	    struct vnode **);
 int	vaccess(enum vtype, mode_t, uid_t, gid_t, mode_t, struct ucred *);
+int	vnoperm(struct vnode *);
 void	vattr_null(struct vattr *);
 void	vdevgone(int, int, int, enum vtype);
 int	vcount(struct vnode *);
diff --git a/sys/ufs/ffs/ffs_vnops.c b/sys/ufs/ffs/ffs_vnops.c
index 09ca4bd0914..f5cf41063c6 100644
--- a/sys/ufs/ffs/ffs_vnops.c
+++ b/sys/ufs/ffs/ffs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ffs_vnops.c,v 1.92 2018/07/21 09:35:08 anton Exp $	*/
+/*	$OpenBSD: ffs_vnops.c,v 1.93 2018/12/23 10:46:51 natano Exp $	*/
 /*	$NetBSD: ffs_vnops.c,v 1.7 1996/05/11 18:27:24 mycroft Exp $	*/
 
 /*
@@ -391,7 +391,7 @@ ffs_write(void *v)
 	 * tampering.
 	 */
 	if (resid > uio->uio_resid && ap->a_cred && ap->a_cred->cr_uid != 0 &&
-	    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0)
+	    !vnoperm(vp))
 		DIP_ASSIGN(ip, mode, DIP(ip, mode) & ~(ISUID | ISGID));
 	if (resid > uio->uio_resid)
 		VN_KNOTE(vp, NOTE_WRITE | (extended ? NOTE_EXTEND : 0));
diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c
index 448edb5a3f2..3825de24188 100644
--- a/sys/ufs/ufs/ufs_lookup.c
+++ b/sys/ufs/ufs/ufs_lookup.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_lookup.c,v 1.53 2018/09/06 11:50:54 jsg Exp $	*/
+/*	$OpenBSD: ufs_lookup.c,v 1.54 2018/12/23 10:46:51 natano Exp $	*/
 /*	$NetBSD: ufs_lookup.c,v 1.7 1996/02/09 22:36:06 christos Exp $	*/
 
 /*
@@ -496,7 +496,7 @@ found:
 		if ((DIP(dp, mode) & ISVTX) &&
 		    cred->cr_uid != 0 &&
 		    cred->cr_uid != DIP(dp, uid) &&
-		    (vdp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+		    !vnoperm(vdp) &&
 		    DIP(VTOI(tdp), uid) != cred->cr_uid) {
 			vput(tdp);
 			return (EPERM);
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index 20f2a179e6b..82ebd356af0 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_vnops.c,v 1.142 2018/06/21 14:17:23 visa Exp $	*/
+/*	$OpenBSD: ufs_vnops.c,v 1.143 2018/12/23 10:46:51 natano Exp $	*/
 /*	$NetBSD: ufs_vnops.c,v 1.18 1996/05/11 18:28:04 mycroft Exp $	*/
 
 /*
@@ -274,9 +274,13 @@ ufs_access(void *v)
 	if ((mode & VWRITE) && (DIP(ip, flags) & IMMUTABLE))
 		return (EPERM);
 
-	if ((vp->v_mount->mnt_flag & MNT_NOPERM) &&
-	    (vp->v_flag & VROOT) == 0)
-		return (0);
+	if (vnoperm(vp)) {
+		/* For VEXEC, at least one of the execute bits must be set. */
+		if ((mode & VEXEC) && vp->v_type != VDIR &&
+		    (DIP(ip, mode) & (S_IXUSR|S_IXGRP|S_IXOTH)) == 0)
+			return EACCES;
+		return 0;
+	}
 
 	return (vaccess(vp->v_type, DIP(ip, mode), DIP(ip, uid), DIP(ip, gid),
 	    mode, ap->a_cred));
@@ -353,10 +357,10 @@ ufs_setattr(void *v)
 		if (vp->v_mount->mnt_flag & MNT_RDONLY)
 			return (EROFS);
 		if (cred->cr_uid != DIP(ip, uid) &&
-		    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+		    !vnoperm(vp) &&
 		    (error = suser_ucred(cred)))
 			return (error);
-		if (cred->cr_uid == 0) {
+		if (cred->cr_uid == 0 || vnoperm(vp)) {
 			if ((DIP(ip, flags) & (SF_IMMUTABLE | SF_APPEND)) &&
 			    securelevel > 0)
 				return (EPERM);
@@ -413,7 +417,7 @@ ufs_setattr(void *v)
 		if (vp->v_mount->mnt_flag & MNT_RDONLY)
 			return (EROFS);
 		if (cred->cr_uid != DIP(ip, uid) &&
-		    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+		    !vnoperm(vp) &&
 		    (error = suser_ucred(cred)) &&
 		    ((vap->va_vaflags & VA_UTIMES_NULL) == 0 || 
 		    (error = VOP_ACCESS(vp, VWRITE, cred, p))))
@@ -461,11 +465,10 @@ ufs_chmod(struct vnode *vp, int mode, struct ucred *cred, struct proc *p)
 	int error;
 
 	if (cred->cr_uid != DIP(ip, uid) &&
-	    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+	    !vnoperm(vp) &&
 	    (error = suser_ucred(cred)))
 		return (error);
-	if (cred->cr_uid &&
-	    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0) {
+	if (cred->cr_uid && !vnoperm(vp)) {
 		if (vp->v_type != VDIR && (mode & S_ISTXT))
 			return (EFTYPE);
 		if (!groupmember(DIP(ip, gid), cred) && (mode & ISGID))
@@ -505,7 +508,7 @@ ufs_chown(struct vnode *vp, uid_t uid, gid_t gid, struct ucred *cred,
 	 */
 	if ((cred->cr_uid != DIP(ip, uid) || uid != DIP(ip, uid) ||
 	    (gid != DIP(ip, gid) && !groupmember(gid, cred))) &&
-	    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+	    !vnoperm(vp) &&
 	    (error = suser_ucred(cred)))
 		return (error);
 	ogid = DIP(ip, gid);
@@ -546,12 +549,12 @@ ufs_chown(struct vnode *vp, uid_t uid, gid_t gid, struct ucred *cred,
 
 	if (ouid != uid || ogid != gid)
 		ip->i_flag |= IN_CHANGE;
-	if (ouid != uid && cred->cr_uid != 0 &&
-	    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0)
-		DIP_AND(ip, mode, ~ISUID);
-	if (ogid != gid && cred->cr_uid != 0 &&
-	    (vp->v_mount->mnt_flag & MNT_NOPERM) == 0)
-		DIP_AND(ip, mode, ~ISGID);
+	if (!vnoperm(vp)) {
+		if (ouid != uid && cred->cr_uid != 0)
+			DIP_AND(ip, mode, ~ISUID);
+		if (ogid != gid && cred->cr_uid != 0)
+			DIP_AND(ip, mode, ~ISGID);
+	}
 	return (0);
 
 error:
@@ -975,7 +978,7 @@ abortit:
 		if ((DIP(dp, mode) & S_ISTXT) && tcnp->cn_cred->cr_uid != 0 &&
 		    tcnp->cn_cred->cr_uid != DIP(dp, uid) &&
 		    DIP(xp, uid )!= tcnp->cn_cred->cr_uid &&
-		    (tdvp->v_mount->mnt_flag & MNT_NOPERM) == 0) {
+		    !vnoperm(tdvp)) {
 			error = EPERM;
 			goto bad;
 		}
@@ -1853,7 +1856,7 @@ ufs_makeinode(int mode, struct vnode *dvp, struct vnode **vpp,
 		softdep_change_linkcnt(ip, 0);
 	if ((DIP(ip, mode) & ISGID) &&
 		!groupmember(DIP(ip, gid), cnp->cn_cred) &&
-	    (dvp->v_mount->mnt_flag & MNT_NOPERM) == 0 &&
+	    !vnoperm(dvp) &&
 	    suser_ucred(cnp->cn_cred))
 		DIP_AND(ip, mode, ~ISGID);