svn commit: r284157 - head/sys/kern

[Bruce Evans]

On Mon, 8 Jun 2015, Ed Maste wrote:

> Log:
>  Add user facing errors for exceeding process memory limits
>
>  Previously the process terminating with SIGABRT at startup was the
>  only notification.

I don't like this.

Errrors in syscalls should be reported by returning an error code, not
by spamming the user's terminal.

> Modified: head/sys/kern/imgact_elf.c
> ==============================================================================
> --- head/sys/kern/imgact_elf.c	Mon Jun  8 15:24:24 2015	(r284156)
> +++ head/sys/kern/imgact_elf.c	Mon Jun  8 16:07:07 2015	(r284157)
> ...
> @@ -755,11 +755,14 @@ __CONCAT(exec_, __elfN(imgact))(struct i
> 	if ((hdr->e_phoff > PAGE_SIZE) ||
> 	    (u_int)hdr->e_phentsize * hdr->e_phnum > PAGE_SIZE - hdr->e_phoff) {
> 		/* Only support headers in first page for now */
> +		uprintf("Program headers not in the first page\n");
> 		return (ENOEXEC);
> 	}
> -	phdr = (const Elf_Phdr *)(imgp->image_header + hdr->e_phoff);
> -	if (!aligned(phdr, Elf_Addr))
> +	phdr = (const Elf_Phdr *)(imgp->image_header + hdr->e_phoff);
> +	if (!aligned(phdr, Elf_Addr)) {
> +		uprintf("Unaligned program headers\n");
> 		return (ENOEXEC);
> +	}
> 	n = 0;
> 	baddr = 0;
> 	for (i = 0; i < hdr->e_phnum; i++) {

Also, the spam doesn't even include much useful info, like the program or
interpreter name.  It can be difficult to tell where the error was for
nested interpreters.

Most of the old uprintf()s for exec failures are similarly deficient.  In
this file:


X 		uprintf("elf_load_section: truncated ELF file\n");
X 		uprintf("ELF binary type \"%u\" not known.\n",

This one also has style bugs (bogus quoting of a number, and termination
with a period).

X 			uprintf("ELF interpreter %s not found\n", interp);

This one at least prints the interpreter name.

I use the following partial fixes:

X diff -u2 imgact_elf.c~ imgact_elf.c
X --- imgact_elf.c~	Sat Jun  5 16:50:05 2004
X +++ imgact_elf.c	Sat Jun  5 16:51:25 2004
X @@ -694,6 +693,6 @@
X  	brand_info = __elfN(get_brandinfo)(hdr, interp);
X  	if (brand_info == NULL) {
X -		uprintf("ELF binary type \"%u\" not known.\n",
X -		    hdr->e_ident[EI_OSABI]);
X +		uprintf("%s: ELF binary type \"%u\" not known.\n",
X +		    imgp->stringbase, hdr->e_ident[EI_OSABI]);
X  		error = ENOEXEC;
X  		goto fail;
X @@ -828,5 +827,6 @@
X  		    &imgp->entry_addr, sv->sv_pagesize);
X  		if (error != 0) {
X -			uprintf("ELF interpreter %s not found\n", interp);
X +			uprintf("%s: ELF interpreter %s not found\n",
X +			    imgp->stringbase, interp);
X  			goto fail;
X  		}

The message should be printed by the application, but there is a problem
getting enough details for interpreters.  Most applications learned to
use the err() family for printing error messages long ago, so they don't
forget to print the program name like these uprintfs, but the interpreter
name is hard to print outside of the kernel.

Bruce