fork: Allow stack to be wiped on fork
Subsystems (3)
PATCH 1/1 fork: Allow stack to be wiped on fork
One of the classes of kernel stack content leaks is exposing the contents of prior heap or stack contents when a new process stack is allocated. Normally, those stacks are not zeroed, and the old contents remain in place. With some types of stack content exposure flaws, those contents can leak to userspace. Kernels built with CONFIG_CLEAR_STACK_FORK will no longer be vulnerable to this, as the stack will be wiped each time a stack is assigned to a new process. There's not a meaningful change in runtime performance; it almost looks like it provides a benefit.
Performing back-to-back kernel builds before:
Run times: 157.86 157.09 158.90 160.94 160.80 Mean: 159.12 Std Dev: 1.54
With CONFIG_CLEAR_STACK_FORK=y:
Run times: 159.31 157.34 156.71 158.15 160.81 Mean: 158.46 Std Dev: 1.46
--- arch/Kconfig | 8 ++++++++ include/linux/thread_info.h | 4 +++- kernel/fork.c | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-)
arch/Kconfig +8 -0
@@ -904,6 +904,14 @@ config VMAP_STACK the stack to map directly to the KASAN shadow map using a formula that is incorrect if the stack is in vmalloc space. +config CLEAR_STACK_FORK + bool "Clear the kernel stack at each fork" + help + To resist stack content leak flaws, this clears newly allocated + kernel stacks to keep previously freed heap or stack contents + from being present in the new stack. This has almost no + measurable performance impact. + config ARCH_OPTIONAL_KERNEL_RWX def_bool n
include/linux/thread_info.h +3 -1
@@ -43,7 +43,9 @@ enum {#define THREAD_ALIGN THREAD_SIZE#endif -#if IS_ENABLED (CONFIG_DEBUG_STACK_USAGE ) ||IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) +#if IS_ENABLED (CONFIG_DEBUG_STACK_USAGE ) || \ +IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) || \ +IS_ENABLED (CONFIG_CLEAR_STACK_FORK )# define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_ZERO)#else # define THREADINFO_GFP (GFP_KERNEL_ACCOUNT)
kernel/fork.c +1 -1
@@ -215,7 +215,7 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)if (!s)continue ; -#ifdef CONFIG_DEBUG_KMEMLEAK +#if IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) ||IS_ENABLED (CONFIG_CLEAR_STACK_FORK )/* Clear stale pointers from reused stack. */ memset (s->addr,0 ,THREAD_SIZE );#endif
Signature · 6 lines
2.7.4 -- Kees Cook Pixel Security
Quoted 8 lines · Kees Cook
One of the classes of kernel stack content leaks is exposing the contents of prior heap or stack contents when a new process stack is allocated. Normally, those stacks are not zeroed, and the old contents remain in place. With some types of stack content exposure flaws, those contents can leak to userspace. Kernels built with
CONFIG_CLEAR_STACK_FORKwill no longer be vulnerable to this, as the stack will be wiped each time a stack is assigned to a new process. There's not a meaningful change in runtime performance; it almost looks like it provides a benefit.
Have you tried something as simple as /bin/true in a loop. kbuild will certainly amortize few cycles for the clearing and I would expect, most reasonable applications would do as well. But it would be better to know the worst case scenario IMHO.
Quoted 11 lines
Performing back-to-back kernel builds before:
Run times: 157.86 157.09 158.90 160.94 160.80 Mean: 159.12 Std Dev: 1.54With
CONFIG_CLEAR_STACK_FORK=y:Run times: 159.31 157.34 156.71 158.15 160.81 Mean: 158.46 Std Dev: 1.46Signed-off-by Kees Cook <[email protected]>
The change seems reasonable to me. Although it would be better to extend on the types of attacks this prevents from, with some examples ideally. How many attacks of that kind we had in the past and how often they appear. That might help people to decide whether to deserve few cycles on each fork. Also the config option sounds rather limiting. Consider distros, should they enable it just to be on the safe side? This is kind of generic concern with other hardening options though.
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 10 lines · Michal Hocko
Quoted 8 lines · Kees Cook
One of the classes of kernel stack content leaks is exposing the contents of prior heap or stack contents when a new process stack is allocated. Normally, those stacks are not zeroed, and the old contents remain in place. With some types of stack content exposure flaws, those contents can leak to userspace. Kernels built with
CONFIG_CLEAR_STACK_FORKwill no longer be vulnerable to this, as the stack will be wiped each time a stack is assigned to a new process. There's not a meaningful change in runtime performance; it almost looks like it provides a benefit.
Quoted 5 lines
Have you tried something as simple as
/bin/truein a loop. kbuild will certainly amortize few cycles for the clearing and I would expect, most reasonable applications would do as well. But it would be better to know the worst case scenario IMHO.
I tried /bin/true in a loop in my QEMU setup and didn't see a difference there.
Quoted 12 lines
Quoted 11 lines
Performing back-to-back kernel builds before:
Run times: 157.86 157.09 158.90 160.94 160.80 Mean: 159.12 Std Dev: 1.54With
CONFIG_CLEAR_STACK_FORK=y:Run times: 159.31 157.34 156.71 158.15 160.81 Mean: 158.46 Std Dev: 1.46Signed-off-by Kees Cook <[email protected]>
Quoted 8 lines
The change seems reasonable to me. Although it would be better to extend on the types of attacks this prevents from, with some examples ideally. How many attacks of that kind we had in the past and how often they appear. That might help people to decide whether to deserve few cycles on each fork. Also the config option sounds rather limiting. Consider distros, should they enable it just to be on the safe side? This is kind of generic concern with other hardening options though.
Agreed this could use a few more words, but it looks good to me overall.
Thanks,
Laura
Quoted 20 lines · Kees Cook
include/linux/thread_info.h +3
-1@@ -43,7 +43,9 @@ enum {#define THREAD_ALIGN THREAD_SIZE#endif -#if IS_ENABLED (CONFIG_DEBUG_STACK_USAGE ) ||IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) +#if IS_ENABLED (CONFIG_DEBUG_STACK_USAGE ) || \ +IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) || \ +IS_ENABLED (CONFIG_CLEAR_STACK_FORK )# define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_ZERO)#else # define THREADINFO_GFP (GFP_KERNEL_ACCOUNT)kernel/fork.c +0
-0
kernel/fork.c static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) · line 221
Quoted 8 lines
kernel/fork.c +1
-1@@ -215,7 +215,7 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)if (!s)continue ; -#ifdef CONFIG_DEBUG_KMEMLEAK +#if IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) ||IS_ENABLED (CONFIG_CLEAR_STACK_FORK )/* Clear stale pointers from reused stack. */ memset (s->addr,0 ,THREAD_SIZE );#endif
Is there any good reason not to do it symmetricaly also for non-vmapped stacks? (by passing __GFP_ZERO to alloc_pages_node())?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Quoted 23 lines · Jiri Kosina
Quoted 23 lines
include/linux/thread_info.h +3
-1@@ -43,7 +43,9 @@ enum {#define THREAD_ALIGN THREAD_SIZE#endif -#if IS_ENABLED (CONFIG_DEBUG_STACK_USAGE ) ||IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) +#if IS_ENABLED (CONFIG_DEBUG_STACK_USAGE ) || \ +IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) || \ +IS_ENABLED (CONFIG_CLEAR_STACK_FORK )# define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_ZERO)#else # define THREADINFO_GFP (GFP_KERNEL_ACCOUNT)kernel/fork.c +0
-0@@ -215,7 +215,7 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)if (!s)continue ;
Quoted 8 lines
kernel/fork.c static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) · line 221
Quoted 5 lines
kernel/fork.c +1
-1-#ifdef CONFIG_DEBUG_KMEMLEAK +#if IS_ENABLED (CONFIG_DEBUG_KMEMLEAK ) ||IS_ENABLED (CONFIG_CLEAR_STACK_FORK )/* Clear stale pointers from reused stack. */ memset (s->addr,0 ,THREAD_SIZE );#endif Is there any good reason not to do it symmetricaly also for non-vmapped stacks? (by passing
__GFP_ZEROtoalloc_pages_node())?
Ah, of course you already do by extending THREADINFO_GFP, sorry for the noise.
Signature · 1 line
Jiri Kosina
Andrew Morton
maintainerQuoted 25 lines · On Tue, 16 Jan 2018 21:50:15 -0800 Kees Cook <[email protected]> wrote:
One of the classes of kernel stack content leaks is exposing the contents of prior heap or stack contents when a new process stack is allocated. Normally, those stacks are not zeroed, and the old contents remain in place. With some types of stack content exposure flaws, those contents can leak to userspace. Kernels built with
CONFIG_CLEAR_STACK_FORKwill no longer be vulnerable to this, as the stack will be wiped each time a stack is assigned to a new process. There's not a meaningful change in runtime performance; it almost looks like it provides a benefit.Performing back-to-back kernel builds before:
Run times: 157.86 157.09 158.90 160.94 160.80 Mean: 159.12 Std Dev: 1.54With
CONFIG_CLEAR_STACK_FORK=y:Run times: 159.31 157.34 156.71 158.15 160.81 Mean: 158.46 Std Dev: 1.46...
arch/Kconfig +0
-0@@ -904,6 +904,14 @@ config VMAP_STACK the stack to map directly to the KASAN shadow map using a formula
arch/Kconfig config VMAP_STACK · line 914
Quoted 10 lines
arch/Kconfig +8
-0that is incorrect if the stack is in vmalloc space. +config CLEAR_STACK_FORK + bool "Clear the kernel stack at each fork" + help + To resist stack content leak flaws, this clears newly allocated + kernel stacks to keep previously freed heap or stack contents + from being present in the new stack. This has almost no + measurable performance impact. +
It would be much nicer to be able to control this at runtime rather than compile-time. Why not a /proc tunable? We could always use more of those ;)
Quoted 29 lines · Andrew Morton
Quoted 27 lines · On Tue, 16 Jan 2018 21:50:15 -0800 Kees Cook <[email protected]> wrote:
One of the classes of kernel stack content leaks is exposing the contents of prior heap or stack contents when a new process stack is allocated. Normally, those stacks are not zeroed, and the old contents remain in place. With some types of stack content exposure flaws, those contents can leak to userspace. Kernels built with
CONFIG_CLEAR_STACK_FORKwill no longer be vulnerable to this, as the stack will be wiped each time a stack is assigned to a new process. There's not a meaningful change in runtime performance; it almost looks like it provides a benefit.Performing back-to-back kernel builds before:
Run times: 157.86 157.09 158.90 160.94 160.80 Mean: 159.12 Std Dev: 1.54With
CONFIG_CLEAR_STACK_FORK=y:Run times: 159.31 157.34 156.71 158.15 160.81 Mean: 158.46 Std Dev: 1.46...
arch/Kconfig +0
-0@@ -904,6 +904,14 @@ config VMAP_STACK the stack to map directly to the KASAN shadow map using a formula that is incorrect if the stack is in vmalloc space.
Quoted 12 lines
arch/Kconfig config VMAP_STACK · line 914
Quoted 8 lines
arch/Kconfig +8
-0+config CLEAR_STACK_FORK + bool "Clear the kernel stack at each fork" + help + To resist stack content leak flaws, this clears newly allocated + kernel stacks to keep previously freed heap or stack contents + from being present in the new stack. This has almost no + measurable performance impact. +It would be much nicer to be able to control this at runtime rather than compile-time. Why not a /proc tunable? We could always use more of those ;)
/proc/sys/kernel/hardening_features_that_cost_essentially_nothing?
Seriously, though, why don't we just enable it unconditionally? It wouldn't surprise me if it really is a speedup on more workloads than it slows down -- it'll fill the kernel stack into the CPU cache with exclusive ownership very quickly (streamily and without actually reading from memory, I imagine, at least on new enough CPUs) rather than grabbing each cache line one by one as they get used.
Quoted 4 lines · Andrew Lutomirski
Quoted 3 lines
It would be much nicer to be able to control this at runtime rather than compile-time. Why not a /proc tunable? We could always use more of those ;)
Quoted 8 lines
/proc/sys/kernel/hardening_features_that_cost_essentially_nothing?Seriously, though, why don't we just enable it unconditionally? It wouldn't surprise me if it really is a speedup on more workloads than it slows down -- it'll fill the kernel stack into the CPU cache with exclusive ownership very quickly (streamily and without actually reading from memory, I imagine, at least on new enough CPUs) rather than grabbing each cache line one by one as they get used.
Note that this is not unconditionally true, it depends on the calling context that clears the page. If this is during fork, then the parent may be doing the clear (I didn't check) which means it's quite likely when the child wakes for the first time that it will not necessary wake on the same CPU. Up until recently on NUMA machines, the child was almost guaranteed to be running on a remote node (mitigated in tip for sched now).
I'm not claiming I've measured the overhead of this, just pointing out that "cache hotness" may actually result in double the cache line bounces -- first clear, then write on first wake. If only zeroing pages was a bit faster :/
Signature · 2 lines
Mel Gorman SUSE Labs