[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-hackers
Subject:    Re: fixes for enhanced coredump
From:       Alfred Perlstein <alfred () freebsd ! org>
Date:       2010-04-28 23:42:35
Message-ID: 20100428234235.GA36233 () elvis ! mu ! org
[Download RAW message or body]

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 <alfred@freebsd.org> [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
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
[prev in list] [next in list] [prev in thread] [next in thread]