On Mon, 3 Oct 2011, Josh Boyer wrote: > > > Perhaps another check here for randomize? Something like: > > > > > > #if defined(CONFIG_X86) || defined(CONFIG_ARM) > > > if (current->flags & PF_RANDOMIZE) > > > load_bias = 0; > > > else if (vaddr) > > > load_bias = 0; > > > else > > > load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE); > > > #else > > > load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr); > > > #endif > > > > > > If that's stupid, then feel free to tell me. I won't pretend like I > > > understand what is going on here yet, but based on the explanation you > > > provided that might work. > > > > I have just verified my hunch that the original patch from H.J. / Josh > > breaks ASLR completely, so Andrew, please drop it for now. > > Yes, please drop the original. Thanks. > > I am now looking into how to fix things properly. > > > > Josh, looking at what you are proposing -- do you see any reason to make > > the behavior different in #else branch and in !(current->flags & > > PF_RANDOMIZE) case? > > 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 :) From: Jiri Kosina 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 and Josh Boyer Cc: Ingo Molnar Cc: Jiri Kosina Cc: Nicolas Pitre Cc: Russell King Signed-off-by: Jiri Kosina --- 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