Date: Tue, 10 Sep 2002 20:22:52 -0700 (PDT) From: Nate Lawson <nate@root.org> To: Don Lewis <dl-freebsd@catspoiler.org> Cc: current@FreeBSD.ORG Subject: Re: Locking problems in exec Message-ID: <Pine.BSF.4.21.0209102018080.19925-100000@root.org> In-Reply-To: <200209110209.g8B29Wwr096594@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 10 Sep 2002, Don Lewis wrote: > On 7 Sep, Garrett Wollman wrote: > > I just noted the following: > > > > ../../../vm/uma_core.c:1332: could sleep with "process lock" locked from ../../../kern/kern_exec.c:368 > > lock order reversal > > 1st 0xc438e6a8 process lock (process lock) @ ../../../kern/kern_exec.c:368 > > 2nd 0xc0413d20 filelist lock (filelist lock) @ ../../../kern/kern_descrip.c:1133 > > ../../../vm/uma_core.c:1332: could sleep with "process lock" locked from ../../../kern/kern_exec.c:368 > > > > $ ident /sys/kern/kern_exec.c > > /sys/kern/kern_exec.c: > > $FreeBSD: src/sys/kern/kern_exec.c,v 1.189 2002/09/05 07:30:14 davidxu Exp $ > > $ ident /sys/kern/kern_descrip.c > > /sys/kern/kern_descrip.c: > > $FreeBSD: src/sys/kern/kern_descrip.c,v 1.158 2002/09/03 20:16:31 jhb Exp $ > > > > This occurred when starting up Netscape. > > I just saw this in a recent version of -stable while running mozilla. I > think the problem is in the following set?id handling code in execve(): > > /* Close any file descriptors 0..2 that reference procfs */ > setugidsafety(td); > /* Make sure file descriptors 0..2 are in use. */ > error = fdcheckstd(td); > if (error != 0) > goto done1; > > When I looked at this code before, I assumed that fdcheckstd() just > verified that the proper fds where in use, but on closer inspection it > appears that fdcheckstd() attempts to point any unused fds in 0..2 to > /dev/null. This calls falloc(), which calls uma_zalloc() and grabs the > filelist lock, all of which happens while execve() holds the process > lock. The do_dup() call in fdcheckstd() could also be a problem. > > I don't think dropping the process lock is correct, and calling falloc() > and vn_open() before grabbing the process lock might also be insecure. > Other than just causing execve() to fail in this situation, the only > other fix that I can think of would be to preallocate the struct file > and pre-open /dev/null if we're execing a set?id executable, and then > jam this into the descriptor table if it turns out to be needed. Barf! I'm not sure why fdcheckstd() and setugidsafety() couldn't both happen before grabbing the proc lock. Dropping locks in the middle or pre-allocating should always be a last resort. -Nate To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0209102018080.19925-100000>