linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] MIPS: Always page align TASK_SIZE
@ 2016-02-08 18:05 Harvey Hunt
  2016-02-08 18:11 ` David Daney
  0 siblings, 1 reply; 8+ messages in thread
From: Harvey Hunt @ 2016-02-08 18:05 UTC (permalink / raw)
  To: linux-mips, ralf
  Cc: Harvey Hunt, David Daney, Paul Burton, James Hogan, linux-kernel

STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init() creates an
initial stack for a process, it does so using STACK_TOP_MAX as the end of the
vma. A process's arguments and environment information are placed on the stack
and then the stack is relocated and aligned on a page boundary. When using a 32
bit kernel with 64k pages, the relocated stack has the process's args
erroneously stored in the middle of the stack. This means that processes
receive no arguments or environment variables, preventing them from running
correctly.

Fix this by aligning TASK_SIZE on a page boundary.

Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
Cc: David Daney <david.daney@cavium.com>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: James Hogan <james.hogan@imgtec.com>
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/include/asm/processor.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h
index 3f832c3..b618b40 100644
--- a/arch/mips/include/asm/processor.h
+++ b/arch/mips/include/asm/processor.h
@@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
 #ifdef CONFIG_32BIT
 #ifdef CONFIG_KVM_GUEST
 /* User space process size is limited to 1GB in KVM Guest Mode */
-#define TASK_SIZE	0x3fff8000UL
+#define TASK_SIZE	(0x40000000UL - PAGE_SIZE)
 #else
 /*
  * User space process size: 2GB. This is hardcoded into a few places,
  * so don't change it unless you know what you are doing.
  */
-#define TASK_SIZE	0x7fff8000UL
+#define TASK_SIZE	(0x7fff8000UL & PAGE_SIZE)
 #endif
 
 #define STACK_TOP_MAX	TASK_SIZE
@@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
  * support 16TB; the architectural reserve for future expansion is
  * 8192EB ...
  */
-#define TASK_SIZE32	0x7fff8000UL
+#define TASK_SIZE32	(0x7fff8000UL & PAGE_SIZE)
 #define TASK_SIZE64	0x10000000000UL
 #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 : TASK_SIZE64)
 #define STACK_TOP_MAX	TASK_SIZE64
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] MIPS: Always page align TASK_SIZE
  2016-02-08 18:05 [PATCH] MIPS: Always page align TASK_SIZE Harvey Hunt
@ 2016-02-08 18:11 ` David Daney
  2016-02-08 18:15   ` Harvey Hunt
  0 siblings, 1 reply; 8+ messages in thread
From: David Daney @ 2016-02-08 18:11 UTC (permalink / raw)
  To: Harvey Hunt
  Cc: linux-mips, ralf, David Daney, Paul Burton, James Hogan,
	linux-kernel

On 02/08/2016 10:05 AM, Harvey Hunt wrote:
> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init() creates an
> initial stack for a process, it does so using STACK_TOP_MAX as the end of the
> vma. A process's arguments and environment information are placed on the stack
> and then the stack is relocated and aligned on a page boundary. When using a 32
> bit kernel with 64k pages, the relocated stack has the process's args
> erroneously stored in the middle of the stack. This means that processes
> receive no arguments or environment variables, preventing them from running
> correctly.
>
> Fix this by aligning TASK_SIZE on a page boundary.
>
> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
> Cc: David Daney <david.daney@cavium.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: James Hogan <james.hogan@imgtec.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>   arch/mips/include/asm/processor.h | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h
> index 3f832c3..b618b40 100644
> --- a/arch/mips/include/asm/processor.h
> +++ b/arch/mips/include/asm/processor.h
> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>   #ifdef CONFIG_32BIT
>   #ifdef CONFIG_KVM_GUEST
>   /* User space process size is limited to 1GB in KVM Guest Mode */
> -#define TASK_SIZE	0x3fff8000UL
> +#define TASK_SIZE	(0x40000000UL - PAGE_SIZE)
>   #else
>   /*
>    * User space process size: 2GB. This is hardcoded into a few places,
>    * so don't change it unless you know what you are doing.
>    */
> -#define TASK_SIZE	0x7fff8000UL
> +#define TASK_SIZE	(0x7fff8000UL & PAGE_SIZE)

Can you check your math here.  This doesn't seem correct.

>   #endif
>
>   #define STACK_TOP_MAX	TASK_SIZE
> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>    * support 16TB; the architectural reserve for future expansion is
>    * 8192EB ...
>    */
> -#define TASK_SIZE32	0x7fff8000UL
> +#define TASK_SIZE32	(0x7fff8000UL & PAGE_SIZE)

Same here.

>   #define TASK_SIZE64	0x10000000000UL
>   #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 : TASK_SIZE64)
>   #define STACK_TOP_MAX	TASK_SIZE64
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] MIPS: Always page align TASK_SIZE
  2016-02-08 18:11 ` David Daney
@ 2016-02-08 18:15   ` Harvey Hunt
  2016-02-08 18:24     ` David Daney
  2016-02-08 21:35     ` Joshua Kinard
  0 siblings, 2 replies; 8+ messages in thread
From: Harvey Hunt @ 2016-02-08 18:15 UTC (permalink / raw)
  To: David Daney
  Cc: linux-mips, ralf, David Daney, Paul Burton, James Hogan,
	linux-kernel

Hi David,

On 02/08/2016 10:11 AM, David Daney wrote:
> On 02/08/2016 10:05 AM, Harvey Hunt wrote:
>> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init()
>> creates an
>> initial stack for a process, it does so using STACK_TOP_MAX as the end
>> of the
>> vma. A process's arguments and environment information are placed on
>> the stack
>> and then the stack is relocated and aligned on a page boundary. When
>> using a 32
>> bit kernel with 64k pages, the relocated stack has the process's args
>> erroneously stored in the middle of the stack. This means that processes
>> receive no arguments or environment variables, preventing them from
>> running
>> correctly.
>>
>> Fix this by aligning TASK_SIZE on a page boundary.
>>
>> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
>> Cc: David Daney <david.daney@cavium.com>
>> Cc: Paul Burton <paul.burton@imgtec.com>
>> Cc: James Hogan <james.hogan@imgtec.com>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   arch/mips/include/asm/processor.h | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/processor.h
>> b/arch/mips/include/asm/processor.h
>> index 3f832c3..b618b40 100644
>> --- a/arch/mips/include/asm/processor.h
>> +++ b/arch/mips/include/asm/processor.h
>> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>>   #ifdef CONFIG_32BIT
>>   #ifdef CONFIG_KVM_GUEST
>>   /* User space process size is limited to 1GB in KVM Guest Mode */
>> -#define TASK_SIZE    0x3fff8000UL
>> +#define TASK_SIZE    (0x40000000UL - PAGE_SIZE)
>>   #else
>>   /*
>>    * User space process size: 2GB. This is hardcoded into a few places,
>>    * so don't change it unless you know what you are doing.
>>    */
>> -#define TASK_SIZE    0x7fff8000UL
>> +#define TASK_SIZE    (0x7fff8000UL & PAGE_SIZE)
>
> Can you check your math here.  This doesn't seem correct.

Thanks for spotting that - it should have been:

(0x7fff8000UL & PAGE_MASK)

I'll do a v2 now.

>
>>   #endif
>>
>>   #define STACK_TOP_MAX    TASK_SIZE
>> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>>    * support 16TB; the architectural reserve for future expansion is
>>    * 8192EB ...
>>    */
>> -#define TASK_SIZE32    0x7fff8000UL
>> +#define TASK_SIZE32    (0x7fff8000UL & PAGE_SIZE)
>
> Same here.

As above.

>
>>   #define TASK_SIZE64    0x10000000000UL
>>   #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 :
>> TASK_SIZE64)
>>   #define STACK_TOP_MAX    TASK_SIZE64
>>
>

Thanks,

Harvey

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] MIPS: Always page align TASK_SIZE
  2016-02-08 18:15   ` Harvey Hunt
@ 2016-02-08 18:24     ` David Daney
  2016-02-08 21:35     ` Joshua Kinard
  1 sibling, 0 replies; 8+ messages in thread
From: David Daney @ 2016-02-08 18:24 UTC (permalink / raw)
  To: Harvey Hunt
  Cc: David Daney, linux-mips, ralf, David Daney, Paul Burton,
	James Hogan, linux-kernel

On 02/08/2016 10:15 AM, Harvey Hunt wrote:
> Hi David,
>
> On 02/08/2016 10:11 AM, David Daney wrote:
>> On 02/08/2016 10:05 AM, Harvey Hunt wrote:
>>> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init()
>>> creates an
>>> initial stack for a process, it does so using STACK_TOP_MAX as the end
>>> of the
>>> vma. A process's arguments and environment information are placed on
>>> the stack
>>> and then the stack is relocated and aligned on a page boundary. When
>>> using a 32
>>> bit kernel with 64k pages, the relocated stack has the process's args
>>> erroneously stored in the middle of the stack. This means that processes
>>> receive no arguments or environment variables, preventing them from
>>> running
>>> correctly.
>>>
>>> Fix this by aligning TASK_SIZE on a page boundary.
>>>
>>> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
>>> Cc: David Daney <david.daney@cavium.com>
>>> Cc: Paul Burton <paul.burton@imgtec.com>
>>> Cc: James Hogan <james.hogan@imgtec.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>   arch/mips/include/asm/processor.h | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/mips/include/asm/processor.h
>>> b/arch/mips/include/asm/processor.h
>>> index 3f832c3..b618b40 100644
>>> --- a/arch/mips/include/asm/processor.h
>>> +++ b/arch/mips/include/asm/processor.h
>>> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>>>   #ifdef CONFIG_32BIT
>>>   #ifdef CONFIG_KVM_GUEST
>>>   /* User space process size is limited to 1GB in KVM Guest Mode */
>>> -#define TASK_SIZE    0x3fff8000UL
>>> +#define TASK_SIZE    (0x40000000UL - PAGE_SIZE)
>>>   #else
>>>   /*
>>>    * User space process size: 2GB. This is hardcoded into a few places,
>>>    * so don't change it unless you know what you are doing.
>>>    */
>>> -#define TASK_SIZE    0x7fff8000UL
>>> +#define TASK_SIZE    (0x7fff8000UL & PAGE_SIZE)
>>
>> Can you check your math here.  This doesn't seem correct.
>
> Thanks for spotting that - it should have been:
>
> (0x7fff8000UL & PAGE_MASK)

This brings up an interesting point.  How was this tested?  Please note 
that in the change log.

Also look at the definition of PAGE_MASK in page.h

Is that correct?  Most of the other related symbols have an "_AC(1,UL)" 
in them.  Why is this not also appropriate for PAGE_MASK?

It may also be a good idea to prepare and test a patch that defines 
PAGE_MASK much in the same way HPAGE_MASK is defined.

David Daney


>
> I'll do a v2 now.
>
>>
>>>   #endif
>>>
>>>   #define STACK_TOP_MAX    TASK_SIZE
>>> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>>>    * support 16TB; the architectural reserve for future expansion is
>>>    * 8192EB ...
>>>    */
>>> -#define TASK_SIZE32    0x7fff8000UL
>>> +#define TASK_SIZE32    (0x7fff8000UL & PAGE_SIZE)
>>
>> Same here.
>
> As above.
>
>>
>>>   #define TASK_SIZE64    0x10000000000UL
>>>   #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 :
>>> TASK_SIZE64)
>>>   #define STACK_TOP_MAX    TASK_SIZE64
>>>
>>
>
> Thanks,
>
> Harvey

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] MIPS: Always page align TASK_SIZE
  2016-02-08 18:15   ` Harvey Hunt
  2016-02-08 18:24     ` David Daney
@ 2016-02-08 21:35     ` Joshua Kinard
  2016-02-08 21:48       ` Harvey Hunt
  1 sibling, 1 reply; 8+ messages in thread
From: Joshua Kinard @ 2016-02-08 21:35 UTC (permalink / raw)
  To: Harvey Hunt, David Daney
  Cc: linux-mips, ralf, David Daney, Paul Burton, James Hogan,
	linux-kernel

On 02/08/2016 13:15, Harvey Hunt wrote:
> Hi David,
> 
> On 02/08/2016 10:11 AM, David Daney wrote:
>> On 02/08/2016 10:05 AM, Harvey Hunt wrote:
>>> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init()
>>> creates an
>>> initial stack for a process, it does so using STACK_TOP_MAX as the end
>>> of the
>>> vma. A process's arguments and environment information are placed on
>>> the stack
>>> and then the stack is relocated and aligned on a page boundary. When
>>> using a 32
>>> bit kernel with 64k pages, the relocated stack has the process's args
>>> erroneously stored in the middle of the stack. This means that processes
>>> receive no arguments or environment variables, preventing them from
>>> running
>>> correctly.
>>>
>>> Fix this by aligning TASK_SIZE on a page boundary.
>>>
>>> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
>>> Cc: David Daney <david.daney@cavium.com>
>>> Cc: Paul Burton <paul.burton@imgtec.com>
>>> Cc: James Hogan <james.hogan@imgtec.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>   arch/mips/include/asm/processor.h | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/mips/include/asm/processor.h
>>> b/arch/mips/include/asm/processor.h
>>> index 3f832c3..b618b40 100644
>>> --- a/arch/mips/include/asm/processor.h
>>> +++ b/arch/mips/include/asm/processor.h
>>> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>>>   #ifdef CONFIG_32BIT
>>>   #ifdef CONFIG_KVM_GUEST
>>>   /* User space process size is limited to 1GB in KVM Guest Mode */
>>> -#define TASK_SIZE    0x3fff8000UL
>>> +#define TASK_SIZE    (0x40000000UL - PAGE_SIZE)
>>>   #else
>>>   /*
>>>    * User space process size: 2GB. This is hardcoded into a few places,
>>>    * so don't change it unless you know what you are doing.
>>>    */
>>> -#define TASK_SIZE    0x7fff8000UL
>>> +#define TASK_SIZE    (0x7fff8000UL & PAGE_SIZE)
>>
>> Can you check your math here.  This doesn't seem correct.
> 
> Thanks for spotting that - it should have been:
> 
> (0x7fff8000UL & PAGE_MASK)
> 
> I'll do a v2 now.
> 

FYI, TASK_SIZE was recently changed to 0x80000000UL in commit 7f8ca9cb1ed3 on
the linux-mips.org tree.


>>
>>>   #endif
>>>
>>>   #define STACK_TOP_MAX    TASK_SIZE
>>> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>>>    * support 16TB; the architectural reserve for future expansion is
>>>    * 8192EB ...
>>>    */
>>> -#define TASK_SIZE32    0x7fff8000UL
>>> +#define TASK_SIZE32    (0x7fff8000UL & PAGE_SIZE)
>>
>> Same here.
> 
> As above.
> 
>>
>>>   #define TASK_SIZE64    0x10000000000UL
>>>   #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 :
>>> TASK_SIZE64)
>>>   #define STACK_TOP_MAX    TASK_SIZE64
>>>
>>
> 
> Thanks,
> 
> Harvey
> 
> 


-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org
6144R/F5C6C943 2015-04-27
177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943

"The past tempts us, the present confuses us, the future frightens us.  And our
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] MIPS: Always page align TASK_SIZE
  2016-02-08 21:35     ` Joshua Kinard
@ 2016-02-08 21:48       ` Harvey Hunt
  2016-02-08 22:08         ` David Daney
  0 siblings, 1 reply; 8+ messages in thread
From: Harvey Hunt @ 2016-02-08 21:48 UTC (permalink / raw)
  To: Joshua Kinard, David Daney
  Cc: linux-mips, ralf, David Daney, Paul Burton, James Hogan,
	linux-kernel

Hi Joshua,

On 02/08/2016 01:35 PM, Joshua Kinard wrote:
> On 02/08/2016 13:15, Harvey Hunt wrote:
>> Hi David,
>>
>> On 02/08/2016 10:11 AM, David Daney wrote:
>>> On 02/08/2016 10:05 AM, Harvey Hunt wrote:
>>>> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init()
>>>> creates an
>>>> initial stack for a process, it does so using STACK_TOP_MAX as the end
>>>> of the
>>>> vma. A process's arguments and environment information are placed on
>>>> the stack
>>>> and then the stack is relocated and aligned on a page boundary. When
>>>> using a 32
>>>> bit kernel with 64k pages, the relocated stack has the process's args
>>>> erroneously stored in the middle of the stack. This means that processes
>>>> receive no arguments or environment variables, preventing them from
>>>> running
>>>> correctly.
>>>>
>>>> Fix this by aligning TASK_SIZE on a page boundary.
>>>>
>>>> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
>>>> Cc: David Daney <david.daney@cavium.com>
>>>> Cc: Paul Burton <paul.burton@imgtec.com>
>>>> Cc: James Hogan <james.hogan@imgtec.com>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> ---
>>>>    arch/mips/include/asm/processor.h | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/mips/include/asm/processor.h
>>>> b/arch/mips/include/asm/processor.h
>>>> index 3f832c3..b618b40 100644
>>>> --- a/arch/mips/include/asm/processor.h
>>>> +++ b/arch/mips/include/asm/processor.h
>>>> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>>>>    #ifdef CONFIG_32BIT
>>>>    #ifdef CONFIG_KVM_GUEST
>>>>    /* User space process size is limited to 1GB in KVM Guest Mode */
>>>> -#define TASK_SIZE    0x3fff8000UL
>>>> +#define TASK_SIZE    (0x40000000UL - PAGE_SIZE)
>>>>    #else
>>>>    /*
>>>>     * User space process size: 2GB. This is hardcoded into a few places,
>>>>     * so don't change it unless you know what you are doing.
>>>>     */
>>>> -#define TASK_SIZE    0x7fff8000UL
>>>> +#define TASK_SIZE    (0x7fff8000UL & PAGE_SIZE)
>>>
>>> Can you check your math here.  This doesn't seem correct.
>>
>> Thanks for spotting that - it should have been:
>>
>> (0x7fff8000UL & PAGE_MASK)
>>
>> I'll do a v2 now.
>>
>
> FYI, TASK_SIZE was recently changed to 0x80000000UL in commit 7f8ca9cb1ed3 on
> the linux-mips.org tree.

Thanks, I'll rebase.

>
>
>>>
>>>>    #endif
>>>>
>>>>    #define STACK_TOP_MAX    TASK_SIZE
>>>> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>>>>     * support 16TB; the architectural reserve for future expansion is
>>>>     * 8192EB ...
>>>>     */
>>>> -#define TASK_SIZE32    0x7fff8000UL
>>>> +#define TASK_SIZE32    (0x7fff8000UL & PAGE_SIZE)
>>>
>>> Same here.
>>
>> As above.
>>
>>>
>>>>    #define TASK_SIZE64    0x10000000000UL
>>>>    #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 :
>>>> TASK_SIZE64)
>>>>    #define STACK_TOP_MAX    TASK_SIZE64
>>>>
>>>
>>
>> Thanks,
>>
>> Harvey
>>
>>
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] MIPS: Always page align TASK_SIZE
  2016-02-08 21:48       ` Harvey Hunt
@ 2016-02-08 22:08         ` David Daney
  2016-02-08 22:26           ` Harvey Hunt
  0 siblings, 1 reply; 8+ messages in thread
From: David Daney @ 2016-02-08 22:08 UTC (permalink / raw)
  To: Harvey Hunt
  Cc: Joshua Kinard, linux-mips, ralf, David Daney, Paul Burton,
	James Hogan, linux-kernel

On 02/08/2016 01:48 PM, Harvey Hunt wrote:
> Hi Joshua,
>
> On 02/08/2016 01:35 PM, Joshua Kinard wrote:
>> On 02/08/2016 13:15, Harvey Hunt wrote:
>>> Hi David,
>>>
>>> On 02/08/2016 10:11 AM, David Daney wrote:
>>>> On 02/08/2016 10:05 AM, Harvey Hunt wrote:
>>>>> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init()
>>>>> creates an
>>>>> initial stack for a process, it does so using STACK_TOP_MAX as the end
>>>>> of the
>>>>> vma. A process's arguments and environment information are placed on
>>>>> the stack
>>>>> and then the stack is relocated and aligned on a page boundary. When
>>>>> using a 32
>>>>> bit kernel with 64k pages, the relocated stack has the process's args
>>>>> erroneously stored in the middle of the stack. This means that
>>>>> processes
>>>>> receive no arguments or environment variables, preventing them from
>>>>> running
>>>>> correctly.
>>>>>
>>>>> Fix this by aligning TASK_SIZE on a page boundary.
>>>>>
>>>>> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
>>>>> Cc: David Daney <david.daney@cavium.com>
>>>>> Cc: Paul Burton <paul.burton@imgtec.com>
>>>>> Cc: James Hogan <james.hogan@imgtec.com>
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> ---
>>>>>    arch/mips/include/asm/processor.h | 6 +++---
>>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/mips/include/asm/processor.h
>>>>> b/arch/mips/include/asm/processor.h
>>>>> index 3f832c3..b618b40 100644
>>>>> --- a/arch/mips/include/asm/processor.h
>>>>> +++ b/arch/mips/include/asm/processor.h
>>>>> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>>>>>    #ifdef CONFIG_32BIT
>>>>>    #ifdef CONFIG_KVM_GUEST
>>>>>    /* User space process size is limited to 1GB in KVM Guest Mode */
>>>>> -#define TASK_SIZE    0x3fff8000UL
>>>>> +#define TASK_SIZE    (0x40000000UL - PAGE_SIZE)
>>>>>    #else
>>>>>    /*
>>>>>     * User space process size: 2GB. This is hardcoded into a few
>>>>> places,
>>>>>     * so don't change it unless you know what you are doing.
>>>>>     */
>>>>> -#define TASK_SIZE    0x7fff8000UL
>>>>> +#define TASK_SIZE    (0x7fff8000UL & PAGE_SIZE)
>>>>
>>>> Can you check your math here.  This doesn't seem correct.
>>>
>>> Thanks for spotting that - it should have been:
>>>
>>> (0x7fff8000UL & PAGE_MASK)
>>>
>>> I'll do a v2 now.
>>>
>>
>> FYI, TASK_SIZE was recently changed to 0x80000000UL in commit
>> 7f8ca9cb1ed3 on
>> the linux-mips.org tree.
>
> Thanks, I'll rebase.

You may find that in rebasing, suddenly you have a completely empty patch!


>
>>
>>
>>>>
>>>>>    #endif
>>>>>
>>>>>    #define STACK_TOP_MAX    TASK_SIZE
>>>>> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>>>>>     * support 16TB; the architectural reserve for future expansion is
>>>>>     * 8192EB ...
>>>>>     */
>>>>> -#define TASK_SIZE32    0x7fff8000UL
>>>>> +#define TASK_SIZE32    (0x7fff8000UL & PAGE_SIZE)
>>>>
>>>> Same here.
>>>
>>> As above.
>>>
>>>>
>>>>>    #define TASK_SIZE64    0x10000000000UL
>>>>>    #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 :
>>>>> TASK_SIZE64)
>>>>>    #define STACK_TOP_MAX    TASK_SIZE64
>>>>>
>>>>
>>>
>>> Thanks,
>>>
>>> Harvey
>>>
>>>
>>
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] MIPS: Always page align TASK_SIZE
  2016-02-08 22:08         ` David Daney
@ 2016-02-08 22:26           ` Harvey Hunt
  0 siblings, 0 replies; 8+ messages in thread
From: Harvey Hunt @ 2016-02-08 22:26 UTC (permalink / raw)
  To: David Daney
  Cc: Joshua Kinard, linux-mips, ralf, David Daney, Paul Burton,
	James Hogan, linux-kernel



On 02/08/2016 02:08 PM, David Daney wrote:
> On 02/08/2016 01:48 PM, Harvey Hunt wrote:
>> Hi Joshua,
>>
>> On 02/08/2016 01:35 PM, Joshua Kinard wrote:
>>> On 02/08/2016 13:15, Harvey Hunt wrote:
>>>> Hi David,
>>>>
>>>> On 02/08/2016 10:11 AM, David Daney wrote:
>>>>> On 02/08/2016 10:05 AM, Harvey Hunt wrote:
>>>>>> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init()
>>>>>> creates an
>>>>>> initial stack for a process, it does so using STACK_TOP_MAX as the
>>>>>> end
>>>>>> of the
>>>>>> vma. A process's arguments and environment information are placed on
>>>>>> the stack
>>>>>> and then the stack is relocated and aligned on a page boundary. When
>>>>>> using a 32
>>>>>> bit kernel with 64k pages, the relocated stack has the process's args
>>>>>> erroneously stored in the middle of the stack. This means that
>>>>>> processes
>>>>>> receive no arguments or environment variables, preventing them from
>>>>>> running
>>>>>> correctly.
>>>>>>
>>>>>> Fix this by aligning TASK_SIZE on a page boundary.
>>>>>>
>>>>>> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
>>>>>> Cc: David Daney <david.daney@cavium.com>
>>>>>> Cc: Paul Burton <paul.burton@imgtec.com>
>>>>>> Cc: James Hogan <james.hogan@imgtec.com>
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> ---
>>>>>>    arch/mips/include/asm/processor.h | 6 +++---
>>>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/mips/include/asm/processor.h
>>>>>> b/arch/mips/include/asm/processor.h
>>>>>> index 3f832c3..b618b40 100644
>>>>>> --- a/arch/mips/include/asm/processor.h
>>>>>> +++ b/arch/mips/include/asm/processor.h
>>>>>> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>>>>>>    #ifdef CONFIG_32BIT
>>>>>>    #ifdef CONFIG_KVM_GUEST
>>>>>>    /* User space process size is limited to 1GB in KVM Guest Mode */
>>>>>> -#define TASK_SIZE    0x3fff8000UL
>>>>>> +#define TASK_SIZE    (0x40000000UL - PAGE_SIZE)
>>>>>>    #else
>>>>>>    /*
>>>>>>     * User space process size: 2GB. This is hardcoded into a few
>>>>>> places,
>>>>>>     * so don't change it unless you know what you are doing.
>>>>>>     */
>>>>>> -#define TASK_SIZE    0x7fff8000UL
>>>>>> +#define TASK_SIZE    (0x7fff8000UL & PAGE_SIZE)
>>>>>
>>>>> Can you check your math here.  This doesn't seem correct.
>>>>
>>>> Thanks for spotting that - it should have been:
>>>>
>>>> (0x7fff8000UL & PAGE_MASK)
>>>>
>>>> I'll do a v2 now.
>>>>
>>>
>>> FYI, TASK_SIZE was recently changed to 0x80000000UL in commit
>>> 7f8ca9cb1ed3 on
>>> the linux-mips.org tree.
>>
>> Thanks, I'll rebase.
>
> You may find that in rebasing, suddenly you have a completely empty patch!
>

Yeah, I noticed that. I'll just drop this patch...

Thanks,

Harvey

>
>>
>>>
>>>
>>>>>
>>>>>>    #endif
>>>>>>
>>>>>>    #define STACK_TOP_MAX    TASK_SIZE
>>>>>> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>>>>>>     * support 16TB; the architectural reserve for future expansion is
>>>>>>     * 8192EB ...
>>>>>>     */
>>>>>> -#define TASK_SIZE32    0x7fff8000UL
>>>>>> +#define TASK_SIZE32    (0x7fff8000UL & PAGE_SIZE)
>>>>>
>>>>> Same here.
>>>>
>>>> As above.
>>>>
>>>>>
>>>>>>    #define TASK_SIZE64    0x10000000000UL
>>>>>>    #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ?
>>>>>> TASK_SIZE32 :
>>>>>> TASK_SIZE64)
>>>>>>    #define STACK_TOP_MAX    TASK_SIZE64
>>>>>>
>>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Harvey
>>>>
>>>>
>>>
>>>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-02-08 22:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-08 18:05 [PATCH] MIPS: Always page align TASK_SIZE Harvey Hunt
2016-02-08 18:11 ` David Daney
2016-02-08 18:15   ` Harvey Hunt
2016-02-08 18:24     ` David Daney
2016-02-08 21:35     ` Joshua Kinard
2016-02-08 21:48       ` Harvey Hunt
2016-02-08 22:08         ` David Daney
2016-02-08 22:26           ` Harvey Hunt

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