On Wed, Jun 21, 2017 at 5:07 AM, Rik van Riel <riel@xxxxxxxxxx> 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
--
Kees Cook
Pixel Security
