From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752678AbbIOCKd (ORCPT ); Mon, 14 Sep 2015 22:10:33 -0400 Received: from icp-osb-irony-out8.external.iinet.net.au ([203.59.1.225]:23739 "EHLO icp-osb-irony-out8.external.iinet.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751847AbbIOCKb (ORCPT ); Mon, 14 Sep 2015 22:10:31 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApIBAF9991VnMNI1/2dsb2JhbAANUIN3ab9LhXkCghEBAQEBAQGFLgEBAQQnUQEQCw0BBwMJFg8JAwIBAgFFBg0GAgEBiBQBIrValR4BAQEBAQEBAQIBAQEBAQEBAQEVBIZzhH2EWjMHhCwFlVk5hFSRBYVjjAmCcR+BZl8BiikBAQE X-IronPort-AV: E=Sophos;i="5.17,532,1437408000"; d="scan'208";a="976792492" Subject: Re: [PATCH] fs/binfmt_elf_fdpic.c: fix brk area overlap with stack on NOMMU To: Rich Felker References: <20150820191106.GA9655@brightrain.aerifal.cx> <55DD15AA.5040503@uclinux.org> <55F6B9CF.2020206@westnet.com.au> <20150914151717.GF17773@brightrain.aerifal.cx> Cc: linux-embedded@vger.kernel.org, Paul Gortmaker , Matt Mackall , David Woodhouse , Alexander Viro , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, David Howells From: Greg Ungerer Message-ID: <55F77ECE.3080507@westnet.com.au> Date: Tue, 15 Sep 2015 12:13:34 +1000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150914151717.GF17773@brightrain.aerifal.cx> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rich, On 15/09/15 01:17, Rich Felker wrote: > On Mon, Sep 14, 2015 at 10:13:03PM +1000, Greg Ungerer wrote: >> On 26/08/15 11:26, Greg Ungerer wrote: >>> On 21/08/15 05:11, Rich Felker wrote: >>>> From: Rich Felker >>>> >>>> On NOMMU archs, the FDPIC ELF loader sets up the usable brk range to >>>> overlap with all but the last PAGE_SIZE bytes of the stack. This leads >>>> to catastrophic memory reuse/corruption if brk is used. Fix by setting >>>> the brk area to zero size to disable its use. >>>> >>>> Signed-off-by: Rich Felker >>> >>> It would make sense to run this by David Howells , >>> I think he wrote this code (added to CC list). >>> >>> I have no problem with it, so: >>> >>> Acked-by: Greg Ungerer >> >> Has anybody picked this up to push to Linus? >> If not I can take it via the m68knommu tree. > > As far as I know, no. If you can do it that would be great. Patch applied to m68knommu git tree (for-next branch). (https://git.kernel.org/cgit/linux/kernel/git/gerg/m68knommu.git/) Regards Greg >>>> --- >>>> >>>> There is no reason for the kernel to be providing a brk area at all on >>>> NOMMU; the bFLT loader does not provide one, uClibc never uses brk on >>>> NOMMU targets, and musl libc goes out of its way to avoid using brk >>>> that might run into the stack. >>> >>> I recall a long time back someone was playing with the idea of setting >>> the brk to the unused parts of the last data area page. (Somewhat like >>> this code seems to be trying). That scheme still allocated the full >>> requested stack size (IIRC) though. And that would have been on bFLT >>> executables. Anyway, just some historical reference, not really >>> relevant now. >>> >>> Regards >>> Greg >>> >>> >>> >>>> --- fs/binfmt_elf_fdpic.c.orig 2015-08-20 18:05:19.089888654 +0000 >>>> +++ fs/binfmt_elf_fdpic.c 2015-08-20 18:10:01.519871432 +0000 >>>> @@ -374,10 +388,7 @@ static int load_elf_fdpic_binary(struct >>>> PAGE_ALIGN(current->mm->start_brk); >>>> >>>> #else >>>> - /* create a stack and brk area big enough for everyone >>>> - * - the brk heap starts at the bottom and works up >>>> - * - the stack starts at the top and works down >>>> - */ >>>> + /* create a stack area and zero-size brk area */ >>>> stack_size = (stack_size + PAGE_SIZE - 1) & PAGE_MASK; >>>> if (stack_size < PAGE_SIZE * 2) >>>> stack_size = PAGE_SIZE * 2; >>>> @@ -400,8 +411,6 @@ static int load_elf_fdpic_binary(struct >>>> >>>> current->mm->brk = current->mm->start_brk; >>>> current->mm->context.end_brk = current->mm->start_brk; >>>> - current->mm->context.end_brk += >>>> - (stack_size > PAGE_SIZE) ? (stack_size - PAGE_SIZE) : 0; >>>> current->mm->start_stack = current->mm->start_brk + stack_size; >>>> #endif >>>> >>>> >>> >