On Tue, 11 Jun 2002 18:33:32 -0400 (EDT)
John Baldwin <[EMAIL PROTECTED]> wrote:

> 
> Well, it shouldn't be but it is. :-P  However, one should try to avoid holding
> locks except when necessary to maximize concurrency.  Thus it is better to do
> things like malloc() and free() while not holding locks if possible.
> 
> > On which way to go:
> > I like your idea better, because it is less work and less bloat. Sometimes
> > I have to keep reminding myself: "Choose the simplest design that works."
> 
> Fair enough.  Thanks for working on this.
> 

np. It's much fun!

I don't know if you recieved my earlier email about a bug that I found in
execve() while working on fixing the "malloc w/ process lock held" bugs.
Here's a simpler patch.

It fixes possible resource leaks and failure to unlock a lock, introduced
by nectar@ in rev. 1.162 of kern/kern_exec.c, in the case where the call
to fdcheckstd() fails. Basically it fails to deallocate resources and unlock the
process lock.

Cheers,
Mike Makonnen

Index: sys/kern/kern_exec.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.164
diff -u -r1.164 kern_exec.c
--- sys/kern/kern_exec.c        7 Jun 2002 05:41:27 -0000       1.164
+++ sys/kern/kern_exec.c        14 Jun 2002 14:06:14 -0000
@@ -133,7 +133,7 @@
        struct image_params image_params, *imgp;
        struct vattr attr;
        int (*img_first)(struct image_params *);
-       struct pargs *oldargs, *newargs = NULL;
+       struct pargs *oldargs=NULL, *newargs = NULL;
        struct procsig *oldprocsig, *newprocsig;
 #ifdef KTRACE
        struct vnode *tracevp = NULL;
@@ -383,8 +383,10 @@
 #endif
                /* Make sure file descriptors 0..2 are in use.  */
                error = fdcheckstd(td);
-               if (error != 0)
-                       goto exec_fail_dealloc;
+               if (error != 0) {
+                       oldcred = NULL;
+                       goto done1;
+               }
                /*
                 * Set the new credentials.
                 */
@@ -467,6 +469,7 @@
                p->p_args = newargs;
                newargs = NULL;
        }
+done1:
        PROC_UNLOCK(p);
 
        /*
@@ -486,7 +489,8 @@
        if (tracevp != NULL)
                vrele(tracevp);
 #endif
-       pargs_drop(oldargs);
+       if (oldargs != NULL)
+               pargs_drop(oldargs);
 
 exec_fail_dealloc:
 

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to