linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: andrey.konovalov@linux.dev
To: Marco Elver <elver@google.com>,
	Alexander Potapenko <glider@google.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <ryabinin.a.a@gmail.com>,
	kasan-dev@googlegroups.com,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Peter Collingbourne <pcc@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrey Konovalov <andreyknvl@google.com>
Subject: [PATCH] kasan, scs: collect stack traces from shadow stack
Date: Sat, 12 Mar 2022 21:14:06 +0100	[thread overview]
Message-ID: <57133fafc4d74377a4a08d98e276d58fe4a127dc.1647115974.git.andreyknvl@google.com> (raw)

From: Andrey Konovalov <andreyknvl@google.com>

Currently, KASAN always uses the normal stack trace collection routines,
which rely on the unwinder, when saving alloc and free stack traces.

Instead of invoking the unwinder, collect the stack trace by copying
frames from the Shadow Call Stack whenever it is enabled. This reduces
boot time by 30% for all KASAN modes when Shadow Call Stack is enabled.

To avoid potentially leaking PAC pointer tags, strip them when saving
the stack trace.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>

---

Things to consider:

We could integrate shadow stack trace collection into kernel/stacktrace.c
as e.g. stack_trace_save_shadow(). However, using stack_trace_consume_fn
leads to invoking a callback on each saved from, which is undesirable.
The plain copy loop is faster.

We could add a command line flag to switch between stack trace collection
modes. I noticed that Shadow Call Stack might be missing certain frames
in stacks originating from a fault that happens in the middle of a
function. I am not sure if this case is important to handle though.

Looking forward to thoughts and comments.

Thanks!

---
 mm/kasan/common.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index d9079ec11f31..65a0723370c7 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -9,6 +9,7 @@
  *        Andrey Konovalov <andreyknvl@gmail.com>
  */
 
+#include <linux/bits.h>
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/kasan.h>
@@ -21,6 +22,7 @@
 #include <linux/printk.h>
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
+#include <linux/scs.h>
 #include <linux/slab.h>
 #include <linux/stacktrace.h>
 #include <linux/string.h>
@@ -30,12 +32,44 @@
 #include "kasan.h"
 #include "../slab.h"
 
+#ifdef CONFIG_SHADOW_CALL_STACK
+
+#ifdef CONFIG_ARM64_PTR_AUTH
+#define PAC_TAG_RESET(x) (x | GENMASK(63, CONFIG_ARM64_VA_BITS))
+#else
+#define PAC_TAG_RESET(x) (x)
+#endif
+
+static unsigned int save_shadow_stack(unsigned long *entries,
+				      unsigned int nr_entries)
+{
+	unsigned long *scs_sp = task_scs_sp(current);
+	unsigned long *scs_base = task_scs(current);
+	unsigned long *frame;
+	unsigned int i = 0;
+
+	for (frame = scs_sp - 1; frame >= scs_base; frame--) {
+		entries[i++] = PAC_TAG_RESET(*frame);
+		if (i >= nr_entries)
+			break;
+	}
+
+	return i;
+}
+#else /* CONFIG_SHADOW_CALL_STACK */
+static inline unsigned int save_shadow_stack(unsigned long *entries,
+					unsigned int nr_entries) { return 0; }
+#endif /* CONFIG_SHADOW_CALL_STACK */
+
 depot_stack_handle_t kasan_save_stack(gfp_t flags, bool can_alloc)
 {
 	unsigned long entries[KASAN_STACK_DEPTH];
 	unsigned int nr_entries;
 
-	nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0);
+	if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK))
+		nr_entries = save_shadow_stack(entries, ARRAY_SIZE(entries));
+	else
+		nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0);
 	return __stack_depot_save(entries, nr_entries, flags, can_alloc);
 }
 
-- 
2.25.1



             reply	other threads:[~2022-03-12 20:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-12 20:14 andrey.konovalov [this message]
2022-03-13 16:59 ` Andrey Konovalov
2022-03-13 23:44 ` Andrey Konovalov
2022-03-14  8:57   ` Marco Elver
2022-03-20 21:12     ` Andrey Konovalov
2022-03-14  7:00 ` Marco Elver
2022-03-20 21:09   ` Andrey Konovalov
2022-03-14  7:17 ` Dmitry Vyukov
2022-03-20 21:09   ` Andrey Konovalov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57133fafc4d74377a4a08d98e276d58fe4a127dc.1647115974.git.andreyknvl@google.com \
    --to=andrey.konovalov@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=andreyknvl@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=eugenis@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mark.rutland@arm.com \
    --cc=pcc@google.com \
    --cc=ryabinin.a.a@gmail.com \
    --cc=samitolvanen@google.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox