On Fri, 16 Apr 2010 18:16:14 -0700
Alfred Perlstein <[email protected]> wrote:

> Can I get a review for this?  
> 
> summary:
> If doing compressed cores and there is an error, we leak
> resources unless this is fixed.
> 
> 
> Index: imgact_elf.c
> ===================================================================
> --- imgact_elf.c      (revision 206498)
> +++ imgact_elf.c      (working copy)
> @@ -29,7 +29,7 @@
>   */
>  
>  #include <sys/cdefs.h>
> -__FBSDID("$FreeBSD$");
> +__FBSDID("$FreeBSD: head/sys/kern/imgact_elf.c 205643 2010-03-25 14:31:26Z 
> nwhitehorn $");
>  
>  #include "opt_compat.h"
>  #include "opt_core.h"
> @@ -1088,8 +1088,10 @@
>       hdrsize = 0;
>       __elfN(puthdr)(td, (void *)NULL, &hdrsize, seginfo.count);
>  
> -     if (hdrsize + seginfo.size >= limit)
> -             return (EFAULT);
> +     if (hdrsize + seginfo.size >= limit) {
> +             error = EFAULT;
> +             goto done;
> +     }
>  
>       /*
>        * Allocate memory for building the header, fill it up,
> @@ -1097,7 +1099,8 @@
>        */
>       hdr = malloc(hdrsize, M_TEMP, M_WAITOK);
>       if (hdr == NULL) {
> -             return (EINVAL);
> +             error = EINVAL;
> +             goto done;
>       }
>       error = __elfN(corehdr)(td, vp, cred, seginfo.count, hdr, hdrsize,
>           gzfile);
> @@ -1125,8 +1128,8 @@
>                   curproc->p_comm, error);
>       }
>  
> +done:
>  #ifdef COMPRESS_USER_CORES
> -done:
>       if (core_buf)
>               free(core_buf, M_TEMP);
>       if (gzfile)
>

Looks good to me.

Actually, you don't need the "if (core_buf)" since free(9) DTRT with NULL.

--
Gary Jennejohn (gj@)
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[email protected]"

Reply via email to