From owner-freebsd-current@FreeBSD.ORG  Wed Apr 28 17:18:40 2010
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: current@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id AC1C0106566B
	for <current@freebsd.org>; Wed, 28 Apr 2010 17:18:40 +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 9FBA98FC19
	for <current@freebsd.org>; Wed, 28 Apr 2010 17:18:40 +0000 (UTC)
Received: by elvis.mu.org (Postfix, from userid 1192)
	id 5855A1A3DBB; Wed, 28 Apr 2010 10:18:40 -0700 (PDT)
Date: Wed, 28 Apr 2010 10:18:40 -0700
From: Alfred Perlstein <alfred@freebsd.org>
To: current@freebsd.org
Message-ID: <20100428171840.GS35381@elvis.mu.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.4.2.3i
Cc: 
Subject: 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
	<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>, 
	<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 28 Apr 2010 17:18:40 -0000

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