Date: Thu, 29 Apr 2010 07:40:43 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-current@freebsd.org Cc: Alfred Perlstein <alfred@freebsd.org> Subject: Re: fixes for enhanced coredump Message-ID: <201004290740.43355.jhb@freebsd.org> In-Reply-To: <20100428171840.GS35381@elvis.mu.org> References: <20100428171840.GS35381@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 28 April 2010 1:18:40 pm Alfred Perlstein wrote:
> I was recently working on the enhanced coredumps
> internal to Juniper and realized that there were
> some defects in the code I pushed (mostly due to
> mismerge), can someone please review?
>
> 1) don't allocate hostname[] on the stack
> 2) don't leak the temp buffer in imgact_elf_coredump.
Doesn't this leak hostname? I don't see it being free'd anywhere.
> thank you,
> -Alfred
>
>
> Index: kern/kern_sig.c
> ===================================================================
> --- kern/kern_sig.c (revision 207329)
> +++ kern/kern_sig.c (working copy)
> @@ -3004,8 +3004,9 @@
> char *temp;
> size_t i;
> int indexpos;
> - char hostname[MAXHOSTNAMELEN];
> + char *hostname;
>
> + hostname = NULL;
> format = corefilename;
> temp = malloc(MAXPATHLEN, M_TEMP, M_NOWAIT | M_ZERO);
> if (temp == NULL)
> @@ -3021,6 +3022,19 @@
> sbuf_putc(&sb, '%');
> break;
> case 'H': /* hostname */
> + if (hostname == NULL) {
> + hostname = malloc(MAXHOSTNAMELEN,
> + M_TEMP, M_NOWAIT);
> + if (hostname == NULL) {
> + log(LOG_ERR,
> + "pid %ld (%s), uid (%lu): "
> + "unable to alloc memory "
> + "for corefile hostname\n",
> + (long)pid, name,
> + (u_long)uid);
> + goto nomem;
> + }
> + }
> getcredhostname(td->td_ucred, hostname,
> sizeof(hostname));
> sbuf_printf(&sb, "%s", hostname);
> @@ -3054,9 +3068,10 @@
> }
> #endif
> if (sbuf_overflowed(&sb)) {
> - sbuf_delete(&sb);
> log(LOG_ERR, "pid %ld (%s), uid (%lu): corename is too "
> "long\n", (long)pid, name, (u_long)uid);
> +nomem:
> + sbuf_delete(&sb);
> free(temp, M_TEMP);
> return (NULL);
> }
> Index: kern/imgact_elf.c
> ===================================================================
> --- kern/imgact_elf.c (revision 207329)
> +++ kern/imgact_elf.c (working copy)
> @@ -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)
> --
> - Alfred Perlstein
> .- AMA, VMOA #5191, 03 vmax, 92 gs500, 85 ch250, 07 zx10
> .- FreeBSD committer
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
>
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201004290740.43355.jhb>