Subject: Re: eliminating veriexec #ifdefs in vfs_vnops.c
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Elad Efrat <elad@NetBSD.org>
List: tech-kern
Date: 12/30/2006 02:21:43
This is a multi-part message in MIME format.
--------------030202020204020901070700
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

YAMAMOTO Takashi wrote:
>> attached is a new diff. one namei().
>>
>> -e.
> 
> the userland pathname buffer can be modified between
> namei() and pathname_get().  is it ok for you?
> 
> YAMAMOTO Takashi

errr - yeah, forgot to do that. must be time (I'm waiting for a scrubs
re-run, you see). see attached diff.

-e.

--------------030202020204020901070700
Content-Type: text/plain;
 name="openchk.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="openchk.diff"

Index: sys/verified_exec.h
===================================================================
RCS file: /usr/cvs/src/sys/sys/verified_exec.h,v
retrieving revision 1.47
diff -u -p -r1.47 verified_exec.h
--- sys/verified_exec.h	26 Dec 2006 07:50:40 -0000	1.47
+++ sys/verified_exec.h	28 Dec 2006 13:51:26 -0000
@@ -122,6 +122,7 @@ int veriexec_removechk(struct vnode *, c
 int veriexec_renamechk(struct vnode *, const char *, struct vnode *,
     const char *, struct lwp *);
 int veriexec_unmountchk(struct mount *);
+int veriexec_openchk(struct lwp *, struct vnode *, const char *, int);
 #endif /* _KERNEL */
 
 #endif /* !_SYS_VERIFIED_EXEC_H_ */
Index: kern/vfs_vnops.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.129
diff -u -p -r1.129 vfs_vnops.c
--- kern/vfs_vnops.c	30 Nov 2006 01:09:47 -0000	1.129
+++ kern/vfs_vnops.c	28 Dec 2006 13:50:51 -0000
@@ -104,29 +104,7 @@ vn_open(struct nameidata *ndp, int fmode
 	kauth_cred_t cred = l->l_cred;
 	struct vattr va;
 	int error;
-#if NVERIEXEC > 0
-	const char *pathbuf;
-	char *tmppathbuf;
-	boolean_t veriexec_monitored = FALSE;
-#endif /* NVERIEXEC > 0 */
-
-#if NVERIEXEC > 0
-	if (ndp->ni_segflg == UIO_USERSPACE) {	
-		tmppathbuf = PNBUF_GET();
-		error = copyinstr(ndp->ni_dirp, tmppathbuf, MAXPATHLEN,
-		    NULL);
-		if (error) {
-			if (veriexec_verbose >= 1)
-				log(LOG_NOTICE, "Veriexec: Can't copy path."
-				    " (error=%d)\n", error);
-			goto bad2;
-		}
-		pathbuf = tmppathbuf;
-	} else {
-		tmppathbuf = NULL;
-		pathbuf = ndp->ni_dirp;
-	}
-#endif /* NVERIEXEC > 0 */
+	pathname_t pn = NULL;
 
 restart:
 	if (fmode & O_CREAT) {
@@ -135,23 +113,31 @@ restart:
 		if ((fmode & O_EXCL) == 0 &&
 		    ((fmode & O_NOFOLLOW) == 0))
 			ndp->ni_cnd.cn_flags |= FOLLOW;
-		if ((error = namei(ndp)) != 0)
-			goto bad2;
-		if (ndp->ni_vp == NULL) {
-#if NVERIEXEC > 0
-			/* Lockdown mode: Prevent creation of new files. */
-			if (veriexec_strict >= VERIEXEC_LOCKDOWN) {
-				VOP_ABORTOP(ndp->ni_dvp, &ndp->ni_cnd);
+	} else {
+		ndp->ni_cnd.cn_nameiop = LOOKUP;
+		ndp->ni_cnd.cn_flags = LOCKLEAF;
+		if ((fmode & O_NOFOLLOW) == 0)
+			ndp->ni_cnd.cn_flags |= FOLLOW;
+	}
+	error = pathname_get(ndp->ni_dirp, ndp->ni_segflg, &pn);
+	if (error)
+		goto bad2;
+	ndp->ni_dirp = pathname_path(pn);
+	ndp->ni_segflg = UIO_SYSSPACE;
+	error = namei(ndp);
+	if (error)
+		goto bad2;
 
-				log(LOG_ALERT, "Veriexec: Preventing "
-				    "new file creation in %s.\n", pathbuf);
+	vp = ndp->ni_vp;
 
-				vp = ndp->ni_dvp;
-				error = EPERM;
-				goto bad;
-			}
+#if NVERIEXEC > 0
+	error = veriexec_openchk(l, ndp->ni_vp, ndp->ni_dirp, fmode);
+	if (error)
+		goto bad;
 #endif /* NVERIEXEC > 0 */
 
+	if (fmode & O_CREAT) {
+		if (ndp->ni_vp == NULL) {
 			VATTR_NULL(&va);
 			va.va_type = VREG;
 			va.va_mode = cmode;
@@ -188,12 +174,6 @@ restart:
 			fmode &= ~O_CREAT;
 		}
 	} else {
-		ndp->ni_cnd.cn_nameiop = LOOKUP;
-		ndp->ni_cnd.cn_flags = LOCKLEAF;
-		if ((fmode & O_NOFOLLOW) == 0)
-			ndp->ni_cnd.cn_flags |= FOLLOW;
-		if ((error = namei(ndp)) != 0)
-			goto bad2;
 		vp = ndp->ni_vp;
 	}
 	if (vp->v_type == VSOCK) {
@@ -206,12 +186,6 @@ restart:
 	}
 
 	if ((fmode & O_CREAT) == 0) {
-#if NVERIEXEC > 0
-		if ((error = veriexec_verify(l, vp, pathbuf, VERIEXEC_FILE,
-		    &veriexec_monitored)) != 0)
-			goto bad;
-#endif /* NVERIEXEC > 0 */
-
 		if (fmode & FREAD) {
 			if ((error = VOP_ACCESS(vp, VREAD, cred, l)) != 0)
 				goto bad;
@@ -225,45 +199,10 @@ restart:
 			if ((error = vn_writechk(vp)) != 0 ||
 			    (error = VOP_ACCESS(vp, VWRITE, cred, l)) != 0)
 				goto bad;
-#if NVERIEXEC > 0
-			if (veriexec_monitored) {
-				veriexec_report("Write access request.",
-				    pathbuf, l, REPORT_ALWAYS|REPORT_ALARM);
-
-				/* IPS mode: Deny writing to monitored files. */
-				if (veriexec_strict >= VERIEXEC_IPS) {
-					error = EPERM;
-					goto bad;
-				} else {
-					veriexec_purge(vp);
-				}
-			}
-#endif /* NVERIEXEC > 0 */
 		}
 	}
 
 	if (fmode & O_TRUNC) {
-#if NVERIEXEC > 0 
-		if ((error = veriexec_verify(l, vp, pathbuf, VERIEXEC_FILE,
-		    &veriexec_monitored)) != 0) {
-			/*VOP_UNLOCK(vp, 0);*/
-			goto bad;
-		}
-
-		if (veriexec_monitored) {
-			veriexec_report("Truncate access request.", pathbuf, l,
-			    REPORT_VERBOSE | REPORT_ALARM);
-
-			/* IPS mode: Deny truncating monitored files. */
-			if (veriexec_strict >= 2) {
-				error = EPERM;
-				goto bad;
-			} else {
-				veriexec_purge(vp);
-			}
-		}
-#endif /* NVERIEXEC > 0 */
-
 		VOP_UNLOCK(vp, 0);			/* XXX */
 
 		if ((error = vn_start_write(vp, &mp, V_WAIT | V_PCATCH)) != 0) {
@@ -294,10 +233,7 @@ bad:
 		vput(vp);
 
 bad2:
-#if NVERIEXEC > 0
-	if (tmppathbuf != NULL)
-		PNBUF_PUT(tmppathbuf);
-#endif /* NVERIEXEC > 0 */
+	pathname_put(pn);
 
 	return (error);
 }
Index: kern/kern_verifiedexec.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/kern_verifiedexec.c,v
retrieving revision 1.87
diff -u -p -r1.87 kern_verifiedexec.c
--- kern/kern_verifiedexec.c	29 Dec 2006 11:34:14 -0000	1.87
+++ kern/kern_verifiedexec.c	28 Dec 2006 13:52:17 -0000
@@ -64,6 +64,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_verifie
 #include <sys/conf.h>
 #include <miscfs/specfs/specdev.h>
 #include <prop/proplib.h>
+#include <sys/fcntl.h>
 
 MALLOC_DEFINE(M_VERIEXEC, "Veriexec", "Veriexec data-structures");
 
@@ -1229,3 +1230,46 @@ veriexec_unmountchk(struct mount *mp)
 
 	return (error);
 }
+
+int
+veriexec_openchk(struct lwp *l, struct vnode *vp, const char *path, int fmode)
+{
+	boolean_t monitored = FALSE;
+	int error = 0;
+
+	if (vp == NULL) {
+		/* If no creation requested, let this fail normally. */
+		if (!(fmode & O_CREAT)) {
+			error = 0;
+			goto out;
+		}
+
+		/* Lockdown mode: Prevent creation of new files. */
+		if (veriexec_strict >= VERIEXEC_LOCKDOWN) {
+			log(LOG_ALERT, "Veriexec: Preventing new file "
+			    "creation in `%s'.\n", path);
+			error = EPERM;
+		}
+
+		goto out;
+	}
+
+	error = veriexec_verify(l, vp, path, VERIEXEC_FILE,
+	    &monitored);
+	if (error)
+		goto out;
+
+	if (monitored && ((fmode & FWRITE) || (fmode & O_TRUNC))) {
+		veriexec_report("Write access request.", path, l,
+		    REPORT_ALWAYS | REPORT_ALARM);
+
+		/* IPS mode: Deny writing to/truncating monitored files. */
+		if (veriexec_strict >= VERIEXEC_IPS)
+			error = EPERM;
+		else
+			veriexec_purge(vp);
+	}
+
+ out:
+	return (error);
+}

--------------030202020204020901070700--