Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753065AbdFUR2P (ORCPT ); Wed, 21 Jun 2017 13:28:15 -0400 Received: from mail-io0-f176.google.com ([209.85.223.176]:35277 "EHLO mail-io0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751116AbdFUR2N (ORCPT ); Wed, 21 Jun 2017 13:28:13 -0400 MIME-Version: 1.0 In-Reply-To: <1498066040.27465.7.camel@gmail.com> References: <20170621055835.GA27467@beast> <1498046876.13083.17.camel@redhat.com> <1498066040.27465.7.camel@gmail.com> From: Kees Cook Date: Wed, 21 Jun 2017 10:28:11 -0700 X-Google-Sender-Auth: zx2mo_sU22xKGffK9nGBKMKUoVQ Message-ID: Subject: Re: [kernel-hardening] [PATCH] [RFC] binfmt_elf: Use ELF_ET_DYN_BASE only for PIE To: Daniel Micay Cc: Rik van Riel , LKML , Qualys Security Advisory , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , "x86@kernel.org" , Alexander Viro , Dmitry Safonov , Masahiro Yamada , Grzegorz Andrejczuk , "linux-fsdevel@vger.kernel.org" , "kernel-hardening@lists.openwall.com" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2309 Lines: 66 On Wed, Jun 21, 2017 at 10:27 AM, Daniel Micay 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 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 -- Kees Cook Pixel Security