Re: [PATCH] binfmt_elf: fix return value in case of interpreterload failure
From: Andrew Morton
Date: Thu Apr 11 2013 - 18:04:21 EST
On Thu, 11 Apr 2013 15:53:09 +0200 Matthieu CASTET <matthieu.castet@xxxxxxxxxx> wrote:
> The current code return the address instead of using PTR_ERR.
I don't understand what you mean here - please describe this error in
much more detail. Help people to identify the section of code which
is being discussed.
> Also the check is done after adding e_entry. This can cause weird behaviour
> because -errno + loc->interp_elf_ex.e_entry can produce a valid address.
Which check?
> Add a check to test load error before adding entry address. Also in this
> case send SIGKILL instead of SIGSEGV to match what is done when loading binary.
>
> ...
>
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -900,18 +900,21 @@ static int load_elf_binary(struct linux_binprm *bprm)
> interpreter,
> &interp_map_addr,
> load_bias);
> - if (!IS_ERR((void *)elf_entry)) {
> - /*
> - * load_elf_interp() returns relocation
> - * adjustment
> - */
> - interp_load_addr = elf_entry;
> - elf_entry += loc->interp_elf_ex.e_entry;
> + if (BAD_ADDR(elf_entry)) {
> + force_sig(SIGKILL, current);
> + retval = IS_ERR((void *)elf_entry) ?
> + PTR_ERR((void *)elf_entry) : -EINVAL;
Thats's a bit verbose - "PTR_ERR((void *)elf_entry)" is equivalent to
"elf_entry". I suppose we can do it this way to document the intent or
something.
It would be helpful if load_elf_interp() had some documentation
describing its return value btw.
> + goto out_free_dentry;
> }
> + /*
> + * load_elf_interp() returns relocation
> + * adjustment
This can now be converted to a single-line comment.
> + */
> + interp_load_addr = elf_entry;
> + elf_entry += loc->interp_elf_ex.e_entry;
> if (BAD_ADDR(elf_entry)) {
> force_sig(SIGSEGV, current);
> - retval = IS_ERR((void *)elf_entry) ?
> - (int)elf_entry : -EINVAL;
> + retval = -EINVAL;
> goto out_free_dentry;
> }
> reloc_func_desc = interp_load_addr;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/