Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 25 Aug 2020 14:18:50 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r364769 - in head/sys: kern sys
Message-ID:  <202008251418.07PEIoBB086223@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mjg
Date: Tue Aug 25 14:18:50 2020
New Revision: 364769
URL: https://svnweb.freebsd.org/changeset/base/364769

Log:
  vfs: respect PRIV_VFS_LOOKUP in vaccess_smr
  
  Reported by:	novel

Modified:
  head/sys/kern/kern_jail.c
  head/sys/kern/kern_priv.c
  head/sys/kern/vfs_subr.c
  head/sys/sys/priv.h

Modified: head/sys/kern/kern_jail.c
==============================================================================
--- head/sys/kern/kern_jail.c	Tue Aug 25 13:45:06 2020	(r364768)
+++ head/sys/kern/kern_jail.c	Tue Aug 25 14:18:50 2020	(r364769)
@@ -3049,6 +3049,7 @@ prison_priv_check(struct ucred *cred, int priv)
 	 * called for them. See priv_check_cred().
 	 */
 	switch (priv) {
+	case PRIV_VFS_LOOKUP:
 	case PRIV_VFS_GENERATION:
 		KASSERT(0, ("prison_priv_check instead of a custom handler "
 		    "called for %d\n", priv));
@@ -3277,7 +3278,6 @@ prison_priv_check(struct ucred *cred, int priv)
 	case PRIV_VFS_WRITE:
 	case PRIV_VFS_ADMIN:
 	case PRIV_VFS_EXEC:
-	case PRIV_VFS_LOOKUP:
 	case PRIV_VFS_BLOCKRESERVE:	/* XXXRW: Slightly surprising. */
 	case PRIV_VFS_CHFLAGS_DEV:
 	case PRIV_VFS_CHOWN:

Modified: head/sys/kern/kern_priv.c
==============================================================================
--- head/sys/kern/kern_priv.c	Tue Aug 25 13:45:06 2020	(r364768)
+++ head/sys/kern/kern_priv.c	Tue Aug 25 14:18:50 2020	(r364769)
@@ -129,6 +129,8 @@ priv_check_cred(struct ucred *cred, int priv)
 	    priv));
 
 	switch (priv) {
+	case PRIV_VFS_LOOKUP:
+		return (priv_check_cred_vfs_lookup(cred));
 	case PRIV_VFS_GENERATION:
 		return (priv_check_cred_vfs_generation(cred));
 	}
@@ -245,6 +247,56 @@ priv_check(struct thread *td, int priv)
 	KASSERT(td == curthread, ("priv_check: td != curthread"));
 
 	return (priv_check_cred(td->td_ucred, priv));
+}
+
+static int __noinline
+priv_check_cred_vfs_lookup_slow(struct ucred *cred)
+{
+	int error;
+
+	error = priv_check_cred_pre(cred, PRIV_VFS_LOOKUP);
+	if (error)
+		goto out;
+
+	if (cred->cr_uid == 0 && suser_enabled) {
+		error = 0;
+		goto out;
+	}
+
+	return (priv_check_cred_post(cred, PRIV_VFS_LOOKUP, error, false));
+out:
+	return (priv_check_cred_post(cred, PRIV_VFS_LOOKUP, error, true));
+
+}
+
+int
+priv_check_cred_vfs_lookup(struct ucred *cred)
+{
+	int error;
+
+	if (__predict_false(mac_priv_check_fp_flag ||
+	    mac_priv_grant_fp_flag || SDT_PROBES_ENABLED()))
+		return (priv_check_cred_vfs_lookup_slow(cred));
+
+	error = EPERM;
+	if (cred->cr_uid == 0 && suser_enabled)
+		error = 0;
+	return (error);
+}
+
+int
+priv_check_cred_vfs_lookup_nomac(struct ucred *cred)
+{
+	int error;
+
+	if (__predict_false(mac_priv_check_fp_flag ||
+	    mac_priv_grant_fp_flag || SDT_PROBES_ENABLED()))
+		return (EAGAIN);
+
+	error = EPERM;
+	if (cred->cr_uid == 0 && suser_enabled)
+		error = 0;
+	return (error);
 }
 
 static int __noinline

Modified: head/sys/kern/vfs_subr.c
==============================================================================
--- head/sys/kern/vfs_subr.c	Tue Aug 25 13:45:06 2020	(r364768)
+++ head/sys/kern/vfs_subr.c	Tue Aug 25 14:18:50 2020	(r364769)
@@ -5045,6 +5045,7 @@ vn_isdisk(struct vnode *vp)
 int
 vaccess_vexec_smr(mode_t file_mode, uid_t file_uid, gid_t file_gid, struct ucred *cred)
 {
+	int error;
 
 	VFS_SMR_ASSERT_ENTERED();
 
@@ -5067,7 +5068,9 @@ vaccess_vexec_smr(mode_t file_mode, uid_t file_uid, gi
 		return (0);
 out_error:
 	/*
-	 * Permission check failed.
+	 * Permission check failed, but it is possible denial will get overwritten
+	 * (e.g., when root is traversing through a 700 directory owned by someone
+	 * else).
 	 *
 	 * vaccess() calls priv_check_cred which in turn can descent into MAC
 	 * modules overriding this result. It's quite unclear what semantics
@@ -5075,9 +5078,20 @@ out_error:
 	 * from within the SMR section. This also means if any such modules
 	 * are present, we have to let the regular lookup decide.
 	 */
-	if (__predict_false(mac_priv_check_fp_flag || mac_priv_grant_fp_flag))
+	error = priv_check_cred_vfs_lookup_nomac(cred);
+	switch (error) {
+	case 0:
+		return (0);
+	case EAGAIN:
+		/*
+		 * MAC modules present.
+		 */
 		return (EAGAIN);
-	return (EACCES);
+	case EPERM:
+		return (EACCES);
+	default:
+		return (error);
+	}
 }
 
 /*

Modified: head/sys/sys/priv.h
==============================================================================
--- head/sys/sys/priv.h	Tue Aug 25 13:45:06 2020	(r364768)
+++ head/sys/sys/priv.h	Tue Aug 25 14:18:50 2020	(r364769)
@@ -536,6 +536,8 @@ struct thread;
 struct ucred;
 int	priv_check(struct thread *td, int priv);
 int	priv_check_cred(struct ucred *cred, int priv);
+int	priv_check_cred_vfs_lookup(struct ucred *cred);
+int	priv_check_cred_vfs_lookup_nomac(struct ucred *cred);
 int	priv_check_cred_vfs_generation(struct ucred *cred);
 #endif

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202008251418.07PEIoBB086223>