linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Possible regression from "fs/exec.c: call arch_pick_mmap_layout() only once"
@ 2014-01-02 21:41 Pat Erley
  2014-01-02 22:24 ` Richard Weinberger
  0 siblings, 1 reply; 4+ messages in thread
From: Pat Erley @ 2014-01-02 21:41 UTC (permalink / raw)
  To: linux-next; +Cc: richard

On my 64bit kernel, commit 283fe963095b38a6ab75dda1436ee66b9e45c7c2 
seems to have broken 32bit compatibility.  I've run the bisection twice, 
and verified that reverting this on HEAD fixes the problem.  I've 
uploaded my .config to pastebin at http://pastebin.com/kVcr9H65

Even this simple program:

main(){puts("HELLO");}

compiled with:

gcc -m32 test.c

Will crash with a segfault.  Stracing shows that it's failing to 
allocate memory.

Pat Erley

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: Possible regression from "fs/exec.c: call arch_pick_mmap_layout() only once"
  2014-01-02 21:41 Possible regression from "fs/exec.c: call arch_pick_mmap_layout() only once" Pat Erley
@ 2014-01-02 22:24 ` Richard Weinberger
  2014-01-02 22:39   ` Pat Erley
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Weinberger @ 2014-01-02 22:24 UTC (permalink / raw)
  To: Pat Erley; +Cc: linux-next, akpm, linux-kernel

Am Donnerstag, 2. Januar 2014, 15:41:27 schrieb Pat Erley:
> On my 64bit kernel, commit 283fe963095b38a6ab75dda1436ee66b9e45c7c2
> seems to have broken 32bit compatibility.  I've run the bisection twice,
> and verified that reverting this on HEAD fixes the problem.  I've
> uploaded my .config to pastebin at http://pastebin.com/kVcr9H65
> 
> Even this simple program:
> 
> main(){puts("HELLO");}
> 
> compiled with:
> 
> gcc -m32 test.c
> 
> Will crash with a segfault.  Stracing shows that it's failing to
> allocate memory.

Good catch!

flush_old_exec() is called before setup_new_exec() and I've removed
arch_pick_mmap_layout() from the second call site.
Which turned out to be wrong.

It is wrong because between both callers current->personality is changed.
So, we have to remove the first call to arch_pick_mmap_layout() and keep the 
latter because only then the correct personality is set up.

Can you please test your config with the following patch applied and having
283fe96 reverted?

If it works out for you I'd send an updated patch to Andrew.
In the meanwhile I'll double check all call sites...

Thanks,
//richard

---
diff --git a/fs/exec.c b/fs/exec.c
index 7ea097f..a733599 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -843,7 +843,6 @@ static int exec_mmap(struct mm_struct *mm)
 	tsk->active_mm = mm;
 	activate_mm(active_mm, mm);
 	task_unlock(tsk);
-	arch_pick_mmap_layout(mm);
 	if (old_mm) {
 		up_read(&old_mm->mmap_sem);
 		BUG_ON(active_mm != old_mm);

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: Possible regression from "fs/exec.c: call arch_pick_mmap_layout() only once"
  2014-01-02 22:24 ` Richard Weinberger
@ 2014-01-02 22:39   ` Pat Erley
  2014-01-03  4:35     ` Pat Erley
  0 siblings, 1 reply; 4+ messages in thread
From: Pat Erley @ 2014-01-02 22:39 UTC (permalink / raw)
  To: Richard Weinberger; +Cc: linux-next, akpm, linux-kernel

On 01/02/2014 04:24 PM, Richard Weinberger wrote:
> Am Donnerstag, 2. Januar 2014, 15:41:27 schrieb Pat Erley:
>> On my 64bit kernel, commit 283fe963095b38a6ab75dda1436ee66b9e45c7c2
>> seems to have broken 32bit compatibility.  I've run the bisection twice,
>> and verified that reverting this on HEAD fixes the problem.  I've
>> uploaded my .config to pastebin at http://pastebin.com/kVcr9H65
>>
>> Even this simple program:
>>
>> main(){puts("HELLO");}
>>
>> compiled with:
>>
>> gcc -m32 test.c
>>
>> Will crash with a segfault.  Stracing shows that it's failing to
>> allocate memory.
>
> Good catch!
>
> flush_old_exec() is called before setup_new_exec() and I've removed
> arch_pick_mmap_layout() from the second call site.
> Which turned out to be wrong.
>
> It is wrong because between both callers current->personality is changed.
> So, we have to remove the first call to arch_pick_mmap_layout() and keep the
> latter because only then the correct personality is set up.
>
> Can you please test your config with the following patch applied and having
> 283fe96 reverted?
>
> If it works out for you I'd send an updated patch to Andrew.
> In the meanwhile I'll double check all call sites...
>
> Thanks,
> //richard
>
> ---
> diff --git a/fs/exec.c b/fs/exec.c
> index 7ea097f..a733599 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -843,7 +843,6 @@ static int exec_mmap(struct mm_struct *mm)
>   	tsk->active_mm = mm;
>   	activate_mm(active_mm, mm);
>   	task_unlock(tsk);
> -	arch_pick_mmap_layout(mm);
>   	if (old_mm) {
>   		up_read(&old_mm->mmap_sem);
>   		BUG_ON(active_mm != old_mm);
>

Compiling right now.  Will test later tonight and let you know.

Pat

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: Possible regression from "fs/exec.c: call arch_pick_mmap_layout() only once"
  2014-01-02 22:39   ` Pat Erley
@ 2014-01-03  4:35     ` Pat Erley
  0 siblings, 0 replies; 4+ messages in thread
From: Pat Erley @ 2014-01-03  4:35 UTC (permalink / raw)
  To: Richard Weinberger; +Cc: linux-next, akpm, linux-kernel

On 01/02/2014 04:39 PM, Pat Erley wrote:
> On 01/02/2014 04:24 PM, Richard Weinberger wrote:
>> Am Donnerstag, 2. Januar 2014, 15:41:27 schrieb Pat Erley:
>>> On my 64bit kernel, commit 283fe963095b38a6ab75dda1436ee66b9e45c7c2
>>> seems to have broken 32bit compatibility.  I've run the bisection twice,
>>> and verified that reverting this on HEAD fixes the problem.  I've
>>> uploaded my .config to pastebin at http://pastebin.com/kVcr9H65
>>>
>>> Even this simple program:
>>>
>>> main(){puts("HELLO");}
>>>
>>> compiled with:
>>>
>>> gcc -m32 test.c
>>>
>>> Will crash with a segfault.  Stracing shows that it's failing to
>>> allocate memory.
>>
>> Good catch!
>>
>> flush_old_exec() is called before setup_new_exec() and I've removed
>> arch_pick_mmap_layout() from the second call site.
>> Which turned out to be wrong.
>>
>> It is wrong because between both callers current->personality is changed.
>> So, we have to remove the first call to arch_pick_mmap_layout() and
>> keep the
>> latter because only then the correct personality is set up.
>>
>> Can you please test your config with the following patch applied and
>> having
>> 283fe96 reverted?
>>
>> If it works out for you I'd send an updated patch to Andrew.
>> In the meanwhile I'll double check all call sites...
>>
>> Thanks,
>> //richard
>>
>> ---
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 7ea097f..a733599 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -843,7 +843,6 @@ static int exec_mmap(struct mm_struct *mm)
>>       tsk->active_mm = mm;
>>       activate_mm(active_mm, mm);
>>       task_unlock(tsk);
>> -    arch_pick_mmap_layout(mm);
>>       if (old_mm) {
>>           up_read(&old_mm->mmap_sem);
>>           BUG_ON(active_mm != old_mm);
>>
>
> Compiling right now.  Will test later tonight and let you know.
>
> Pat

I can confirm that this works with 283fe96 reverted.

Pat

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-03  4:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-02 21:41 Possible regression from "fs/exec.c: call arch_pick_mmap_layout() only once" Pat Erley
2014-01-02 22:24 ` Richard Weinberger
2014-01-02 22:39   ` Pat Erley
2014-01-03  4:35     ` Pat Erley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).