From 312c23d957e7b6b17b569ddbb87153b1211128f1 Mon Sep 17 00:00:00 2001
From: Artur Grabowski <art@cvs.openbsd.org>
Date: Thu, 28 Sep 2000 13:41:40 +0000
Subject: When allocating the unallocated file descriptors 0, 1 and 2 for suid
 execs, don't do it by doing namei on /dev/null.

The vnode for the executed file is locked and we had a race where other
processes could lock the parent directories up to the root. When the
executing process did the lookup on /dev/null it could deadlock on the
root vnode while still holding the lock on the executed vnode.

Also, it's really bad idea to depend on certain filesystem layout inside
the kernel. Now we get the null device vnode by cdevvp(getnulldev(), ...

Thanks to Matrin Portmann <map@infinitum.ch> for providing the (large)
ktrace that allowed me to track this down.

Fixes 1369.
---
 sys/kern/kern_exec.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

(limited to 'sys')

diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 098edd5049e..47d7e22691e 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_exec.c,v 1.43 2000/09/26 14:01:39 art Exp $	*/
+/*	$OpenBSD: kern_exec.c,v 1.44 2000/09/28 13:41:39 art Exp $	*/
 /*	$NetBSD: kern_exec.c,v 1.75 1996/02/09 18:59:28 christos Exp $	*/
 
 /*-
@@ -51,6 +51,7 @@
 #include <sys/mman.h>
 #include <sys/signalvar.h>
 #include <sys/stat.h>
+#include <sys/conf.h>
 #ifdef SYSVSHM
 #include <sys/shm.h>
 #endif
@@ -535,26 +536,38 @@ sys_execve(p, v, retval)
 			 * allocated.  We do not want userland to accidentally
 			 * allocate descriptors in this range which has implied
 			 * meaning to libc.
+			 *
+			 * XXX - Shouldn't the exec fail if we can't allocate
+			 *       resources here?
 			 */
 			if (fp == NULL) {
 				short flags = FREAD | (i == 0 ? 0 : FWRITE);
-				struct nameidata nd;
+				struct vnode *vp;
 				int indx;
 
 				if ((error = falloc(p, &fp, &indx)) != 0)
-					continue;
-				NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE,
-				    "/dev/null", p);
-				if ((error = vn_open(&nd, flags, 0)) != 0) {
+					break;
+#ifdef DIAGNOSTIC
+				if (indx != i)
+					panic("sys_execve: falloc indx != i");
+#endif
+				if ((error = cdevvp(getnulldev(), &vp)) != 0) {
+					ffree(fp);
+					fdremove(p->p_fd, indx);
+					break;
+				}
+				if ((error = VOP_OPEN(vp, flags, p->p_ucred, p)) != 0) {
 					ffree(fp);
 					fdremove(p->p_fd, indx);
+					vrele(vp);
 					break;
 				}
+				if (flags & FWRITE)
+					vp->v_writecount++;
 				fp->f_flag = flags;
 				fp->f_type = DTYPE_VNODE;
 				fp->f_ops = &vnops;
-				fp->f_data = (caddr_t)nd.ni_vp;
-				VOP_UNLOCK(nd.ni_vp, 0, p);
+				fp->f_data = (caddr_t)vp;
 			}
 		}
 	} else
-- 
cgit v1.2.3