Subject: Re: veriexec by default (Re: CVS commit: src/sys/arch)
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Elad Efrat <elad@NetBSD.org>
List: tech-kern
Date: 10/27/2006 14:38:44
This is a multi-part message in MIME format.

--Boundary_(ID_Rs+x5fZQeEfOzkkCdafaRA)
Content-type: text/plain; charset=ISO-8859-1
Content-transfer-encoding: 7BIT

YAMAMOTO Takashi wrote:
>>>> vn_open uses MAXPATHLEN-sized buffer on kernel stack.
>> just like lots of other code... but okay, this can be fixed.
> 
> lots?  where?  if so, they should be fixed as well.

okay not lots :) I just grepped it and saw it's not as many (in /kern
anyway) as I expected.

anyway, see attached diff, would something like that be okay?

-e.

-- 
Elad Efrat

--Boundary_(ID_Rs+x5fZQeEfOzkkCdafaRA)
Content-type: text/plain; name=vfs_vnops.c.diff
Content-transfer-encoding: 7BIT
Content-disposition: inline; filename=vfs_vnops.c.diff

Index: vfs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.125
diff -u -p -r1.125 vfs_vnops.c
--- vfs_vnops.c	5 Oct 2006 14:48:32 -0000	1.125
+++ vfs_vnops.c	27 Oct 2006 12:37:09 -0000
@@ -105,21 +105,21 @@ vn_open(struct nameidata *ndp, int fmode
 	int error;
 #if NVERIEXEC > 0
 	struct veriexec_file_entry *vfe = NULL;
-	char pathbuf[MAXPATHLEN];
-	size_t pathlen;
-	int (*copyfun)(const void *, void *, size_t, size_t *) =
-	    ndp->ni_segflg == UIO_SYSSPACE ? copystr : copyinstr;
+	char *pathbuf;
 #endif /* NVERIEXEC > 0 */
 
 #if NVERIEXEC > 0
-	error = (*copyfun)(ndp->ni_dirp, pathbuf, sizeof(pathbuf), &pathlen);
-	if (error) {
-		if (veriexec_verbose >= 1)
-			printf("veriexec: Can't copy path. (error=%d)\n",
-			    error);
-
-		return (error);
-	}
+	if (ndp->ni_segflg == UIO_USERSPACE) {	
+		pathbuf = PNBUF_GET();
+		error = copyinstr(ndp->ni_dirp, pathbuf, MAXPATHLEN, NULL);
+		if (error) {
+			if (veriexec_verbose >= 1)
+				printf("Veriexec: Can't copy path. (err=%d\n",
+				    error);
+			goto bad2;
+		}
+	} else
+		pathbuf = __UNCONST(ndp->ni_dirp);
 #endif /* NVERIEXEC > 0 */
 
 restart:
@@ -130,7 +130,7 @@ restart:
 		    ((fmode & O_NOFOLLOW) == 0))
 			ndp->ni_cnd.cn_flags |= FOLLOW;
 		if ((error = namei(ndp)) != 0)
-			return (error);
+			goto bad2;
 		if (ndp->ni_vp == NULL) {
 #if NVERIEXEC > 0
 			/* Lockdown mode: Prevent creation of new files. */
@@ -157,7 +157,7 @@ restart:
 				vput(ndp->ni_dvp);
 				if ((error = vn_start_write(NULL, &mp,
 				    V_WAIT | V_SLEEPONLY | V_PCATCH)) != 0)
-					return (error);
+					goto bad2;
 				goto restart;
 			}
 			VOP_LEASE(ndp->ni_dvp, l, cred, LEASE_WRITE);
@@ -165,7 +165,7 @@ restart:
 					   &ndp->ni_cnd, &va);
 			vn_finished_write(mp, 0);
 			if (error)
-				return (error);
+				goto bad2;
 			fmode &= ~O_TRUNC;
 			vp = ndp->ni_vp;
 		} else {
@@ -188,7 +188,7 @@ restart:
 		if ((fmode & O_NOFOLLOW) == 0)
 			ndp->ni_cnd.cn_flags |= FOLLOW;
 		if ((error = namei(ndp)) != 0)
-			return (error);
+			goto bad2;
 		vp = ndp->ni_vp;
 	}
 	if (vp->v_type == VSOCK) {
@@ -264,7 +264,7 @@ restart:
 
 		if ((error = vn_start_write(vp, &mp, V_WAIT | V_PCATCH)) != 0) {
 			vrele(vp);
-			return (error);
+			goto bad2;
 		}
 		VOP_LEASE(vp, l, cred, LEASE_WRITE);
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);	/* XXX */
@@ -288,6 +288,11 @@ restart:
 	return (0);
 bad:
 	vput(vp);
+bad2:
+#if NVERIEXEC > 0
+	if (ndp->ni_segflg == UIO_USERSPACE)
+		PNBUF_PUT(pathbuf);
+#endif /* NVERIEXEC > 0 */
 	return (error);
 }
 

--Boundary_(ID_Rs+x5fZQeEfOzkkCdafaRA)--