Re: fixes for enhanced coredump

* John Baldwin <jhb_at_freebsd.org> [100429 05:46] wrote:
> 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, I missed bringing that line over from JUNOS.

-Alfred

> 
> > 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

-- 
- Alfred Perlstein
.- AMA, VMOA #5191, 03 vmax, 92 gs500, 85 ch250, 07 zx10
.- FreeBSD committer