Fix end_of_stack() and location of stack canary for archs using STACK_GROWSUP
PATCH v2 1/1 Fix end_of_stack() and location of stack canary for archs using STACK_GROWSUP
Aaron Tomlin recently posted patches [1] to enable checking the stack canary on every task switch. Looking at the canary code, I realized that every arch (except ia64, which adds some space for register spill above the stack) shares a definition of end_of_stack() that makes it the first long after the threadinfo.
For stacks that grow down, this low address is correct because the stack starts at the end of the thread area and grows toward lower addresses. However, for stacks that grow up, toward higher addresses, this is wrong. (The stack actually grows away from the canary.) On these archs end_of_stack() should return the address of the last long, at the highest possible address for the stack.
[1] http://lkml.org/lkml/2014/9/12/293
---
V2: Fix line length, add Tested-by and Acked-bys
include/linux/sched.h +5 -0
@@ -2611,7 +2611,12 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_structstatic inline unsigned long *end_of_stack (struct task_struct *p) { +#ifdef CONFIG_STACK_GROWSUP +return (unsigned long *) + ((unsigned long )task_thread_info (p) +THREAD_SIZE ) -1 ; +#else return (unsigned long *)(task_thread_info (p) +1 ); +#endif }#endif
tip-bot for Chuck Ebbert
merge bot[tip:sched/urgent] sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
Commit-ID: a3215fb47c7ecb814dc16815245db4f375841268 Gitweb: http://git.kernel.org/tip/a3215fb47c7ecb814dc16815245db4f375841268 Author: Chuck Ebbert <[email protected]> AuthorDate: Fri, 19 Sep 2014 09:35:05 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 20 Sep 2014 08:38:16 +0200
sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
Aaron Tomlin recently posted patches to enable checking the stack canary on every task switch:
http://lkml.org/lkml/2014/9/12/293
Looking at the canary code, I realized that every arch (except ia64, which adds some space for register spill above the stack) shares a definition of end_of_stack() that makes it the first long after the threadinfo.
For stacks that grow down, this low address is correct because the stack starts at the end of the thread area and grows toward lower addresses. However, for stacks that grow up, toward higher addresses, this is wrong. (The stack actually grows away from the canary.)
On these archs end_of_stack() should return the address of the last long, at the highest possible address for the stack.
--- include/linux/sched.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
include/linux/sched.h +13 -0
@@ -2608,9 +2608,22 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_struct task_thread_info(p)->task = p; } +/* +* Return the address of the first long before (or after, +* depending on the architecture's default stack growth +* direction) * a task's task_info structure, which is the +* kernel stack's last usable spot. +* +* This is the point of no return, if the stack grows +* beyond that position, we corrupt the task's state. +*/ static inline unsigned long *end_of_stack (struct task_struct *p) { +#ifdef CONFIG_STACK_GROWSUP +return (unsigned long *)((unsigned long )task_thread_info (p) +THREAD_SIZE ) -1 ; +#else return (unsigned long *)(task_thread_info (p) +1 ); +#endif }#endif
Re: [tip:sched/urgent] sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
Quoted 47 lines · On Fri, 19 Sep 2014 23:42:37 -0700 tip-bot for Chuck Ebbert <[email protected]>…
Commit-ID: a3215fb47c7ecb814dc16815245db4f375841268 Gitweb: http://git.kernel.org/tip/a3215fb47c7ecb814dc16815245db4f375841268 Author: Chuck Ebbert <[email protected]> AuthorDate: Fri, 19 Sep 2014 09:35:05 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 20 Sep 2014 08:38:16 +0200sched: Fix
end_of_stack()and location of stack canary for architectures usingCONFIG_STACK_GROWSUPAaron Tomlin recently posted patches to enable checking the stack canary on every task switch:
http://lkml.org/lkml/2014/9/12/293
Looking at the canary code, I realized that every arch (except ia64, which adds some space for register spill above the stack) shares a definition of
end_of_stack()that makes it the first long after the threadinfo.For stacks that grow down, this low address is correct because the stack starts at the end of the thread area and grows toward lower addresses. However, for stacks that grow up, toward higher addresses, this is wrong. (The stack actually grows away from the canary.)
On these archs
end_of_stack()should return the address of the last long, at the highest possible address for the stack.Signed-off-by Chuck Ebbert <[email protected]>Tested-by James Hogan <[email protected]> [metag]Acked-by James Hogan <[email protected]>Acked-by Aaron Tomlin <[email protected]>[ Added comments to end_of_stack(). ]Signed-off-by Ingo Molnar <[email protected]>--- include/linux/sched.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)include/linux/sched.h +0
-0@@ -2608,9 +2608,22 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_structtask_thread_info (p)->task = p; }
include/linux/sched.h static inline void setup_thread_stack(struct task_struct *p, struct task_struct · line 2619
Quoted 10 lines
include/linux/sched.h +9
-0+/* +* Return the address of the first long before (or after, +* depending on the architecture's default stack growth +* direction) * a task's task_info structure, which is the +* kernel stack's last usable spot. +* +* This is the point of no return, if the stack grows +* beyond that position, we corrupt the task's state. +*/
This comment you added is not correct for archs where the stack grows up. The threadinfo is always at the lowest address on the stack in every case. Instead of corrupting the thread info, a stack overflow will corrupt whatever is on the next page after the stack if it grows upward.
Quoted 10 lines
include/linux/sched.h +4
-0static inline unsigned long *end_of_stack (struct task_struct *p) { +#ifdef CONFIG_STACK_GROWSUP +return (unsigned long *)((unsigned long )task_thread_info (p) +THREAD_SIZE ) -1 ; +#else return (unsigned long *)(task_thread_info (p) +1 ); +#endif }#endif
Re: [tip:sched/urgent] sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
Quoted 51 lines · Chuck Ebbert
Quoted 48 lines · On Fri, 19 Sep 2014 23:42:37 -0700 tip-bot for Chuck Ebbert <[email protected]>…
Commit-ID: a3215fb47c7ecb814dc16815245db4f375841268 Gitweb: http://git.kernel.org/tip/a3215fb47c7ecb814dc16815245db4f375841268 Author: Chuck Ebbert <[email protected]> AuthorDate: Fri, 19 Sep 2014 09:35:05 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 20 Sep 2014 08:38:16 +0200sched: Fix
end_of_stack()and location of stack canary for architectures usingCONFIG_STACK_GROWSUPAaron Tomlin recently posted patches to enable checking the stack canary on every task switch:
http://lkml.org/lkml/2014/9/12/293
Looking at the canary code, I realized that every arch (except ia64, which adds some space for register spill above the stack) shares a definition of
end_of_stack()that makes it the first long after the threadinfo.For stacks that grow down, this low address is correct because the stack starts at the end of the thread area and grows toward lower addresses. However, for stacks that grow up, toward higher addresses, this is wrong. (The stack actually grows away from the canary.)
On these archs
end_of_stack()should return the address of the last long, at the highest possible address for the stack.Signed-off-by Chuck Ebbert <[email protected]>Tested-by James Hogan <[email protected]> [metag]Acked-by James Hogan <[email protected]>Acked-by Aaron Tomlin <[email protected]>[ Added comments to end_of_stack(). ]Signed-off-by Ingo Molnar <[email protected]>--- include/linux/sched.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)include/linux/sched.h +0
-0@@ -2608,9 +2608,22 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_structtask_thread_info (p)->task = p; }
Quoted 15 lines
include/linux/sched.h static inline void setup_thread_stack(struct task_struct *p, struct task_struct · line 2619
Quoted 9 lines
include/linux/sched.h +9
-0+/* +* Return the address of the first long before (or after, +* depending on the architecture's default stack growth +* direction) * a task's task_info structure, which is the +* kernel stack's last usable spot. +* +* This is the point of no return, if the stack grows +* beyond that position, we corrupt the task's state. +*/ This comment you added is not correct for archs where the stack grows up. The threadinfo is always at the lowest address on the stack in every case. Instead of corrupting the thread info, a stack overflow will corrupt whatever is on the next page after the stack if it grows upward.
Okay, I've zapped the commit, please resubmit the patch with the comment corrected.
Thanks,
Ingo
PATCH v3 1/1 sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
Aaron Tomlin recently posted patches [1] to enable checking the stack canary on every task switch. Looking at the canary code, I realized that every arch (except ia64, which adds some space for register spill above the stack) shares a definition of end_of_stack() that makes it the first long after the threadinfo.
For stacks that grow down, this low address is correct because the stack starts at the end of the thread area and grows toward lower addresses. However, for stacks that grow up, toward higher addresses, this is wrong. (The stack actually grows away from the canary.) On these archs end_of_stack() should return the address of the last long, at the highest possible address for the stack.
[1] http://lkml.org/lkml/2014/9/12/293
---
V3: Fix line length, add comment.
include/linux/sched.h +14 -0
@@ -2609,9 +2609,23 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_struct task_thread_info(p)->task = p; } +/* +* Return the address of the last usable long on the stack. +* +* When the stack grows down, this is just above the thread +* info struct. Going any lower will corrupt the threadinfo. +* +* When the stack grows up, this is the highest address. +* Beyond that position, we corrupt data on the next page. +*/ static inline unsigned long *end_of_stack (struct task_struct *p) { +#ifdef CONFIG_STACK_GROWSUP +return (unsigned long *) + ((unsigned long )task_thread_info (p) +THREAD_SIZE ) -1 ; +#else return (unsigned long *)(task_thread_info (p) +1 ); +#endif }#endif
tip-bot for Chuck Ebbert
merge bot[tip:sched/urgent] sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
Commit-ID: 6a40281ab5c1ed8ba2253857118a5d400a2d084b Gitweb: http://git.kernel.org/tip/6a40281ab5c1ed8ba2253857118a5d400a2d084b Author: Chuck Ebbert <[email protected]> AuthorDate: Sat, 20 Sep 2014 10:17:51 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 20 Sep 2014 19:44:04 +0200
sched: Fix end_of_stack() and location of stack canary for architectures using CONFIG_STACK_GROWSUP
Aaron Tomlin recently posted patches [1] to enable checking the stack canary on every task switch. Looking at the canary code, I realized that every arch (except ia64, which adds some space for register spill above the stack) shares a definition of end_of_stack() that makes it the first long after the threadinfo.
For stacks that grow down, this low address is correct because the stack starts at the end of the thread area and grows toward lower addresses. However, for stacks that grow up, toward higher addresses, this is wrong. (The stack actually grows away from the canary.) On these archs end_of_stack() should return the address of the last long, at the highest possible address for the stack.
[1] http://lkml.org/lkml/2014/9/12/293
--- include/linux/sched.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
include/linux/sched.h +13 -0
@@ -2608,9 +2608,22 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_struct task_thread_info(p)->task = p; } +/* +* Return the address of the last usable long on the stack. +* +* When the stack grows down, this is just above the thread +* info struct. Going any lower will corrupt the threadinfo. +* +* When the stack grows up, this is the highest address. +* Beyond that position, we corrupt data on the next page. +*/ static inline unsigned long *end_of_stack (struct task_struct *p) { +#ifdef CONFIG_STACK_GROWSUP +return (unsigned long *)((unsigned long )task_thread_info (p) +THREAD_SIZE ) -1 ; +#else return (unsigned long *)(task_thread_info (p) +1 ); +#endif }#endif