Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1224069 > unrolled thread

Re: [PATCH] fs/binfmt_elf_fdpic.c: fix brk area overlap with stack on NOMMU

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] fs/binfmt_elf_fdpic.c: fix brk area overlap with stack on  NOMMU Greg Ungerer <gregungerer@westnet.com.au> - 2015-09-14 14:30 +0200
    Re: [PATCH] fs/binfmt_elf_fdpic.c: fix brk area overlap with stack  on NOMMU Rich Felker <dalias@libc.org> - 2015-09-14 17:20 +0200
      Re: [PATCH] fs/binfmt_elf_fdpic.c: fix brk area overlap with stack on  NOMMU Greg Ungerer <gregungerer@westnet.com.au> - 2015-09-15 04:20 +0200

#1224069 — Re: [PATCH] fs/binfmt_elf_fdpic.c: fix brk area overlap with stack on NOMMU

Hi Rich,


On 26/08/15 11:26, Greg Ungerer wrote:
> On 21/08/15 05:11, Rich Felker wrote:
>> From: Rich Felker <dalias@libc.org>
>>
>> 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 <dalias@libc.org>
>
> It would make sense to run this by David Howells <dhowells@redhat.com>,
> I think he wrote this code (added to CC list).
>
> I have no problem with it, so:
>
> Acked-by: Greg Ungerer <gerg@uclinux.org>

Has anybody picked this up to push to Linus?
If not I can take it via the m68knommu tree.

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
>>
>>
>

--
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/

[toc] | [next] | [standalone]

#1224233 — Re: [PATCH] fs/binfmt_elf_fdpic.c: fix brk area overlap with stack on NOMMU

On Mon, Sep 14, 2015 at 10:13:03PM +1000, Greg Ungerer wrote:
> Hi Rich,
> 
> 
> On 26/08/15 11:26, Greg Ungerer wrote:
> >On 21/08/15 05:11, Rich Felker wrote:
> >>From: Rich Felker <dalias@libc.org>
> >>
> >>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 <dalias@libc.org>
> >
> >It would make sense to run this by David Howells <dhowells@redhat.com>,
> >I think he wrote this code (added to CC list).
> >
> >I have no problem with it, so:
> >
> >Acked-by: Greg Ungerer <gerg@uclinux.org>
> 
> 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.

Rich


> >>---
> >>
> >>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
> >>
> >>
> >
--
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/

[toc] | [prev] | [next] | [standalone]

#1224596

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 <dalias@libc.org>
>>>>
>>>> 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 <dalias@libc.org>
>>>
>>> It would make sense to run this by David Howells <dhowells@redhat.com>,
>>> I think he wrote this code (added to CC list).
>>>
>>> I have no problem with it, so:
>>>
>>> Acked-by: Greg Ungerer <gerg@uclinux.org>
>>
>> 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
>>>>
>>>>
>>>
> 

--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web