Re: fixes for enhanced coredump

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)
> _at__at_ -3004,8 +3004,9 _at__at_
>  	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)
> _at__at_ -3021,6 +3022,19 _at__at_
>  				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);
> _at__at_ -3054,9 +3068,10 _at__at_
>  	}
>  #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)
> _at__at_ -1088,8 +1088,10 _at__at_
>  	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,
> _at__at_ -1097,7 +1099,8 _at__at_
>  	 */
>  	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);
> _at__at_ -1125,8 +1128,8 _at__at_
>  		    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_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"
> 

-- 
John Baldwin