From a459a6cfe7ee2c9cc00fd2e8c0b6c3c32b8a8d2b Mon Sep 17 00:00:00 2001
From: Mateusz Guzik <mjg@FreeBSD.org>
Date: Tue, 25 Aug 2020 14:18:50 +0000
Subject: [PATCH] vfs: respect PRIV_VFS_LOOKUP in vaccess_smr

Reported by:	novel
---
 sys/kern/kern_jail.c |  2 +-
 sys/kern/kern_priv.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
 sys/kern/vfs_subr.c  | 20 ++++++++++++++---
 sys/sys/priv.h       |  2 ++
 4 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index 81000783265..742b60daf62 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -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:
diff --git a/sys/kern/kern_priv.c b/sys/kern/kern_priv.c
index 710e9b90894..05c08890c77 100644
--- a/sys/kern/kern_priv.c
+++ b/sys/kern/kern_priv.c
@@ -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));
 	}
@@ -247,6 +249,56 @@ priv_check(struct thread *td, int priv)
 	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
 priv_check_cred_vfs_generation_slow(struct ucred *cred)
 {
diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index 87990121780..49274153847 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -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, gid_t file_gid, struct ucred
 		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);
+	}
 }
 
 /*
diff --git a/sys/sys/priv.h b/sys/sys/priv.h
index f5f683f107e..7ef54782a60 100644
--- a/sys/sys/priv.h
+++ b/sys/sys/priv.h
@@ -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