Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933621AbbESPBE (ORCPT ); Tue, 19 May 2015 11:01:04 -0400 Received: from mail-ob0-f179.google.com ([209.85.214.179]:35580 "EHLO mail-ob0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932830AbbESPBB (ORCPT ); Tue, 19 May 2015 11:01:01 -0400 MIME-Version: 1.0 X-Originating-IP: [217.156.156.35] In-Reply-To: <1428965343-17762-1-git-send-email-md@google.com> References: <1428965343-17762-1-git-send-email-md@google.com> Date: Tue, 19 May 2015 16:01:00 +0100 X-Google-Sender-Auth: oh7zM1sE0xglZV2E0YLKCr1Eldc Message-ID: Subject: Re: [PATCH] binfmt_elf: Fix bug in loading of PIE binaries. From: James Hogan To: Michael Davidson Cc: Alexander Viro , Jiri Kosina , Andrew Morton , linux-fsdevel@vger.kernel.org, LKML , James Hogan 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: 3524 Lines: 85 Hi Michael, On 13 April 2015 at 23:49, Michael Davidson wrote: > With CONFIG_ARCH_BINFMT_ELF_RANDOMIZE_PIE enabled, and a normal > top-down address allocation strategy, load_elf_binary() will > attempt to map a PIE binary into an address range immediately > below mm->mmap_base. > > Unfortunately, load_elf_ binary() does not take account of the > need to allocate sufficient space for the entire binary which > means that, while the first PT_LOAD segment is mapped below > mm->mmap_base, the subsequent PT_LOAD segment(s) end up being > mapped above mm->mmap_base into the are that is supposed to > be the "gap" between the stack and the binary. > > Since the size of the "gap" on x86_64 is only guaranteed to be > 128MB this means that binaries with large data segments > 128MB > can end up mapping part of their data segment over their stack > resulting in corruption of the stack (and the data segment once > the binary starts to run). > > Any PIE binary with a data segment > 128MB is vulnerable to this > although address randomization means that the actual gap between > the stack and the end of the binary is normally greater than 128MB. > The larger the data segment of the binary the higher the probability > of failure. > > Fix this by calculating the total size of the binary in the same > way as load_elf_interp(). > > Signed-off-by: Michael Davidson > --- > fs/binfmt_elf.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index 995986b..d925f55 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -862,6 +862,7 @@ static int load_elf_binary(struct linux_binprm *bprm) > i < loc->elf_ex.e_phnum; i++, elf_ppnt++) { > int elf_prot = 0, elf_flags; > unsigned long k, vaddr; > + unsigned long total_size = 0; > > if (elf_ppnt->p_type != PT_LOAD) > continue; > @@ -924,10 +925,16 @@ static int load_elf_binary(struct linux_binprm *bprm) > #else > load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr); > #endif > + total_size = total_mapping_size(elf_phdata, > + loc->elf_ex.e_phnum); > + if (!total_size) { > + error = -EINVAL; I was just printk debugging this function, and this stood out. Should that be retval instead of error? Cheers James > + goto out_free_dentry; > + } > } > > error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt, > - elf_prot, elf_flags, 0); > + elf_prot, elf_flags, total_size); > if (BAD_ADDR(error)) { > retval = IS_ERR((void *)error) ? > PTR_ERR((void*)error) : -EINVAL; > -- > 2.2.0.rc0.207.ga3a616c > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/