Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
Hello all,
Any comments about this patch?
Cheers,
Dongsheng
On 2018/11/30 10:19, Wang, Dongsheng wrote:
> On 2018/11/30 10:04, Wang, Dongsheng wrote:
>> On 2018/11/30 5:22, Kees Cook wrote:
>>> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>>> wrote:
Hello Kees,
On 2018/11/28 6:38, Kees Cook wrote:
> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
> wrote:
>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>> is not a real task on stack, it's only init_task on init_stack.
>>
>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>> rather than by union") added this macro and put task_strcut into
>> thread_union. This brings us the following possibilities:
>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>> - <-- thread_info & stack
>> NN | | --- <-- task
>>| || |
>> - ---
>>
>> - <-- stack
>> NY | | --- <--
>> task(Including thread_info)
>>| || |
>> - ---
>>
>> - <-- stack & task &
>> thread_info
>> YN | |
>>| |
>> -
>>
>> - <-- stack &
>> task(Including thread_info)
>> YY | |
>>| |
>> -
>> The kernel has handled the first two cases correctly.
>>
>> For the third case:
>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>> should never happen, because the task and thread_info will overlap. So
>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>
>> For the fourth case:
>> When task on stack, the end of stack should add a sizeof(task_struct)
>> offset.
>>
>> This patch handled with the third and fourth case.
>>
>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>
>> Signed-off-by: Wang Dongsheng
>> Signed-off-by: Shunyong Yang
>> ---
>> arch/Kconfig | 1 +
>> include/linux/sched/task_stack.h | 5 -
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index e1e540ffa979..0a2c73e73195 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>> # Select if arch init_task must go in the __init_task_data section
>> config ARCH_TASK_STRUCT_ON_STACK
>> bool
>> + depends on THREAD_INFO_IN_TASK || IA64
> The "IA64" part shouldn't be needed since IA64 already selects it.
>
> Since it's selected, it also can't have a depends, IIUC.
Since the IA64 thread_info including task_struct, it doesn't need to
select THREAD_INFO_IN_TASK.
So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
THREAD_INFO.
>>> Okay.
>>>
>> # Select if arch has its private alloc_task_struct() function
>> config ARCH_TASK_STRUCT_ALLOCATOR
>> diff --git a/include/linux/sched/task_stack.h
>> b/include/linux/sched/task_stack.h
>> index 6a841929073f..624c48defb9e 100644
>> --- a/include/linux/sched/task_stack.h
>> +++ b/include/linux/sched/task_stack.h
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include
>> +#include
>> #include
>>
>> #ifdef CONFIG_THREAD_INFO_IN_TASK
>> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
>> task_struct *task)
>>
>> static inline unsigned long *end_of_stack(const struct task_struct
>> *task)
>> {
>> - return task->stack;
>> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
>> _task)
>> + return task->stack;
>> + return (unsigned long *)(task + 1);
>> }
> This seems like a strange place for the change. It feels more like
> init_task has been defined incorrectly.
The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
selected.
include/asm-generic/vmlinux.lds.h:
#define INIT_TASK_DATA(align)\
. = ALIGN(align);\
__start_init_task = .;
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On 2018/11/30 10:04, Wang, Dongsheng wrote:
> On 2018/11/30 5:22, Kees Cook wrote:
>> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>> wrote:
>>> Hello Kees,
>>>
>>> On 2018/11/28 6:38, Kees Cook wrote:
On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
wrote:
> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
> is not a real task on stack, it's only init_task on init_stack.
>
> Commit 0500871f21b2 ("Construct init thread stack in the linker script
> rather than by union") added this macro and put task_strcut into
> thread_union. This brings us the following possibilities:
> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
> - <-- thread_info & stack
> NN | | --- <-- task
>| || |
> - ---
>
> - <-- stack
> NY | | --- <--
> task(Including thread_info)
>| || |
> - ---
>
> - <-- stack & task &
> thread_info
> YN | |
>| |
> -
>
> - <-- stack &
> task(Including thread_info)
> YY | |
>| |
> -
> The kernel has handled the first two cases correctly.
>
> For the third case:
> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
> should never happen, because the task and thread_info will overlap. So
> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>
> For the fourth case:
> When task on stack, the end of stack should add a sizeof(task_struct)
> offset.
>
> This patch handled with the third and fourth case.
>
> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>
> Signed-off-by: Wang Dongsheng
> Signed-off-by: Shunyong Yang
> ---
> arch/Kconfig | 1 +
> include/linux/sched/task_stack.h | 5 -
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e1e540ffa979..0a2c73e73195 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
> # Select if arch init_task must go in the __init_task_data section
> config ARCH_TASK_STRUCT_ON_STACK
> bool
> + depends on THREAD_INFO_IN_TASK || IA64
The "IA64" part shouldn't be needed since IA64 already selects it.
Since it's selected, it also can't have a depends, IIUC.
>>> Since the IA64 thread_info including task_struct, it doesn't need to
>>> select THREAD_INFO_IN_TASK.
>>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>>> THREAD_INFO.
>> Okay.
>>
> # Select if arch has its private alloc_task_struct() function
> config ARCH_TASK_STRUCT_ALLOCATOR
> diff --git a/include/linux/sched/task_stack.h
> b/include/linux/sched/task_stack.h
> index 6a841929073f..624c48defb9e 100644
> --- a/include/linux/sched/task_stack.h
> +++ b/include/linux/sched/task_stack.h
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
>
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
> task_struct *task)
>
> static inline unsigned long *end_of_stack(const struct task_struct *task)
> {
> - return task->stack;
> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
> _task)
> + return task->stack;
> + return (unsigned long *)(task + 1);
> }
This seems like a strange place for the change. It feels more like
init_task has been defined incorrectly.
>>> The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
>>> selected.
>>> include/asm-generic/vmlinux.lds.h:
>>> #define INIT_TASK_DATA(align)\
>>> . = ALIGN(align);\
>>> __start_init_task = .;\
>>> init_thread_union = .;\
>>> init_stack = .;\
>>> KEEP(*(.data..init_task))\
>>> KEEP(*(.data..init_thread_info))\
>>>
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On 2018/11/30 10:04, Wang, Dongsheng wrote:
> On 2018/11/30 5:22, Kees Cook wrote:
>> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>> wrote:
>>> Hello Kees,
>>>
>>> On 2018/11/28 6:38, Kees Cook wrote:
On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
wrote:
> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
> is not a real task on stack, it's only init_task on init_stack.
>
> Commit 0500871f21b2 ("Construct init thread stack in the linker script
> rather than by union") added this macro and put task_strcut into
> thread_union. This brings us the following possibilities:
> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
> - <-- thread_info & stack
> NN | | --- <-- task
>| || |
> - ---
>
> - <-- stack
> NY | | --- <--
> task(Including thread_info)
>| || |
> - ---
>
> - <-- stack & task &
> thread_info
> YN | |
>| |
> -
>
> - <-- stack &
> task(Including thread_info)
> YY | |
>| |
> -
> The kernel has handled the first two cases correctly.
>
> For the third case:
> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
> should never happen, because the task and thread_info will overlap. So
> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>
> For the fourth case:
> When task on stack, the end of stack should add a sizeof(task_struct)
> offset.
>
> This patch handled with the third and fourth case.
>
> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>
> Signed-off-by: Wang Dongsheng
> Signed-off-by: Shunyong Yang
> ---
> arch/Kconfig | 1 +
> include/linux/sched/task_stack.h | 5 -
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e1e540ffa979..0a2c73e73195 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
> # Select if arch init_task must go in the __init_task_data section
> config ARCH_TASK_STRUCT_ON_STACK
> bool
> + depends on THREAD_INFO_IN_TASK || IA64
The "IA64" part shouldn't be needed since IA64 already selects it.
Since it's selected, it also can't have a depends, IIUC.
>>> Since the IA64 thread_info including task_struct, it doesn't need to
>>> select THREAD_INFO_IN_TASK.
>>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>>> THREAD_INFO.
>> Okay.
>>
> # Select if arch has its private alloc_task_struct() function
> config ARCH_TASK_STRUCT_ALLOCATOR
> diff --git a/include/linux/sched/task_stack.h
> b/include/linux/sched/task_stack.h
> index 6a841929073f..624c48defb9e 100644
> --- a/include/linux/sched/task_stack.h
> +++ b/include/linux/sched/task_stack.h
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
>
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
> task_struct *task)
>
> static inline unsigned long *end_of_stack(const struct task_struct *task)
> {
> - return task->stack;
> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
> _task)
> + return task->stack;
> + return (unsigned long *)(task + 1);
> }
This seems like a strange place for the change. It feels more like
init_task has been defined incorrectly.
>>> The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
>>> selected.
>>> include/asm-generic/vmlinux.lds.h:
>>> #define INIT_TASK_DATA(align)\
>>> . = ALIGN(align);\
>>> __start_init_task = .;\
>>> init_thread_union = .;\
>>> init_stack = .;\
>>> KEEP(*(.data..init_task))\
>>> KEEP(*(.data..init_thread_info))\
>>>
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On 2018/11/30 5:22, Kees Cook wrote:
> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
> wrote:
>> Hello Kees,
>>
>> On 2018/11/28 6:38, Kees Cook wrote:
>>> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>>> wrote:
When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
is not a real task on stack, it's only init_task on init_stack.
Commit 0500871f21b2 ("Construct init thread stack in the linker script
rather than by union") added this macro and put task_strcut into
thread_union. This brings us the following possibilities:
TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
- <-- thread_info & stack
NN | | --- <-- task
| || |
- ---
- <-- stack
NY | | --- <--
task(Including thread_info)
| || |
- ---
- <-- stack & task &
thread_info
YN | |
| |
-
- <-- stack &
task(Including thread_info)
YY | |
| |
-
The kernel has handled the first two cases correctly.
For the third case:
TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
should never happen, because the task and thread_info will overlap. So
when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
For the fourth case:
When task on stack, the end of stack should add a sizeof(task_struct)
offset.
This patch handled with the third and fourth case.
Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
Signed-off-by: Wang Dongsheng
Signed-off-by: Shunyong Yang
---
arch/Kconfig | 1 +
include/linux/sched/task_stack.h | 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index e1e540ffa979..0a2c73e73195 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
# Select if arch init_task must go in the __init_task_data section
config ARCH_TASK_STRUCT_ON_STACK
bool
+ depends on THREAD_INFO_IN_TASK || IA64
>>> The "IA64" part shouldn't be needed since IA64 already selects it.
>>>
>>> Since it's selected, it also can't have a depends, IIUC.
>> Since the IA64 thread_info including task_struct, it doesn't need to
>> select THREAD_INFO_IN_TASK.
>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>> THREAD_INFO.
> Okay.
>
# Select if arch has its private alloc_task_struct() function
config ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/include/linux/sched/task_stack.h
b/include/linux/sched/task_stack.h
index 6a841929073f..624c48defb9e 100644
--- a/include/linux/sched/task_stack.h
+++ b/include/linux/sched/task_stack.h
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#ifdef CONFIG_THREAD_INFO_IN_TASK
@@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
task_struct *task)
static inline unsigned long *end_of_stack(const struct task_struct *task)
{
- return task->stack;
+ if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
_task)
+ return task->stack;
+ return (unsigned long *)(task + 1);
}
>>> This seems like a strange place for the change. It feels more like
>>> init_task has been defined incorrectly.
>> The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
>> selected.
>> include/asm-generic/vmlinux.lds.h:
>> #define INIT_TASK_DATA(align)\
>> . = ALIGN(align);\
>> __start_init_task = .;\
>> init_thread_union = .;\
>> init_stack = .;\
>> KEEP(*(.data..init_task))\
>> KEEP(*(.data..init_thread_info))\
>> . = __start_init_task + THREAD_SIZE;\
>> __end_init_task = .;
>>
>> So we need end_of_stack to offset sizeof(task_struct).
> Well, I guess
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On 2018/11/30 5:22, Kees Cook wrote:
> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
> wrote:
>> Hello Kees,
>>
>> On 2018/11/28 6:38, Kees Cook wrote:
>>> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>>> wrote:
When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
is not a real task on stack, it's only init_task on init_stack.
Commit 0500871f21b2 ("Construct init thread stack in the linker script
rather than by union") added this macro and put task_strcut into
thread_union. This brings us the following possibilities:
TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
- <-- thread_info & stack
NN | | --- <-- task
| || |
- ---
- <-- stack
NY | | --- <--
task(Including thread_info)
| || |
- ---
- <-- stack & task &
thread_info
YN | |
| |
-
- <-- stack &
task(Including thread_info)
YY | |
| |
-
The kernel has handled the first two cases correctly.
For the third case:
TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
should never happen, because the task and thread_info will overlap. So
when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
For the fourth case:
When task on stack, the end of stack should add a sizeof(task_struct)
offset.
This patch handled with the third and fourth case.
Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
Signed-off-by: Wang Dongsheng
Signed-off-by: Shunyong Yang
---
arch/Kconfig | 1 +
include/linux/sched/task_stack.h | 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index e1e540ffa979..0a2c73e73195 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
# Select if arch init_task must go in the __init_task_data section
config ARCH_TASK_STRUCT_ON_STACK
bool
+ depends on THREAD_INFO_IN_TASK || IA64
>>> The "IA64" part shouldn't be needed since IA64 already selects it.
>>>
>>> Since it's selected, it also can't have a depends, IIUC.
>> Since the IA64 thread_info including task_struct, it doesn't need to
>> select THREAD_INFO_IN_TASK.
>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>> THREAD_INFO.
> Okay.
>
# Select if arch has its private alloc_task_struct() function
config ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/include/linux/sched/task_stack.h
b/include/linux/sched/task_stack.h
index 6a841929073f..624c48defb9e 100644
--- a/include/linux/sched/task_stack.h
+++ b/include/linux/sched/task_stack.h
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#ifdef CONFIG_THREAD_INFO_IN_TASK
@@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
task_struct *task)
static inline unsigned long *end_of_stack(const struct task_struct *task)
{
- return task->stack;
+ if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
_task)
+ return task->stack;
+ return (unsigned long *)(task + 1);
}
>>> This seems like a strange place for the change. It feels more like
>>> init_task has been defined incorrectly.
>> The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
>> selected.
>> include/asm-generic/vmlinux.lds.h:
>> #define INIT_TASK_DATA(align)\
>> . = ALIGN(align);\
>> __start_init_task = .;\
>> init_thread_union = .;\
>> init_stack = .;\
>> KEEP(*(.data..init_task))\
>> KEEP(*(.data..init_thread_info))\
>> . = __start_init_task + THREAD_SIZE;\
>> __end_init_task = .;
>>
>> So we need end_of_stack to offset sizeof(task_struct).
> Well, I guess
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
wrote:
>
> Hello Kees,
>
> On 2018/11/28 6:38, Kees Cook wrote:
> > On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
> > wrote:
> >> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
> >> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
> >> is not a real task on stack, it's only init_task on init_stack.
> >>
> >> Commit 0500871f21b2 ("Construct init thread stack in the linker script
> >> rather than by union") added this macro and put task_strcut into
> >> thread_union. This brings us the following possibilities:
> >> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
> >> - <-- thread_info & stack
> >> NN | | --- <-- task
> >>| || |
> >> - ---
> >>
> >> - <-- stack
> >> NY | | --- <--
> >> task(Including thread_info)
> >>| || |
> >> - ---
> >>
> >> - <-- stack & task &
> >> thread_info
> >> YN | |
> >>| |
> >> -
> >>
> >> - <-- stack &
> >> task(Including thread_info)
> >> YY | |
> >>| |
> >> -
> >> The kernel has handled the first two cases correctly.
> >>
> >> For the third case:
> >> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
> >> should never happen, because the task and thread_info will overlap. So
> >> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
> >>
> >> For the fourth case:
> >> When task on stack, the end of stack should add a sizeof(task_struct)
> >> offset.
> >>
> >> This patch handled with the third and fourth case.
> >>
> >> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
> >>
> >> Signed-off-by: Wang Dongsheng
> >> Signed-off-by: Shunyong Yang
> >> ---
> >> arch/Kconfig | 1 +
> >> include/linux/sched/task_stack.h | 5 -
> >> 2 files changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/Kconfig b/arch/Kconfig
> >> index e1e540ffa979..0a2c73e73195 100644
> >> --- a/arch/Kconfig
> >> +++ b/arch/Kconfig
> >> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
> >> # Select if arch init_task must go in the __init_task_data section
> >> config ARCH_TASK_STRUCT_ON_STACK
> >> bool
> >> + depends on THREAD_INFO_IN_TASK || IA64
> > The "IA64" part shouldn't be needed since IA64 already selects it.
> >
> > Since it's selected, it also can't have a depends, IIUC.
>
> Since the IA64 thread_info including task_struct, it doesn't need to
> select THREAD_INFO_IN_TASK.
> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
> THREAD_INFO.
Okay.
> >> # Select if arch has its private alloc_task_struct() function
> >> config ARCH_TASK_STRUCT_ALLOCATOR
> >> diff --git a/include/linux/sched/task_stack.h
> >> b/include/linux/sched/task_stack.h
> >> index 6a841929073f..624c48defb9e 100644
> >> --- a/include/linux/sched/task_stack.h
> >> +++ b/include/linux/sched/task_stack.h
> >> @@ -7,6 +7,7 @@
> >> */
> >>
> >> #include
> >> +#include
> >> #include
> >>
> >> #ifdef CONFIG_THREAD_INFO_IN_TASK
> >> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
> >> task_struct *task)
> >>
> >> static inline unsigned long *end_of_stack(const struct task_struct *task)
> >> {
> >> - return task->stack;
> >> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
> >> _task)
> >> + return task->stack;
> >> + return (unsigned long *)(task + 1);
> >> }
> > This seems like a strange place for the change. It feels more like
> > init_task has been defined incorrectly.
>
> The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
> selected.
> include/asm-generic/vmlinux.lds.h:
> #define INIT_TASK_DATA(align)\
> . = ALIGN(align);\
> __start_init_task = .;\
> init_thread_union = .;\
> init_stack = .;\
> KEEP(*(.data..init_task))\
> KEEP(*(.data..init_thread_info))\
> . = __start_init_task + THREAD_SIZE;\
> __end_init_task = .;
>
> So we need end_of_stack to offset sizeof(task_struct).
Well, I guess I mean I'd rather the end_of_stack() code not be
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
wrote:
>
> Hello Kees,
>
> On 2018/11/28 6:38, Kees Cook wrote:
> > On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
> > wrote:
> >> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
> >> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
> >> is not a real task on stack, it's only init_task on init_stack.
> >>
> >> Commit 0500871f21b2 ("Construct init thread stack in the linker script
> >> rather than by union") added this macro and put task_strcut into
> >> thread_union. This brings us the following possibilities:
> >> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
> >> - <-- thread_info & stack
> >> NN | | --- <-- task
> >>| || |
> >> - ---
> >>
> >> - <-- stack
> >> NY | | --- <--
> >> task(Including thread_info)
> >>| || |
> >> - ---
> >>
> >> - <-- stack & task &
> >> thread_info
> >> YN | |
> >>| |
> >> -
> >>
> >> - <-- stack &
> >> task(Including thread_info)
> >> YY | |
> >>| |
> >> -
> >> The kernel has handled the first two cases correctly.
> >>
> >> For the third case:
> >> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
> >> should never happen, because the task and thread_info will overlap. So
> >> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
> >>
> >> For the fourth case:
> >> When task on stack, the end of stack should add a sizeof(task_struct)
> >> offset.
> >>
> >> This patch handled with the third and fourth case.
> >>
> >> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
> >>
> >> Signed-off-by: Wang Dongsheng
> >> Signed-off-by: Shunyong Yang
> >> ---
> >> arch/Kconfig | 1 +
> >> include/linux/sched/task_stack.h | 5 -
> >> 2 files changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/Kconfig b/arch/Kconfig
> >> index e1e540ffa979..0a2c73e73195 100644
> >> --- a/arch/Kconfig
> >> +++ b/arch/Kconfig
> >> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
> >> # Select if arch init_task must go in the __init_task_data section
> >> config ARCH_TASK_STRUCT_ON_STACK
> >> bool
> >> + depends on THREAD_INFO_IN_TASK || IA64
> > The "IA64" part shouldn't be needed since IA64 already selects it.
> >
> > Since it's selected, it also can't have a depends, IIUC.
>
> Since the IA64 thread_info including task_struct, it doesn't need to
> select THREAD_INFO_IN_TASK.
> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
> THREAD_INFO.
Okay.
> >> # Select if arch has its private alloc_task_struct() function
> >> config ARCH_TASK_STRUCT_ALLOCATOR
> >> diff --git a/include/linux/sched/task_stack.h
> >> b/include/linux/sched/task_stack.h
> >> index 6a841929073f..624c48defb9e 100644
> >> --- a/include/linux/sched/task_stack.h
> >> +++ b/include/linux/sched/task_stack.h
> >> @@ -7,6 +7,7 @@
> >> */
> >>
> >> #include
> >> +#include
> >> #include
> >>
> >> #ifdef CONFIG_THREAD_INFO_IN_TASK
> >> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
> >> task_struct *task)
> >>
> >> static inline unsigned long *end_of_stack(const struct task_struct *task)
> >> {
> >> - return task->stack;
> >> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
> >> _task)
> >> + return task->stack;
> >> + return (unsigned long *)(task + 1);
> >> }
> > This seems like a strange place for the change. It feels more like
> > init_task has been defined incorrectly.
>
> The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
> selected.
> include/asm-generic/vmlinux.lds.h:
> #define INIT_TASK_DATA(align)\
> . = ALIGN(align);\
> __start_init_task = .;\
> init_thread_union = .;\
> init_stack = .;\
> KEEP(*(.data..init_task))\
> KEEP(*(.data..init_thread_info))\
> . = __start_init_task + THREAD_SIZE;\
> __end_init_task = .;
>
> So we need end_of_stack to offset sizeof(task_struct).
Well, I guess I mean I'd rather the end_of_stack() code not be
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
Hello Kees,
On 2018/11/28 6:38, Kees Cook wrote:
> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
> wrote:
>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>> is not a real task on stack, it's only init_task on init_stack.
>>
>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>> rather than by union") added this macro and put task_strcut into
>> thread_union. This brings us the following possibilities:
>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>> - <-- thread_info & stack
>> NN | | --- <-- task
>>| || |
>> - ---
>>
>> - <-- stack
>> NY | | --- <--
>> task(Including thread_info)
>>| || |
>> - ---
>>
>> - <-- stack & task &
>> thread_info
>> YN | |
>>| |
>> -
>>
>> - <-- stack & task(Including
>> thread_info)
>> YY | |
>>| |
>> -
>> The kernel has handled the first two cases correctly.
>>
>> For the third case:
>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>> should never happen, because the task and thread_info will overlap. So
>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>
>> For the fourth case:
>> When task on stack, the end of stack should add a sizeof(task_struct) offset.
>>
>> This patch handled with the third and fourth case.
>>
>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>
>> Signed-off-by: Wang Dongsheng
>> Signed-off-by: Shunyong Yang
>> ---
>> arch/Kconfig | 1 +
>> include/linux/sched/task_stack.h | 5 -
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index e1e540ffa979..0a2c73e73195 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>> # Select if arch init_task must go in the __init_task_data section
>> config ARCH_TASK_STRUCT_ON_STACK
>> bool
>> + depends on THREAD_INFO_IN_TASK || IA64
> The "IA64" part shouldn't be needed since IA64 already selects it.
>
> Since it's selected, it also can't have a depends, IIUC.
Since the IA64 thread_info including task_struct, it doesn't need to
select THREAD_INFO_IN_TASK.
So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
THREAD_INFO.
>> # Select if arch has its private alloc_task_struct() function
>> config ARCH_TASK_STRUCT_ALLOCATOR
>> diff --git a/include/linux/sched/task_stack.h
>> b/include/linux/sched/task_stack.h
>> index 6a841929073f..624c48defb9e 100644
>> --- a/include/linux/sched/task_stack.h
>> +++ b/include/linux/sched/task_stack.h
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include
>> +#include
>> #include
>>
>> #ifdef CONFIG_THREAD_INFO_IN_TASK
>> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
>> task_struct *task)
>>
>> static inline unsigned long *end_of_stack(const struct task_struct *task)
>> {
>> - return task->stack;
>> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
>> _task)
>> + return task->stack;
>> + return (unsigned long *)(task + 1);
>> }
> This seems like a strange place for the change. It feels more like
> init_task has been defined incorrectly.
The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
selected.
include/asm-generic/vmlinux.lds.h:
#define INIT_TASK_DATA(align)\
. = ALIGN(align);\
__start_init_task = .;\
init_thread_union = .;\
init_stack = .;\
KEEP(*(.data..init_task))\
KEEP(*(.data..init_thread_info))\
. = __start_init_task + THREAD_SIZE;\
__end_init_task = .;
So we need end_of_stack to offset sizeof(task_struct).
Cheers,
Dongsheng
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
Hello Kees,
On 2018/11/28 6:38, Kees Cook wrote:
> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
> wrote:
>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>> is not a real task on stack, it's only init_task on init_stack.
>>
>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>> rather than by union") added this macro and put task_strcut into
>> thread_union. This brings us the following possibilities:
>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>> - <-- thread_info & stack
>> NN | | --- <-- task
>>| || |
>> - ---
>>
>> - <-- stack
>> NY | | --- <--
>> task(Including thread_info)
>>| || |
>> - ---
>>
>> - <-- stack & task &
>> thread_info
>> YN | |
>>| |
>> -
>>
>> - <-- stack & task(Including
>> thread_info)
>> YY | |
>>| |
>> -
>> The kernel has handled the first two cases correctly.
>>
>> For the third case:
>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>> should never happen, because the task and thread_info will overlap. So
>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>
>> For the fourth case:
>> When task on stack, the end of stack should add a sizeof(task_struct) offset.
>>
>> This patch handled with the third and fourth case.
>>
>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>
>> Signed-off-by: Wang Dongsheng
>> Signed-off-by: Shunyong Yang
>> ---
>> arch/Kconfig | 1 +
>> include/linux/sched/task_stack.h | 5 -
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index e1e540ffa979..0a2c73e73195 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>> # Select if arch init_task must go in the __init_task_data section
>> config ARCH_TASK_STRUCT_ON_STACK
>> bool
>> + depends on THREAD_INFO_IN_TASK || IA64
> The "IA64" part shouldn't be needed since IA64 already selects it.
>
> Since it's selected, it also can't have a depends, IIUC.
Since the IA64 thread_info including task_struct, it doesn't need to
select THREAD_INFO_IN_TASK.
So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
THREAD_INFO.
>> # Select if arch has its private alloc_task_struct() function
>> config ARCH_TASK_STRUCT_ALLOCATOR
>> diff --git a/include/linux/sched/task_stack.h
>> b/include/linux/sched/task_stack.h
>> index 6a841929073f..624c48defb9e 100644
>> --- a/include/linux/sched/task_stack.h
>> +++ b/include/linux/sched/task_stack.h
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include
>> +#include
>> #include
>>
>> #ifdef CONFIG_THREAD_INFO_IN_TASK
>> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
>> task_struct *task)
>>
>> static inline unsigned long *end_of_stack(const struct task_struct *task)
>> {
>> - return task->stack;
>> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
>> _task)
>> + return task->stack;
>> + return (unsigned long *)(task + 1);
>> }
> This seems like a strange place for the change. It feels more like
> init_task has been defined incorrectly.
The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
selected.
include/asm-generic/vmlinux.lds.h:
#define INIT_TASK_DATA(align)\
. = ALIGN(align);\
__start_init_task = .;\
init_thread_union = .;\
init_stack = .;\
KEEP(*(.data..init_task))\
KEEP(*(.data..init_thread_info))\
. = __start_init_task + THREAD_SIZE;\
__end_init_task = .;
So we need end_of_stack to offset sizeof(task_struct).
Cheers,
Dongsheng
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
wrote:
> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
> is not a real task on stack, it's only init_task on init_stack.
>
> Commit 0500871f21b2 ("Construct init thread stack in the linker script
> rather than by union") added this macro and put task_strcut into
> thread_union. This brings us the following possibilities:
> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
> - <-- thread_info & stack
> NN | | --- <-- task
>| || |
> - ---
>
> - <-- stack
> NY | | --- <-- task(Including
> thread_info)
>| || |
> - ---
>
> - <-- stack & task &
> thread_info
> YN | |
>| |
> -
>
> - <-- stack & task(Including
> thread_info)
> YY | |
>| |
> -
> The kernel has handled the first two cases correctly.
>
> For the third case:
> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
> should never happen, because the task and thread_info will overlap. So
> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>
> For the fourth case:
> When task on stack, the end of stack should add a sizeof(task_struct) offset.
>
> This patch handled with the third and fourth case.
>
> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>
> Signed-off-by: Wang Dongsheng
> Signed-off-by: Shunyong Yang
> ---
> arch/Kconfig | 1 +
> include/linux/sched/task_stack.h | 5 -
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e1e540ffa979..0a2c73e73195 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
> # Select if arch init_task must go in the __init_task_data section
> config ARCH_TASK_STRUCT_ON_STACK
> bool
> + depends on THREAD_INFO_IN_TASK || IA64
The "IA64" part shouldn't be needed since IA64 already selects it.
Since it's selected, it also can't have a depends, IIUC.
>
> # Select if arch has its private alloc_task_struct() function
> config ARCH_TASK_STRUCT_ALLOCATOR
> diff --git a/include/linux/sched/task_stack.h
> b/include/linux/sched/task_stack.h
> index 6a841929073f..624c48defb9e 100644
> --- a/include/linux/sched/task_stack.h
> +++ b/include/linux/sched/task_stack.h
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
>
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
> task_struct *task)
>
> static inline unsigned long *end_of_stack(const struct task_struct *task)
> {
> - return task->stack;
> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
> _task)
> + return task->stack;
> + return (unsigned long *)(task + 1);
> }
This seems like a strange place for the change. It feels more like
init_task has been defined incorrectly.
-Kees
>
> #elif !defined(__HAVE_THREAD_FUNCTIONS)
> --
> 2.19.1
>
--
Kees Cook
Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
wrote:
> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
> is not a real task on stack, it's only init_task on init_stack.
>
> Commit 0500871f21b2 ("Construct init thread stack in the linker script
> rather than by union") added this macro and put task_strcut into
> thread_union. This brings us the following possibilities:
> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
> - <-- thread_info & stack
> NN | | --- <-- task
>| || |
> - ---
>
> - <-- stack
> NY | | --- <-- task(Including
> thread_info)
>| || |
> - ---
>
> - <-- stack & task &
> thread_info
> YN | |
>| |
> -
>
> - <-- stack & task(Including
> thread_info)
> YY | |
>| |
> -
> The kernel has handled the first two cases correctly.
>
> For the third case:
> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
> should never happen, because the task and thread_info will overlap. So
> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>
> For the fourth case:
> When task on stack, the end of stack should add a sizeof(task_struct) offset.
>
> This patch handled with the third and fourth case.
>
> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>
> Signed-off-by: Wang Dongsheng
> Signed-off-by: Shunyong Yang
> ---
> arch/Kconfig | 1 +
> include/linux/sched/task_stack.h | 5 -
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e1e540ffa979..0a2c73e73195 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
> # Select if arch init_task must go in the __init_task_data section
> config ARCH_TASK_STRUCT_ON_STACK
> bool
> + depends on THREAD_INFO_IN_TASK || IA64
The "IA64" part shouldn't be needed since IA64 already selects it.
Since it's selected, it also can't have a depends, IIUC.
>
> # Select if arch has its private alloc_task_struct() function
> config ARCH_TASK_STRUCT_ALLOCATOR
> diff --git a/include/linux/sched/task_stack.h
> b/include/linux/sched/task_stack.h
> index 6a841929073f..624c48defb9e 100644
> --- a/include/linux/sched/task_stack.h
> +++ b/include/linux/sched/task_stack.h
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
>
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct
> task_struct *task)
>
> static inline unsigned long *end_of_stack(const struct task_struct *task)
> {
> - return task->stack;
> + if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task !=
> _task)
> + return task->stack;
> + return (unsigned long *)(task + 1);
> }
This seems like a strange place for the change. It feels more like
init_task has been defined incorrectly.
-Kees
>
> #elif !defined(__HAVE_THREAD_FUNCTIONS)
> --
> 2.19.1
>
--
Kees Cook
[PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
is not a real task on stack, it's only init_task on init_stack.
Commit 0500871f21b2 ("Construct init thread stack in the linker script
rather than by union") added this macro and put task_strcut into
thread_union. This brings us the following possibilities:
TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
- <-- thread_info & stack
NN | | --- <-- task
| || |
- ---
- <-- stack
NY | | --- <-- task(Including
thread_info)
| || |
- ---
- <-- stack & task & thread_info
YN | |
| |
-
- <-- stack & task(Including
thread_info)
YY | |
| |
-
The kernel has handled the first two cases correctly.
For the third case:
TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
should never happen, because the task and thread_info will overlap. So
when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
For the fourth case:
When task on stack, the end of stack should add a sizeof(task_struct) offset.
This patch handled with the third and fourth case.
Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
Signed-off-by: Wang Dongsheng
Signed-off-by: Shunyong Yang
---
arch/Kconfig | 1 +
include/linux/sched/task_stack.h | 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index e1e540ffa979..0a2c73e73195 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
# Select if arch init_task must go in the __init_task_data section
config ARCH_TASK_STRUCT_ON_STACK
bool
+ depends on THREAD_INFO_IN_TASK || IA64
# Select if arch has its private alloc_task_struct() function
config ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/include/linux/sched/task_stack.h b/include/linux/sched/task_stack.h
index 6a841929073f..624c48defb9e 100644
--- a/include/linux/sched/task_stack.h
+++ b/include/linux/sched/task_stack.h
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#ifdef CONFIG_THREAD_INFO_IN_TASK
@@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct task_struct
*task)
static inline unsigned long *end_of_stack(const struct task_struct *task)
{
- return task->stack;
+ if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task != _task)
+ return task->stack;
+ return (unsigned long *)(task + 1);
}
#elif !defined(__HAVE_THREAD_FUNCTIONS)
--
2.19.1
[PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC
When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
is not a real task on stack, it's only init_task on init_stack.
Commit 0500871f21b2 ("Construct init thread stack in the linker script
rather than by union") added this macro and put task_strcut into
thread_union. This brings us the following possibilities:
TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
- <-- thread_info & stack
NN | | --- <-- task
| || |
- ---
- <-- stack
NY | | --- <-- task(Including
thread_info)
| || |
- ---
- <-- stack & task & thread_info
YN | |
| |
-
- <-- stack & task(Including
thread_info)
YY | |
| |
-
The kernel has handled the first two cases correctly.
For the third case:
TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
should never happen, because the task and thread_info will overlap. So
when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
For the fourth case:
When task on stack, the end of stack should add a sizeof(task_struct) offset.
This patch handled with the third and fourth case.
Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
Signed-off-by: Wang Dongsheng
Signed-off-by: Shunyong Yang
---
arch/Kconfig | 1 +
include/linux/sched/task_stack.h | 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index e1e540ffa979..0a2c73e73195 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
# Select if arch init_task must go in the __init_task_data section
config ARCH_TASK_STRUCT_ON_STACK
bool
+ depends on THREAD_INFO_IN_TASK || IA64
# Select if arch has its private alloc_task_struct() function
config ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/include/linux/sched/task_stack.h b/include/linux/sched/task_stack.h
index 6a841929073f..624c48defb9e 100644
--- a/include/linux/sched/task_stack.h
+++ b/include/linux/sched/task_stack.h
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#ifdef CONFIG_THREAD_INFO_IN_TASK
@@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct task_struct
*task)
static inline unsigned long *end_of_stack(const struct task_struct *task)
{
- return task->stack;
+ if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task != _task)
+ return task->stack;
+ return (unsigned long *)(task + 1);
}
#elif !defined(__HAVE_THREAD_FUNCTIONS)
--
2.19.1
