lkml.org 
[lkml]   [2011]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRE: [PATCH v2] binfmt_elf: Fix PIE execution with randomization disabled (was Re: [RFC PATCH] binfmt_elf: Fix PIE execution with randomization disabled)
    Why not

    #if defined(CONFIG_X86) || defined(CONFIG_ARM)
    #define CHECK_PF_RANDOMIZE 1
    #else
    #define CHECK_PF_RANDOMIZE 0
    #endif
    if (CHECK_PF_RANDOMIZE && (current->flags & PF_RANDOMIZE))
    load_bias = 0;
    else
    load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);

    H.J.

    > -----Original Message-----
    > From: Nicolas Pitre [mailto:nicolas.pitre@linaro.org]
    > Sent: Monday, October 03, 2011 8:56 AM
    > To: Jiri Kosina
    > Cc: Josh Boyer; Andrew Morton; Ingo Molnar; Lu, Hongjiu; linux-
    > kernel@vger.kernel.org; Andrew Morton; Russell King
    > Subject: Re: [PATCH v2] binfmt_elf: Fix PIE execution with randomization
    > disabled (was Re: [RFC PATCH] binfmt_elf: Fix PIE execution with randomization
    > disabled)
    >
    > On Mon, 3 Oct 2011, Jiri Kosina wrote:
    >
    > > 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 <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: Nicolas Pitre <nicolas.pitre@linaro.org>
    >
    > However I'd suggest a change to deal with style issues:
    >
    > > 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
    >
    > What about this instead, saving 3 lines out of 8:
    >
    > load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
    > #if defined(CONFIG_X86) || defined(CONFIG_ARM)
    > if (current->flags & PF_RANDOMIZE)
    > load_bias = 0;
    > #endif
    >
    >
    > ?
    >
    >
    > Nicolas


    \
     
     \ /
      Last update: 2011-10-03 18:05    [W:0.033 / U:94.104 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site