Since I don't get any answer, I guess I have not been convincing
enough. Here is the deal:
a) It is obvious that there's an inconsistency: 'error' is almost
always set before goto bad.
b) Beyond the fact that it is obvious: if you have a look at this
function, you will see that there are cases where the return
value shouldn't be ENOEXEC, at l.546 & l.697
- l.546
ELFNAME(read_from) calls vn_rdwr(), which then calls VOP_{READ/
WRITE}; the man page of VOP_{READ/WRITE} cleary points out that
"Upon success, zero is returned; otherwise, an appropriate error
code is returned."
Thus, it could return something else than ENOEXEC.
- l.697
If you take a binary with a PT_OPENBSD_RANDOMIZE section which
value is above 1024, you will see that exec() returns ENOEXEC
whereas it is expected to return ENOMEM.
Does it sound more... glamourous?
Le 26/08/2013 15:39, Maxime Villard a écrit :
> Hi,
> after playing with some ELF binaries, I figured out that error
> codes received in errno do not always match with what is expected
> in the kernel.
>
> Actually, this function should return (error) instead of (ENOEXEC):
>
>
> Index: exec_elf.c
> ===
> RCS file: /cvs/src/sys/kern/exec_elf.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 exec_elf.c
> --- exec_elf.c4 Jul 2013 17:37:05 - 1.93
> +++ exec_elf.c26 Aug 2013 15:26:06 -
> @@ -552,8 +552,10 @@ ELFNAME2(exec,makecmds)(struct proc *p,
>
> for (i = 0, pp = ph; i < eh->e_phnum; i++, pp++) {
> if (pp->p_type == PT_INTERP && !interp) {
> - if (pp->p_filesz >= MAXPATHLEN)
> + if (pp->p_filesz >= MAXPATHLEN) {
> + error = ENOEXEC;
> goto bad;
> + }
> interp = pool_get(&namei_pool, PR_WAITOK);
> if ((error = ELFNAME(read_from)(p, epp->ep_vp,
> pp->p_offset, interp, pp->p_filesz)) != 0) {
> @@ -567,8 +569,10 @@ ELFNAME2(exec,makecmds)(struct proc *p,
>
> if (eh->e_type == ET_DYN) {
> /* need an interpreter and load sections for PIE */
> - if (interp == NULL || base_ph == NULL)
> + if (interp == NULL || base_ph == NULL) {
> + error = ENOEXEC;
> goto bad;
> + }
> /* randomize exe_base for PIE */
> exe_base = uvm_map_pie(base_ph->p_align);
> }
> @@ -589,9 +593,9 @@ ELFNAME2(exec,makecmds)(struct proc *p,
>* *interp with a changed path (/emul/xxx/), and also
>* set the ep_emul field in the exec package structure.
>*/
> - error = ENOEXEC;
> if (eh->e_ident[EI_OSABI] != ELFOSABI_OPENBSD &&
> ELFNAME(os_pt_note)(p, epp, epp->ep_hdr, "OpenBSD", 8, 4) != 0) {
> + error = ENOEXEC;
> for (i = 0; ELFNAME(probes)[i].func != NULL && error; i++)
> error = (*ELFNAME(probes)[i].func)(p, epp, interp,
> &pos);
> if (error)
> @@ -760,7 +764,7 @@ bad:
> pool_put(&namei_pool, interp);
> free(ph, M_TEMP);
> kill_vmcmds(&epp->ep_vmcmds);
> - return (ENOEXEC);
> + return (error);
> }
>
> /*
>
>
> Ok/Comments?
>
>
