From owner-freebsd-current@FreeBSD.ORG Thu Apr 29 12:46:18 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 289C31065670; Thu, 29 Apr 2010 12:46:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id DBBA48FC16; Thu, 29 Apr 2010 12:46:17 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 63F3F46B29; Thu, 29 Apr 2010 08:46:17 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 087F08A021; Thu, 29 Apr 2010 08:46:05 -0400 (EDT) From: John Baldwin To: freebsd-current@freebsd.org Date: Thu, 29 Apr 2010 07:40:43 -0400 User-Agent: KMail/1.12.1 (FreeBSD/7.3-CBSD-20100217; KDE/4.3.1; amd64; ; ) References: <20100428171840.GS35381@elvis.mu.org> In-Reply-To: <20100428171840.GS35381@elvis.mu.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201004290740.43355.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 29 Apr 2010 08:46:05 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Alfred Perlstein Subject: Re: fixes for enhanced coredump X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Apr 2010 12:46:18 -0000 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