On 8/2/18 3:22 PM, Kees Cook wrote: > On Thu, Aug 2, 2018 at 12:22 PM, Srivatsa S. Bhat > <srivatsa@xxxxxxxxxxxxx> wrote: >> On 7/26/18 4:09 PM, Kees Cook wrote: >>> On Tue, Jul 24, 2018 at 3:02 PM, Jiri Kosina <jikos@xxxxxxxxxx> wrote: >>>> On Tue, 24 Jul 2018, Srivatsa S. Bhat wrote: >>>> >>>>> However, if you are proposing that you'd like to contribute the enhanced >>>>> PTI/Spectre (upstream) patches from the SLES 4.4 tree to 4.4 stable, and >>>>> have them merged instead of this patch series, then I would certainly >>>>> welcome it! >>>> >>>> I'd in principle love us to push everything back to 4.4, but there are a >>>> few reasons (*) why that's not happening shortly. >>>> >>>> Anyway, to point out explicitly what's really needed for those folks >>>> running 4.4-stable and relying on PTI providing The Real Thing(TM), it's >>>> either a 4.4-stable port of >>>> >>>> http://kernel.suse.com/cgit/kernel-source/plain/patches.suse/x86-entry-64-use-a-per-cpu-trampoline-stack.patch?id=3428a77b02b1ba03e45d8fc352ec350429f57fc7 >>>> >>>> or making THREADINFO_GFP imply __GFP_ZERO. >>> >>> This is true in Linus's tree now. Should be trivial to backport: >>> https://git.kernel.org/linus/e01e80634ecdd >>> >> >> Hi Jiri, Kees, >> >> Thank you for suggesting the patch! I have attached the (locally >> tested) 4.4 and 4.9 backports of that patch with this mail. (The >> mainline commit applies cleanly on 4.14). >> >> Greg, could you please consider including them in stable 4.4, 4.9 >> and 4.14? > > I don't think your v4.9 is sufficient: it leaves the vmapped stack > uncleared. v4.9 needs ca182551857 ("kmemleak: clear stale pointers > from task stacks") included in the backport (really, just adding the > memset()). > Ah, I see, thank you! I have attached the updated patchset for 4.9 with this mail. > Otherwise, yup, looks good. > Thank you for reviewing the patches! Regards, Srivatsa VMware Photon OS
From edf835d8b6bac08bc5e69efb3e1cc321e2457f61 Mon Sep 17 00:00:00 2001 From: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx> Date: Fri, 13 Oct 2017 15:58:22 -0700 Subject: [PATCH 1/2] kmemleak: clear stale pointers from task stacks commit ca182551857cc2c1e6a2b7f1e72090a137a15008 upstream. Kmemleak considers any pointers on task stacks as references. This patch clears newly allocated and reused vmap stacks. Link: http://lkml.kernel.org/r/150728990124.744199.8403409836394318684.stgit@buzz Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx> Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> [ Srivatsa: Backported to 4.9.y ] Signed-off-by: Srivatsa S. Bhat <srivatsa@xxxxxxxxxxxxx> --- include/linux/thread_info.h | 2 +- kernel/fork.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h index 2873baf..cf87c16 100644 --- a/include/linux/thread_info.h +++ b/include/linux/thread_info.h @@ -59,7 +59,7 @@ extern long do_no_restart_syscall(struct restart_block *parm); #ifdef __KERNEL__ -#ifdef CONFIG_DEBUG_STACK_USAGE +#if IS_ENABLED(CONFIG_DEBUG_STACK_USAGE) || IS_ENABLED(CONFIG_DEBUG_KMEMLEAK) # define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_NOTRACK | \ __GFP_ZERO) #else diff --git a/kernel/fork.c b/kernel/fork.c index 70e10cb..c19e6d4 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -184,6 +184,10 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) continue; this_cpu_write(cached_stacks[i], NULL); +#ifdef CONFIG_DEBUG_KMEMLEAK + /* Clear stale pointers from reused stack. */ + memset(s->addr, 0, THREAD_SIZE); +#endif tsk->stack_vm_area = s; local_irq_enable(); return s->addr; -- 2.7.4
From 5371cd0bb1e2ca8d1603845c764e1524f7e729ad Mon Sep 17 00:00:00 2001 From: Kees Cook <keescook@xxxxxxxxxxxx> Date: Fri, 20 Apr 2018 14:55:31 -0700 Subject: [PATCH 2/2] fork: unconditionally clear stack on fork commit e01e80634ecdde1dd113ac43b3adad21b47f3957 upstream. One of the classes of kernel stack content leaks[1] 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. In the face of stack content exposure flaws, those contents can leak to userspace. Fixing this will make the kernel no longer vulnerable to these flaws, 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 and after: Run times: 159.31 157.34 156.71 158.15 160.81 Mean: 158.46 Std Dev: 1.46 Instead of making this a build or runtime config, Andy Lutomirski recommended this just be enabled by default. [1] A noisy search for many kinds of stack content leaks can be seen here: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=linux+kernel+stack+leak I did some more with perf and cycle counts on running 100,000 execs of /bin/true. before: Cycles: 218858861551 218853036130 214727610969 227656844122 224980542841 Mean: 221015379122.60 Std Dev: 4662486552.47 after: Cycles: 213868945060 213119275204 211820169456 224426673259 225489986348 Mean: 217745009865.40 Std Dev: 5935559279.99 It continues to look like it's faster, though the deviation is rather wide, but I'm not sure what I could do that would be less noisy. I'm open to ideas! Link: http://lkml.kernel.org/r/20180221021659.GA37073@beast Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> Acked-by: Michal Hocko <mhocko@xxxxxxxx> Reviewed-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Andy Lutomirski <luto@xxxxxxxxxx> Cc: Laura Abbott <labbott@xxxxxxxxxx> Cc: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> [ Srivatsa: Backported to 4.9.y ] Signed-off-by: Srivatsa S. Bhat <srivatsa@xxxxxxxxxxxxx> Reviewed-by: Srinidhi Rao <srinidhir@xxxxxxxxxx> --- include/linux/thread_info.h | 7 +------ kernel/fork.c | 3 +-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h index cf87c16..5e64367 100644 --- a/include/linux/thread_info.h +++ b/include/linux/thread_info.h @@ -59,12 +59,7 @@ extern long do_no_restart_syscall(struct restart_block *parm); #ifdef __KERNEL__ -#if IS_ENABLED(CONFIG_DEBUG_STACK_USAGE) || IS_ENABLED(CONFIG_DEBUG_KMEMLEAK) -# define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_NOTRACK | \ - __GFP_ZERO) -#else -# define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_NOTRACK) -#endif +#define THREADINFO_GFP (GFP_KERNEL_ACCOUNT | __GFP_NOTRACK | __GFP_ZERO) /* * flag set/clear/test wrappers diff --git a/kernel/fork.c b/kernel/fork.c index c19e6d4..2c98b98 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -184,10 +184,9 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) continue; this_cpu_write(cached_stacks[i], NULL); -#ifdef CONFIG_DEBUG_KMEMLEAK /* Clear stale pointers from reused stack. */ memset(s->addr, 0, THREAD_SIZE); -#endif + tsk->stack_vm_area = s; local_irq_enable(); return s->addr; -- 2.7.4
