Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1349377 > unrolled thread

[PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to kernel

Back to article view | Back to linux.kernel

Contents

  [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to kernel Mark Rutland <mark.rutland@arm.com> - 2016-03-03 18:00 +0100
    [PATCHv2 1/3] kasan: add functions to clear stack poison Mark Rutland <mark.rutland@arm.com> - 2016-03-03 18:00 +0100
    Re: [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to kernel Alexander Potapenko <glider@google.com> - 2016-03-03 18:20 +0100
      Re: [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to kernel Alexander Potapenko <glider@google.com> - 2016-03-03 18:50 +0100
        Re: [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to  kernel Mark Rutland <mark.rutland@arm.com> - 2016-03-03 19:20 +0100
      Re: [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to  kernel Mark Rutland <mark.rutland@arm.com> - 2016-03-03 18:50 +0100

#1349377 — [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to kernel

Functions which the compiler has instrumented for ASAN place poison on
the stack shadow upon entry and remove this poison prior to returning.

In some cases (e.g. hotplug and idle), CPUs may exit the kernel a number
of levels deep in C code. If there are any instrumented functions on
this critical path, these will leave portions of the idle thread stack
shadow poisoned.

If a CPU returns to the kernel via a different path (e.g. a cold entry),
then depending on stack frame layout subsequent calls to instrumented
functions may use regions of the stack with stale poison, resulting in
(spurious) KASAN splats to the console.

Contemporary GCCs always add stack shadow poisoning when ASAN is
enabled, even when asked to not instrument a function [1], so we can't
simply annotate functions on the critical path to avoid poisoning.

Instead, this series explicitly removes any stale poison before it can
be hit. In the common hotplug case we clear the entire stack shadow in
common code, before a CPU is brought online.

On architectures which perform a cold return as part of cpu idle may
retain an architecture-specific amount of stack contents. To retain the
poison for this retained context, the arch code must call the core KASAN
code, passing a "watermark" stack pointer value beyond which shadow will
be cleared. Architectures which don't perform a cold return as part of
idle do not need any additional code.

This is a combination of previous approaches [2,3], attempting to keep
as much as possible generic.

Since v1 [4]:
* Clean from task_stack_page(task)
* Add acks from v1

Andrew, the conclusion [5] from v1 was that this should go via the mm tree.
Are you happy to pick this up? 

Ingo was happy for the sched patch to go via the arm64 tree, and I assume that
also holds for going via mm. Ingo, please shout if that's not the case!

Thanks,
Mark.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69863
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/409466.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/411850.html
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/413093.html
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/413475.html

Mark Rutland (3):
  kasan: add functions to clear stack poison
  sched/kasan: remove stale KASAN poison after hotplug
  arm64: kasan: clear stale stack poison

 arch/arm64/kernel/sleep.S |  4 ++++
 include/linux/kasan.h     |  6 +++++-
 kernel/sched/core.c       |  3 +++
 mm/kasan/kasan.c          | 20 ++++++++++++++++++++
 4 files changed, 32 insertions(+), 1 deletion(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1349381 — [PATCHv2 1/3] kasan: add functions to clear stack poison

Functions which the compiler has instrumented for ASAN place poison on
the stack shadow upon entry and remove this poision prior to returning.

In some cases (e.g. hotplug and idle), CPUs may exit the kernel a number
of levels deep in C code. If there are any instrumented functions on
this critical path, these will leave portions of the stack shadow
poisoned.

If a CPU returns to the kernel via a different path (e.g. a cold entry),
then depending on stack frame layout subsequent calls to instrumented
functions may use regions of the stack with stale poison, resulting in
(spurious) KASAN splats to the console.

To avoid this, we must clear stale poison from the stack prior to
instrumented functions being called. This patch adds functions to the
KASAN core for removing poison from (portions of) a task's stack. These
will be used by subsequent patches to avoid problems with hotplug and
idle.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 include/linux/kasan.h |  6 +++++-
 mm/kasan/kasan.c      | 20 ++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 4b9f85c..0fdc798 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -1,6 +1,7 @@
 #ifndef _LINUX_KASAN_H
 #define _LINUX_KASAN_H
 
+#include <linux/sched.h>
 #include <linux/types.h>
 
 struct kmem_cache;
@@ -13,7 +14,6 @@ struct vm_struct;
 
 #include <asm/kasan.h>
 #include <asm/pgtable.h>
-#include <linux/sched.h>
 
 extern unsigned char kasan_zero_page[PAGE_SIZE];
 extern pte_t kasan_zero_pte[PTRS_PER_PTE];
@@ -43,6 +43,8 @@ static inline void kasan_disable_current(void)
 
 void kasan_unpoison_shadow(const void *address, size_t size);
 
+void kasan_unpoison_task_stack(struct task_struct *task);
+
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 
@@ -66,6 +68,8 @@ void kasan_free_shadow(const struct vm_struct *vm);
 
 static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
 
+static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
+
 static inline void kasan_enable_current(void) {}
 static inline void kasan_disable_current(void) {}
 
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index bc0a8d8..1ad20ad 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/kmemleak.h>
+#include <linux/linkage.h>
 #include <linux/memblock.h>
 #include <linux/memory.h>
 #include <linux/mm.h>
@@ -60,6 +61,25 @@ void kasan_unpoison_shadow(const void *address, size_t size)
 	}
 }
 
+static void __kasan_unpoison_stack(struct task_struct *task, void *sp)
+{
+	void *base = task_stack_page(task);
+	size_t size = sp - base;
+
+	kasan_unpoison_shadow(base, size);
+}
+
+/* Unpoison the entire stack for a task. */
+void kasan_unpoison_task_stack(struct task_struct *task)
+{
+	__kasan_unpoison_stack(task, task_stack_page(task) + THREAD_SIZE);
+}
+
+/* Unpoison the stack for the current task beyond a watermark sp value. */
+asmlinkage void kasan_unpoison_remaining_stack(void *sp)
+{
+	__kasan_unpoison_stack(current, sp);
+}
 
 /*
  * All functions below always inlined so compiler could
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1349417

Please replace "ASAN" with "KASAN".

On Thu, Mar 3, 2016 at 5:54 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> Functions which the compiler has instrumented for ASAN place poison on
> the stack shadow upon entry and remove this poison prior to returning.
>
> In some cases (e.g. hotplug and idle), CPUs may exit the kernel a number
> of levels deep in C code. If there are any instrumented functions on
> this critical path, these will leave portions of the idle thread stack
> shadow poisoned.
>
> If a CPU returns to the kernel via a different path (e.g. a cold entry),
> then depending on stack frame layout subsequent calls to instrumented
> functions may use regions of the stack with stale poison, resulting in
> (spurious) KASAN splats to the console.
>
> Contemporary GCCs always add stack shadow poisoning when ASAN is
> enabled, even when asked to not instrument a function [1], so we can't
> simply annotate functions on the critical path to avoid poisoning.
>
> Instead, this series explicitly removes any stale poison before it can
> be hit. In the common hotplug case we clear the entire stack shadow in
> common code, before a CPU is brought online.
>
> On architectures which perform a cold return as part of cpu idle may
> retain an architecture-specific amount of stack contents. To retain the
> poison for this retained context, the arch code must call the core KASAN
> code, passing a "watermark" stack pointer value beyond which shadow will
> be cleared. Architectures which don't perform a cold return as part of
> idle do not need any additional code.
>
> This is a combination of previous approaches [2,3], attempting to keep
> as much as possible generic.
>
> Since v1 [4]:
> * Clean from task_stack_page(task)
> * Add acks from v1
>
> Andrew, the conclusion [5] from v1 was that this should go via the mm tree.
> Are you happy to pick this up?
>
> Ingo was happy for the sched patch to go via the arm64 tree, and I assume that
> also holds for going via mm. Ingo, please shout if that's not the case!
>
> Thanks,
> Mark.
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69863
> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/409466.html
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/411850.html
> [4] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/413093.html
> [5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/413475.html
>
> Mark Rutland (3):
>   kasan: add functions to clear stack poison
>   sched/kasan: remove stale KASAN poison after hotplug
>   arm64: kasan: clear stale stack poison
>
>  arch/arm64/kernel/sleep.S |  4 ++++
>  include/linux/kasan.h     |  6 +++++-
>  kernel/sched/core.c       |  3 +++
>  mm/kasan/kasan.c          | 20 ++++++++++++++++++++
>  4 files changed, 32 insertions(+), 1 deletion(-)
>
> --
> 1.9.1
>



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den
Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
This e-mail is confidential. If you are not the right addressee please
do not forward it, please inform the sender, and please erase this
e-mail including any attachments. Thanks.

[toc] | [prev] | [next] | [standalone]

#1349448

On Thu, Mar 3, 2016 at 6:40 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> On Thu, Mar 03, 2016 at 06:17:31PM +0100, Alexander Potapenko wrote:
>> Please replace "ASAN" with "KASAN".
>>
>> On Thu, Mar 3, 2016 at 5:54 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > Functions which the compiler has instrumented for ASAN place poison on
>> > the stack shadow upon entry and remove this poison prior to returning.
>> >
>> > In some cases (e.g. hotplug and idle), CPUs may exit the kernel a number
>> > of levels deep in C code. If there are any instrumented functions on
>> > this critical path, these will leave portions of the idle thread stack
>> > shadow poisoned.
>> >
>> > If a CPU returns to the kernel via a different path (e.g. a cold entry),
>> > then depending on stack frame layout subsequent calls to instrumented
>> > functions may use regions of the stack with stale poison, resulting in
>> > (spurious) KASAN splats to the console.
>> >
>> > Contemporary GCCs always add stack shadow poisoning when ASAN is
>> > enabled, even when asked to not instrument a function [1], so we can't
>> > simply annotate functions on the critical path to avoid poisoning.
>> >
>> > Instead, this series explicitly removes any stale poison before it can
>> > be hit. In the common hotplug case we clear the entire stack shadow in
>> > common code, before a CPU is brought online.
>> >
>> > On architectures which perform a cold return as part of cpu idle may
>> > retain an architecture-specific amount of stack contents. To retain the
>> > poison for this retained context, the arch code must call the core KASAN
>> > code, passing a "watermark" stack pointer value beyond which shadow will
>> > be cleared. Architectures which don't perform a cold return as part of
>> > idle do not need any additional code.
>
> For the above, and the rest of the series, ASAN consistently refers to
> the compiler AddressSanitizer feature, and KASAN consistently refers to
> the Linux-specific infrastructure. A simple s/[^K]ASAN/KASAN/ would
> arguably be wrong (e.g. when referring to GCC behaviour above).
I don't think there's been any convention about the compiler feature
name, we usually talked about ASan as a userspace tool and KASAN as a
kernel-space one, although they share the compiler part.

> If there is a this needs rework, then I'm happy to s/[^K]ASAN/ASan/ to
> follow the usual ASan naming convention and avoid confusion. Otherwise,
> spinning a v3 is simply churn.
I don't insist on changing this, I should've chimed in before.
Feel free to retain the above patch description.
> Thanks,
> Mark.



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den
Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
This e-mail is confidential. If you are not the right addressee please
do not forward it, please inform the sender, and please erase this
e-mail including any attachments. Thanks.

[toc] | [prev] | [next] | [standalone]

#1349468 — Re: [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to kernel

On Thu, Mar 03, 2016 at 06:45:55PM +0100, Alexander Potapenko wrote:
> On Thu, Mar 3, 2016 at 6:40 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Hi,
> >
> > On Thu, Mar 03, 2016 at 06:17:31PM +0100, Alexander Potapenko wrote:
> >> Please replace "ASAN" with "KASAN".
> >>
> >> On Thu, Mar 3, 2016 at 5:54 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > Functions which the compiler has instrumented for ASAN place poison on
> >> > the stack shadow upon entry and remove this poison prior to returning.

[...]

> > For the above, and the rest of the series, ASAN consistently refers to
> > the compiler AddressSanitizer feature, and KASAN consistently refers to
> > the Linux-specific infrastructure. A simple s/[^K]ASAN/KASAN/ would
> > arguably be wrong (e.g. when referring to GCC behaviour above).
> I don't think there's been any convention about the compiler feature
> name, we usually talked about ASan as a userspace tool and KASAN as a
> kernel-space one, although they share the compiler part.

Ah, ok.

In future I'll speak in terms of "AddressSanitizer instrumentation" or
something like that, as that's fairly unambigious.

> > If there is a this needs rework, then I'm happy to s/[^K]ASAN/ASan/ to
> > follow the usual ASan naming convention and avoid confusion. Otherwise,
> > spinning a v3 is simply churn.
> I don't insist on changing this, I should've chimed in before.
> Feel free to retain the above patch description.

No worries, thanks for the info.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1349450 — Re: [PATCHv2 0/3] KASAN: clean stale poison upon cold re-entry to kernel

Hi,

On Thu, Mar 03, 2016 at 06:17:31PM +0100, Alexander Potapenko wrote:
> Please replace "ASAN" with "KASAN".
> 
> On Thu, Mar 3, 2016 at 5:54 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Functions which the compiler has instrumented for ASAN place poison on
> > the stack shadow upon entry and remove this poison prior to returning.
> >
> > In some cases (e.g. hotplug and idle), CPUs may exit the kernel a number
> > of levels deep in C code. If there are any instrumented functions on
> > this critical path, these will leave portions of the idle thread stack
> > shadow poisoned.
> >
> > If a CPU returns to the kernel via a different path (e.g. a cold entry),
> > then depending on stack frame layout subsequent calls to instrumented
> > functions may use regions of the stack with stale poison, resulting in
> > (spurious) KASAN splats to the console.
> >
> > Contemporary GCCs always add stack shadow poisoning when ASAN is
> > enabled, even when asked to not instrument a function [1], so we can't
> > simply annotate functions on the critical path to avoid poisoning.
> >
> > Instead, this series explicitly removes any stale poison before it can
> > be hit. In the common hotplug case we clear the entire stack shadow in
> > common code, before a CPU is brought online.
> >
> > On architectures which perform a cold return as part of cpu idle may
> > retain an architecture-specific amount of stack contents. To retain the
> > poison for this retained context, the arch code must call the core KASAN
> > code, passing a "watermark" stack pointer value beyond which shadow will
> > be cleared. Architectures which don't perform a cold return as part of
> > idle do not need any additional code.

For the above, and the rest of the series, ASAN consistently refers to
the compiler AddressSanitizer feature, and KASAN consistently refers to
the Linux-specific infrastructure. A simple s/[^K]ASAN/KASAN/ would
arguably be wrong (e.g. when referring to GCC behaviour above).

If there is a this needs rework, then I'm happy to s/[^K]ASAN/ASan/ to
follow the usual ASan naming convention and avoid confusion. Otherwise,
spinning a v3 is simply churn.

Thanks,
Mark.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web