linux-kernel - Re: [kernel-hardening] [PATCH] [RFC] binfmt_elf: Use ELF_ET_DYN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jL7+1EUZCZfmOd5oQVfKACM7cTfCr0ezh5q7NosYHvogA@mail.gmail.com>
Date:   Wed, 21 Jun 2017 10:33:01 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Daniel Micay <danielmicay@...il.com>
Cc:     Rik van Riel <riel@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Qualys Security Advisory <qsa@...lys.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        "x86@...nel.org" <x86@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Dmitry Safonov <dsafonov@...tuozzo.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Grzegorz Andrejczuk <grzegorz.andrejczuk@...el.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>
Subject: Re: [kernel-hardening] [PATCH] [RFC] binfmt_elf: Use ELF_ET_DYN_BASE
 only for PIE

On Wed, Jun 21, 2017 at 10:32 AM, Daniel Micay <danielmicay@...il.com> wrote:
> On Wed, 2017-06-21 at 10:28 -0700, Kees Cook wrote:
>> On Wed, Jun 21, 2017 at 10:27 AM, Daniel Micay <danielmicay@...il.com>
>> wrote:
>> > On Wed, 2017-06-21 at 10:23 -0700, Kees Cook wrote:
>> > > On Wed, Jun 21, 2017 at 5:07 AM, Rik van Riel <riel@...hat.com>
>> > > wrote:
>> > > > On Tue, 2017-06-20 at 22:58 -0700, Kees Cook wrote:
>> > > > > +/* This is the base location for PIE (ET_DYN with INTERP)
>> > > > > loads.
>> > > > > */
>> > > > > +#define ELF_ET_DYN_BASE              0x400000UL
>> > > >
>> > > > This value is good for 32 bit binaries, but for 64
>> > > > bit binaries you probably want to put ELF_ET_DYN_BASE
>> > > > at 4GB or higher.
>> > > >
>> > > > The latter is necessary because Android uses the
>> > > > lower 4GB of address space for its JVM runtime,
>> > > > with 32 bit pointers inside that part of the otherwise
>> > > > 64 bit address space.
>> > > >
>> > > > In other words:
>> > > >
>> > > > #define ELF_ET_DYN_BASE (mmap_is_ia32() ? 0x400000UL :
>> > > > 0x100000000UL)
>> > >
>> > > Ah, interesting. Okay, that should be fine. I'll adjust it.
>> > >
>> > > > > +++ b/fs/binfmt_elf.c
>> > > > >
>> > > > > +                      * Therefore, programs are loaded offset
>> > > > > from
>> > > > > +                      * ELF_ET_DYN_BASE and loaders are
>> > > > > loaded
>> > > > > into the
>> > > > > +                      * independently randomized mmap region
>> > > > > (0
>> > > > > load_bias
>> > > > > +                      * without MAP_FIXED).
>> > > > > +                      */
>> > > > > +                     if (elf_interpreter) {
>> > > > > +                             load_bias = ELF_ET_DYN_BASE;
>> > > > > +                             if (current->flags &
>> > > > > PF_RANDOMIZE)
>> > > > > +                                     load_bias +=
>> > > > > arch_mmap_rnd();
>> > > > > +                             elf_flags |= MAP_FIXED;
>> > > > > +                     } else
>> > > > > +                             load_bias = 0;
>> > > > > +
>> > > > > +                     load_bias -= vaddr;
>> > > >
>> > > > I like this, and the big comment telling people how it
>> > > > works :)
>> > >
>> > > Thanks! It looks like your patch for commenting load_bias never
>> > > got
>> > > picked up, so I've added some more comments for that and some
>> > > other
>> > > things too. (Mostly for all the stuff I have to read a few times
>> > > when
>> > > I look at this code.)
>> > >
>> > > -Kees
>> > >
>> >
>> > The stack rlimit calculation fix for space potentially lost to ASLR
>> > is
>> > probably still needed too, right?
>>
>> Yes. Was that picked up by akpm already?
>>
>> -Kees
>
> I think it was dropped when the ET_DYN changes were dropped.

Ah! Rik, can you resend just the stack calculation fixes? I can Ack those.

-Kees

-- 
Kees Cook
Pixel Security