From owner-freebsd-hackers@FreeBSD.ORG Wed Apr 28 23:42:36 2010 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 83BF0106566C for ; Wed, 28 Apr 2010 23:42:36 +0000 (UTC) (envelope-from bright@elvis.mu.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 7674D8FC15 for ; Wed, 28 Apr 2010 23:42:36 +0000 (UTC) Received: by elvis.mu.org (Postfix, from userid 1192) id 389F41A3D86; Wed, 28 Apr 2010 16:42:35 -0700 (PDT) Date: Wed, 28 Apr 2010 16:42:35 -0700 From: Alfred Perlstein To: hackers@freebsd.org Message-ID: <20100428234235.GA36233@elvis.mu.org> References: <20100428171840.GS35381@elvis.mu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100428171840.GS35381@elvis.mu.org> User-Agent: Mutt/1.4.2.3i Cc: Subject: Re: fixes for enhanced coredump X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Apr 2010 23:42:36 -0000 Additionally I need to remove all traces of IO_NODELOCKED from kern_gzio.c as they leads to unlocked vnode access otherwise in the gzip coredump routine. A review would be much appreciated. thank you, -Alfred * Alfred Perlstein [100428 10:18] 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. > > 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 -- - Alfred Perlstein .- AMA, VMOA #5191, 03 vmax, 92 gs500, 85 ch250, 07 zx10 .- FreeBSD committer