Re: [PATCH v2] binfmt_elf: Fix PIE execution with randomization disabled (was Re: [RFC PATCH] binfmt_elf: Fix PIE execution with randomization disabled)

[Josh Boyer <jwboyer@redhat.com>]

On Mon, Oct 03, 2011 at 05:11:47PM +0200, Jiri Kosina wrote:
> > I was mostly just trying to adapt H.J.'s patch to account for the
> > PF_RANDOMIZE case.  Looking at it a bit more, I'm not sure why they
> > would need to be different.  H.J., do you recall why you made that
> > change originally?
> 
> How about the patch below instead? It survives my testing, and I believe 
> it handles both cases properly.
> 
> Confirmation from the original bug reporter would obviously be a nice 
> bonus too :)

I built an F15 kernel with this patch included.  The testcase included
in the original bug report seems to run with and without randomization
enabled.  Looking at the ldd output on the binary shows that both cases
are working appropriately as well.

I'm happy to add my Acked-by below, but it would be nice if H.J.
confirmed as well.

> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] binfmt_elf: fix PIE execution with randomization disabled
> 
> The case of address space randomization being disabled in runtime through
> randomize_va_space sysctl is not treated properly in load_elf_binary(),
> resulting in SIGKILL coming at exec() time for certain PIE-linked binaries
> in case the randomization has been disabled at runtime prior to calling
> exec().
> 
> Handle the randomize_va_space == 0 case the same way as if we were not
> supporting .text randomization at all.
> 
> Based on original patch by H.J. Lu <hongjiu.lu@intel.com> and
> Josh Boyer <jwboyer@redhat.com>
> 
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
> Cc: Russell King <rmk@arm.linux.org.uk>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>

Acked-by: Josh Boyer <jwboyer@redhat.com>

> ---
>  fs/binfmt_elf.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index dd0fdfc..bb11fe4 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -795,7 +795,10 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
>  			 * might try to exec.  This is because the brk will
>  			 * follow the loader, and is not movable.  */
>  #if defined(CONFIG_X86) || defined(CONFIG_ARM)
> -			load_bias = 0;
> +			if (current->flags & PF_RANDOMIZE)
> +				load_bias = 0;
> +			else
> +				load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
>  #else
>  			load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
>  #endif
>