linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument
@ 2020-12-03  5:12 Peter Collingbourne
  2020-12-03  5:12 ` [PATCH v3 2/3] arm64: stacktrace: Relax frame record alignment requirement to 8 bytes Peter Collingbourne
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Peter Collingbourne @ 2020-12-03  5:12 UTC (permalink / raw)
  To: Mark Brown, Mark Rutland, Will Deacon, Catalin Marinas,
	Andrey Konovalov, Evgenii Stepanov, Ard Biesheuvel
  Cc: Peter Collingbourne, Linux ARM

unwind_frame() was previously implicitly checking that the frame
record is in bounds of the stack by enforcing that FP is both aligned
to 16 and in bounds of the stack. Once the FP alignment requirement
is relaxed to 8 this will not be sufficient because it does not
account for the case where FP points to 8 bytes before the end of the
stack.

Make the check explicit by changing the on_*stack functions to take a
size argument and adjusting the callers to pass the appropriate sizes.

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/Ib7a3eb3eea41b0687ffaba045ceb2012d077d8b4
---
 arch/arm64/include/asm/processor.h  | 12 +++++------
 arch/arm64/include/asm/sdei.h       |  7 ++++---
 arch/arm64/include/asm/stacktrace.h | 32 ++++++++++++++---------------
 arch/arm64/kernel/ptrace.c          |  2 +-
 arch/arm64/kernel/sdei.c            | 16 ++++++++-------
 arch/arm64/kernel/stacktrace.c      |  2 +-
 6 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index fce8cbecd6bc..bf3e07f67e79 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -318,13 +318,13 @@ long get_tagged_addr_ctrl(struct task_struct *task);
  * of header definitions for the use of task_stack_page.
  */
 
-#define current_top_of_stack()							\
-({										\
-	struct stack_info _info;						\
-	BUG_ON(!on_accessible_stack(current, current_stack_pointer, &_info));	\
-	_info.high;								\
+#define current_top_of_stack()								\
+({											\
+	struct stack_info _info;							\
+	BUG_ON(!on_accessible_stack(current, current_stack_pointer, 1, &_info));	\
+	_info.high;									\
 })
-#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, NULL))
+#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, 1, NULL))
 
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/asm/sdei.h b/arch/arm64/include/asm/sdei.h
index 63e0b92a5fbb..8bc30a5c4569 100644
--- a/arch/arm64/include/asm/sdei.h
+++ b/arch/arm64/include/asm/sdei.h
@@ -42,8 +42,9 @@ unsigned long sdei_arch_get_entry_point(int conduit);
 
 struct stack_info;
 
-bool _on_sdei_stack(unsigned long sp, struct stack_info *info);
-static inline bool on_sdei_stack(unsigned long sp,
+bool _on_sdei_stack(unsigned long sp, unsigned long size,
+		    struct stack_info *info);
+static inline bool on_sdei_stack(unsigned long sp, unsigned long size,
 				struct stack_info *info)
 {
 	if (!IS_ENABLED(CONFIG_VMAP_STACK))
@@ -51,7 +52,7 @@ static inline bool on_sdei_stack(unsigned long sp,
 	if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
 		return false;
 	if (in_nmi())
-		return _on_sdei_stack(sp, info);
+		return _on_sdei_stack(sp, size, info);
 
 	return false;
 }
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index eb29b1fe8255..77192df232bf 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -69,14 +69,14 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
 
 DECLARE_PER_CPU(unsigned long *, irq_stack_ptr);
 
-static inline bool on_stack(unsigned long sp, unsigned long low,
-				unsigned long high, enum stack_type type,
-				struct stack_info *info)
+static inline bool on_stack(unsigned long sp, unsigned long size,
+			    unsigned long low, unsigned long high,
+			    enum stack_type type, struct stack_info *info)
 {
 	if (!low)
 		return false;
 
-	if (sp < low || sp >= high)
+	if (sp < low || sp + size < sp || sp + size > high)
 		return false;
 
 	if (info) {
@@ -87,38 +87,38 @@ static inline bool on_stack(unsigned long sp, unsigned long low,
 	return true;
 }
 
-static inline bool on_irq_stack(unsigned long sp,
+static inline bool on_irq_stack(unsigned long sp, unsigned long size,
 				struct stack_info *info)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
 	unsigned long high = low + IRQ_STACK_SIZE;
 
-	return on_stack(sp, low, high, STACK_TYPE_IRQ, info);
+	return on_stack(sp, size, low, high, STACK_TYPE_IRQ, info);
 }
 
 static inline bool on_task_stack(const struct task_struct *tsk,
-				 unsigned long sp,
+				 unsigned long sp, unsigned long size,
 				 struct stack_info *info)
 {
 	unsigned long low = (unsigned long)task_stack_page(tsk);
 	unsigned long high = low + THREAD_SIZE;
 
-	return on_stack(sp, low, high, STACK_TYPE_TASK, info);
+	return on_stack(sp, size, low, high, STACK_TYPE_TASK, info);
 }
 
 #ifdef CONFIG_VMAP_STACK
 DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack);
 
-static inline bool on_overflow_stack(unsigned long sp,
+static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
 				struct stack_info *info)
 {
 	unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack);
 	unsigned long high = low + OVERFLOW_STACK_SIZE;
 
-	return on_stack(sp, low, high, STACK_TYPE_OVERFLOW, info);
+	return on_stack(sp, size, low, high, STACK_TYPE_OVERFLOW, info);
 }
 #else
-static inline bool on_overflow_stack(unsigned long sp,
+static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
 			struct stack_info *info) { return false; }
 #endif
 
@@ -128,21 +128,21 @@ static inline bool on_overflow_stack(unsigned long sp,
  * context.
  */
 static inline bool on_accessible_stack(const struct task_struct *tsk,
-				       unsigned long sp,
+				       unsigned long sp, unsigned long size,
 				       struct stack_info *info)
 {
 	if (info)
 		info->type = STACK_TYPE_UNKNOWN;
 
-	if (on_task_stack(tsk, sp, info))
+	if (on_task_stack(tsk, sp, size, info))
 		return true;
 	if (tsk != current || preemptible())
 		return false;
-	if (on_irq_stack(sp, info))
+	if (on_irq_stack(sp, size, info))
 		return true;
-	if (on_overflow_stack(sp, info))
+	if (on_overflow_stack(sp, size, info))
 		return true;
-	if (on_sdei_stack(sp, info))
+	if (on_sdei_stack(sp, size, info))
 		return true;
 
 	return false;
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index f49b349e16a3..b44d6981decb 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -122,7 +122,7 @@ static bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr)
 {
 	return ((addr & ~(THREAD_SIZE - 1))  ==
 		(kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1))) ||
-		on_irq_stack(addr, NULL);
+		on_irq_stack(addr, sizeof(unsigned long), NULL);
 }
 
 /**
diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index 793c46d6a447..ee8a13ce239b 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -91,31 +91,33 @@ static int init_sdei_stacks(void)
 	return err;
 }
 
-static bool on_sdei_normal_stack(unsigned long sp, struct stack_info *info)
+static bool on_sdei_normal_stack(unsigned long sp, unsigned long size,
+				 struct stack_info *info)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
 	unsigned long high = low + SDEI_STACK_SIZE;
 
-	return on_stack(sp, low, high, STACK_TYPE_SDEI_NORMAL, info);
+	return on_stack(sp, size, low, high, STACK_TYPE_SDEI_NORMAL, info);
 }
 
-static bool on_sdei_critical_stack(unsigned long sp, struct stack_info *info)
+static bool on_sdei_critical_stack(unsigned long sp, unsigned long size,
+				   struct stack_info *info)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
 	unsigned long high = low + SDEI_STACK_SIZE;
 
-	return on_stack(sp, low, high, STACK_TYPE_SDEI_CRITICAL, info);
+	return on_stack(sp, size, low, high, STACK_TYPE_SDEI_CRITICAL, info);
 }
 
-bool _on_sdei_stack(unsigned long sp, struct stack_info *info)
+bool _on_sdei_stack(unsigned long sp, unsigned long size, struct stack_info *info)
 {
 	if (!IS_ENABLED(CONFIG_VMAP_STACK))
 		return false;
 
-	if (on_sdei_critical_stack(sp, info))
+	if (on_sdei_critical_stack(sp, size, info))
 		return true;
 
-	if (on_sdei_normal_stack(sp, info))
+	if (on_sdei_normal_stack(sp, size, info))
 		return true;
 
 	return false;
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index fa56af1a59c3..ce613e22b58b 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -50,7 +50,7 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
 	if (!tsk)
 		tsk = current;
 
-	if (!on_accessible_stack(tsk, fp, &info))
+	if (!on_accessible_stack(tsk, fp, 16, &info))
 		return -EINVAL;
 
 	if (test_bit(info.type, frame->stacks_done))
-- 
2.29.2.454.gaff20da3a2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] arm64: stacktrace: Relax frame record alignment requirement to 8 bytes
  2020-12-03  5:12 [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument Peter Collingbourne
@ 2020-12-03  5:12 ` Peter Collingbourne
  2020-12-03  5:12 ` [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks Peter Collingbourne
  2020-12-08 10:20 ` [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument Mark Rutland
  2 siblings, 0 replies; 8+ messages in thread
From: Peter Collingbourne @ 2020-12-03  5:12 UTC (permalink / raw)
  To: Mark Brown, Mark Rutland, Will Deacon, Catalin Marinas,
	Andrey Konovalov, Evgenii Stepanov, Ard Biesheuvel
  Cc: Peter Collingbourne, Linux ARM

The AAPCS places no requirements on the alignment of the frame
record. In theory it could be placed anywhere, although it seems
sensible to require it to be aligned to 8 bytes. With an upcoming
enhancement to tag-based KASAN Clang will begin creating frame records
located at an address that is only aligned to 8 bytes. Accommodate
such frame records in the stack unwinding code.

As pointed out by Mark Rutland, the userspace stack unwinding code
has the same problem, so fix it there as well.

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/Ia22c375230e67ca055e9e4bb639383567f7ad268
Acked-by: Andrey Konovalov <andreyknvl@google.com>
---
v2:
- fix it in the userspace unwinding code as well

 arch/arm64/kernel/perf_callchain.c | 2 +-
 arch/arm64/kernel/stacktrace.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/perf_callchain.c b/arch/arm64/kernel/perf_callchain.c
index 88ff471b0bce..4a72c2727309 100644
--- a/arch/arm64/kernel/perf_callchain.c
+++ b/arch/arm64/kernel/perf_callchain.c
@@ -116,7 +116,7 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
 		tail = (struct frame_tail __user *)regs->regs[29];
 
 		while (entry->nr < entry->max_stack &&
-		       tail && !((unsigned long)tail & 0xf))
+		       tail && !((unsigned long)tail & 0x7))
 			tail = user_backtrace(tail, entry);
 	} else {
 #ifdef CONFIG_COMPAT
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index ce613e22b58b..3bc1c44b7910 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -44,7 +44,7 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
 	unsigned long fp = frame->fp;
 	struct stack_info info;
 
-	if (fp & 0xf)
+	if (fp & 0x7)
 		return -EINVAL;
 
 	if (!tsk)
-- 
2.29.2.454.gaff20da3a2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks
  2020-12-03  5:12 [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument Peter Collingbourne
  2020-12-03  5:12 ` [PATCH v3 2/3] arm64: stacktrace: Relax frame record alignment requirement to 8 bytes Peter Collingbourne
@ 2020-12-03  5:12 ` Peter Collingbourne
  2020-12-08 17:49   ` Catalin Marinas
  2021-01-07 15:20   ` Andrey Konovalov
  2020-12-08 10:20 ` [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument Mark Rutland
  2 siblings, 2 replies; 8+ messages in thread
From: Peter Collingbourne @ 2020-12-03  5:12 UTC (permalink / raw)
  To: Mark Brown, Mark Rutland, Will Deacon, Catalin Marinas,
	Andrey Konovalov, Evgenii Stepanov, Ard Biesheuvel
  Cc: Peter Collingbourne, Linux ARM

By using outlined checks we can achieve a significant code size
improvement by moving the tag-based ASAN checks into separate
functions. Unlike the existing CONFIG_KASAN_OUTLINE mode these
functions have a custom calling convention that preserves most
registers and is specialized to the register containing the address
and the type of access, and as a result we can eliminate the code
size and performance overhead of a standard calling convention such
as AAPCS for these functions.

This change depends on a separate series of changes to Clang [1] to
support outlined checks in the kernel, although the change works fine
without them (we just don't get outlined checks). This is because the
flag -mllvm -hwasan-inline-all-checks=0 has no effect until the Clang
changes land. The flag was introduced in the Clang 9.0 timeframe as
part of the support for outlined checks in userspace and because our
minimum Clang version is 10.0 we can pass it unconditionally.

Outlined checks require a new runtime function with a custom calling
convention. Add this function to arch/arm64/lib.

I measured the code size of defconfig + tag-based KASAN, as well
as boot time (i.e. time to init launch) on a DragonBoard 845c with
an Android arm64 GKI kernel. The results are below:

                               code size    boot time
CONFIG_KASAN_INLINE=y before    92824064      6.18s
CONFIG_KASAN_INLINE=y after     38822400      6.65s
CONFIG_KASAN_OUTLINE=y          39215616     11.48s

We can see straight away that specialized outlined checks beat the
existing CONFIG_KASAN_OUTLINE=y on both code size and boot time
for tag-based ASAN.

As for the comparison between CONFIG_KASAN_INLINE=y before and after
we saw similar performance numbers in userspace [2] and decided
that since the performance overhead is minimal compared to the
overhead of tag-based ASAN itself as well as compared to the code
size improvements we would just replace the inlined checks with the
specialized outlined checks without the option to select between them,
and that is what I have implemented in this patch. But we may make a
different decision for the kernel such as having CONFIG_KASAN_OUTLINE=y
turn on specialized outlined checks if Clang is new enough.

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/I1a30036c70ab3c3ee78d75ed9b87ef7cdc3fdb76
Link: [1] https://reviews.llvm.org/D90426
Link: [2] https://reviews.llvm.org/D56954
---
v3:
- adopt Mark Rutland's suggested changes
- move frame record alignment patches behind this one

v2:
- use calculations in the stack spills and restores
- improve the comment at the top of the function
- add a BTI instruction

 arch/arm64/include/asm/asm-prototypes.h |  6 ++
 arch/arm64/include/asm/module.lds.h     | 17 +++++-
 arch/arm64/lib/Makefile                 |  2 +
 arch/arm64/lib/kasan_sw_tags.S          | 76 +++++++++++++++++++++++++
 mm/kasan/tags.c                         |  7 +++
 scripts/Makefile.kasan                  |  1 +
 6 files changed, 107 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/lib/kasan_sw_tags.S

diff --git a/arch/arm64/include/asm/asm-prototypes.h b/arch/arm64/include/asm/asm-prototypes.h
index 1c9a3a0c5fa5..ec1d9655f885 100644
--- a/arch/arm64/include/asm/asm-prototypes.h
+++ b/arch/arm64/include/asm/asm-prototypes.h
@@ -23,4 +23,10 @@ long long __ashlti3(long long a, int b);
 long long __ashrti3(long long a, int b);
 long long __lshrti3(long long a, int b);
 
+/*
+ * This function uses a custom calling convention and cannot be called from C so
+ * this prototype is not entirely accurate.
+ */
+void __hwasan_tag_mismatch(unsigned long addr, unsigned long access_info);
+
 #endif /* __ASM_PROTOTYPES_H */
diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
index 691f15af788e..4a6d717f75f3 100644
--- a/arch/arm64/include/asm/module.lds.h
+++ b/arch/arm64/include/asm/module.lds.h
@@ -1,7 +1,20 @@
-#ifdef CONFIG_ARM64_MODULE_PLTS
 SECTIONS {
+#ifdef CONFIG_ARM64_MODULE_PLTS
 	.plt (NOLOAD) : { BYTE(0) }
 	.init.plt (NOLOAD) : { BYTE(0) }
 	.text.ftrace_trampoline (NOLOAD) : { BYTE(0) }
-}
 #endif
+
+#ifdef CONFIG_KASAN_SW_TAGS
+	/*
+	 * Outlined checks go into comdat-deduplicated sections named .text.hot.
+	 * Because they are in comdats they are not combined by the linker and
+	 * we otherwise end up with multiple sections with the same .text.hot
+	 * name in the .ko file. The kernel module loader warns if it sees
+	 * multiple sections with the same name so we use this sections
+	 * directive to force them into a single section and silence the
+	 * warning.
+	 */
+	.text.hot : { *(.text.hot) }
+#endif
+}
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index d31e1169d9b8..8e60d76a1b47 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -18,3 +18,5 @@ obj-$(CONFIG_CRC32) += crc32.o
 obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
 
 obj-$(CONFIG_ARM64_MTE) += mte.o
+
+obj-$(CONFIG_KASAN_SW_TAGS) += kasan_sw_tags.o
diff --git a/arch/arm64/lib/kasan_sw_tags.S b/arch/arm64/lib/kasan_sw_tags.S
new file mode 100644
index 000000000000..5b04464c045e
--- /dev/null
+++ b/arch/arm64/lib/kasan_sw_tags.S
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Google LLC
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+/*
+ * Report a tag mismatch detected by tag-based KASAN.
+ *
+ * A compiler-generated thunk calls this with a non-AAPCS calling
+ * convention. Upon entry to this function, registers are as follows:
+ *
+ * x0:         fault address (see below for restore)
+ * x1:         fault description (see below for restore)
+ * x2 to x15:  callee-saved
+ * x16 to x17: safe to clobber
+ * x18 to x30: callee-saved
+ * sp:         pre-decremented by 256 bytes (see below for restore)
+ *
+ * The caller has decremented the SP by 256 bytes, and created a
+ * structure on the stack as follows:
+ *
+ * sp + 0..15:    x0 and x1 to be restored
+ * sp + 16..231:  free for use
+ * sp + 232..247: x29 and x30 (same as in GPRs)
+ * sp + 248..255: free for use
+ *
+ * Note that this is not a struct pt_regs.
+ *
+ * To call a regular AAPCS function we must save x2 to x15 (which we can
+ * store in the gaps), and create a frame record (for which we can use
+ * x29 and x30 spilled by the caller as those match the GPRs).
+ *
+ * The caller expects x0 and x1 to be restored from the structure, and
+ * for the structure to be removed from the stack (i.e. the SP must be
+ * incremented by 256 prior to return).
+ */
+SYM_CODE_START(__hwasan_tag_mismatch)
+#ifdef BTI_C
+	BTI_C
+#endif
+	add	x29, sp, #232
+	stp	x2, x3, [sp, #8 * 2]
+	stp	x4, x5, [sp, #8 * 4]
+	stp	x6, x7, [sp, #8 * 6]
+	stp	x8, x9, [sp, #8 * 8]
+	stp	x10, x11, [sp, #8 * 10]
+	stp	x12, x13, [sp, #8 * 12]
+	stp	x14, x15, [sp, #8 * 14]
+#ifndef CONFIG_SHADOW_CALL_STACK
+	str	x18, [sp, #8 * 18]
+#endif
+
+	mov	x2, x30
+	bl	kasan_tag_mismatch
+
+	ldp	x0, x1, [sp]
+	ldp	x2, x3, [sp, #8 * 2]
+	ldp	x4, x5, [sp, #8 * 4]
+	ldp	x6, x7, [sp, #8 * 6]
+	ldp	x8, x9, [sp, #8 * 8]
+	ldp	x10, x11, [sp, #8 * 10]
+	ldp	x12, x13, [sp, #8 * 12]
+	ldp	x14, x15, [sp, #8 * 14]
+#ifndef CONFIG_SHADOW_CALL_STACK
+	ldr	x18, [sp, #8 * 18]
+#endif
+	ldp	x29, x30, [sp, #8 * 29]
+
+	/* remove the structure from the stack */
+	add	sp, sp, #256
+	ret
+SYM_CODE_END(__hwasan_tag_mismatch)
+EXPORT_SYMBOL(__hwasan_tag_mismatch)
diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
index e02a36a51f42..d00613956c79 100644
--- a/mm/kasan/tags.c
+++ b/mm/kasan/tags.c
@@ -198,3 +198,10 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
 
 	return &alloc_meta->free_track[i];
 }
+
+void kasan_tag_mismatch(unsigned long addr, unsigned long access_info,
+			unsigned long ret_ip)
+{
+	kasan_report(addr, 1 << (access_info & 0xf), access_info & 0x10,
+		     ret_ip);
+}
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index 1e000cc2e7b4..1f2cccc264d8 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -44,6 +44,7 @@ endif
 CFLAGS_KASAN := -fsanitize=kernel-hwaddress \
 		-mllvm -hwasan-instrument-stack=$(CONFIG_KASAN_STACK) \
 		-mllvm -hwasan-use-short-granules=0 \
+		-mllvm -hwasan-inline-all-checks=0 \
 		$(instrumentation_flags)
 
 endif # CONFIG_KASAN_SW_TAGS
-- 
2.29.2.454.gaff20da3a2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument
  2020-12-03  5:12 [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument Peter Collingbourne
  2020-12-03  5:12 ` [PATCH v3 2/3] arm64: stacktrace: Relax frame record alignment requirement to 8 bytes Peter Collingbourne
  2020-12-03  5:12 ` [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks Peter Collingbourne
@ 2020-12-08 10:20 ` Mark Rutland
  2 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2020-12-08 10:20 UTC (permalink / raw)
  To: Peter Collingbourne
  Cc: Catalin Marinas, Mark Brown, Linux ARM, Andrey Konovalov,
	Will Deacon, Ard Biesheuvel, Evgenii Stepanov

Hi Peter,

Thanks for bearing with all the review comments. The patches look good
to me now, and I've given this some light testing locally with a
tip-of-tree clang (testing KASAN with LKDTM, and a short soak under
Syzkaller with lots of debug enabled) and that hasn't highlighted any
problems.

So for the series:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

On Wed, Dec 02, 2020 at 09:12:22PM -0800, Peter Collingbourne wrote:
> unwind_frame() was previously implicitly checking that the frame
> record is in bounds of the stack by enforcing that FP is both aligned
> to 16 and in bounds of the stack. Once the FP alignment requirement
> is relaxed to 8 this will not be sufficient because it does not
> account for the case where FP points to 8 bytes before the end of the
> stack.
> 
> Make the check explicit by changing the on_*stack functions to take a
> size argument and adjusting the callers to pass the appropriate sizes.
> 
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/Ib7a3eb3eea41b0687ffaba045ceb2012d077d8b4
> ---
>  arch/arm64/include/asm/processor.h  | 12 +++++------
>  arch/arm64/include/asm/sdei.h       |  7 ++++---
>  arch/arm64/include/asm/stacktrace.h | 32 ++++++++++++++---------------
>  arch/arm64/kernel/ptrace.c          |  2 +-
>  arch/arm64/kernel/sdei.c            | 16 ++++++++-------
>  arch/arm64/kernel/stacktrace.c      |  2 +-
>  6 files changed, 37 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index fce8cbecd6bc..bf3e07f67e79 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -318,13 +318,13 @@ long get_tagged_addr_ctrl(struct task_struct *task);
>   * of header definitions for the use of task_stack_page.
>   */
>  
> -#define current_top_of_stack()							\
> -({										\
> -	struct stack_info _info;						\
> -	BUG_ON(!on_accessible_stack(current, current_stack_pointer, &_info));	\
> -	_info.high;								\
> +#define current_top_of_stack()								\
> +({											\
> +	struct stack_info _info;							\
> +	BUG_ON(!on_accessible_stack(current, current_stack_pointer, 1, &_info));	\
> +	_info.high;									\
>  })
> -#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, NULL))
> +#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, 1, NULL))
>  
>  #endif /* __ASSEMBLY__ */
>  #endif /* __ASM_PROCESSOR_H */
> diff --git a/arch/arm64/include/asm/sdei.h b/arch/arm64/include/asm/sdei.h
> index 63e0b92a5fbb..8bc30a5c4569 100644
> --- a/arch/arm64/include/asm/sdei.h
> +++ b/arch/arm64/include/asm/sdei.h
> @@ -42,8 +42,9 @@ unsigned long sdei_arch_get_entry_point(int conduit);
>  
>  struct stack_info;
>  
> -bool _on_sdei_stack(unsigned long sp, struct stack_info *info);
> -static inline bool on_sdei_stack(unsigned long sp,
> +bool _on_sdei_stack(unsigned long sp, unsigned long size,
> +		    struct stack_info *info);
> +static inline bool on_sdei_stack(unsigned long sp, unsigned long size,
>  				struct stack_info *info)
>  {
>  	if (!IS_ENABLED(CONFIG_VMAP_STACK))
> @@ -51,7 +52,7 @@ static inline bool on_sdei_stack(unsigned long sp,
>  	if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
>  		return false;
>  	if (in_nmi())
> -		return _on_sdei_stack(sp, info);
> +		return _on_sdei_stack(sp, size, info);
>  
>  	return false;
>  }
> diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
> index eb29b1fe8255..77192df232bf 100644
> --- a/arch/arm64/include/asm/stacktrace.h
> +++ b/arch/arm64/include/asm/stacktrace.h
> @@ -69,14 +69,14 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
>  
>  DECLARE_PER_CPU(unsigned long *, irq_stack_ptr);
>  
> -static inline bool on_stack(unsigned long sp, unsigned long low,
> -				unsigned long high, enum stack_type type,
> -				struct stack_info *info)
> +static inline bool on_stack(unsigned long sp, unsigned long size,
> +			    unsigned long low, unsigned long high,
> +			    enum stack_type type, struct stack_info *info)
>  {
>  	if (!low)
>  		return false;
>  
> -	if (sp < low || sp >= high)
> +	if (sp < low || sp + size < sp || sp + size > high)
>  		return false;
>  
>  	if (info) {
> @@ -87,38 +87,38 @@ static inline bool on_stack(unsigned long sp, unsigned long low,
>  	return true;
>  }
>  
> -static inline bool on_irq_stack(unsigned long sp,
> +static inline bool on_irq_stack(unsigned long sp, unsigned long size,
>  				struct stack_info *info)
>  {
>  	unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
>  	unsigned long high = low + IRQ_STACK_SIZE;
>  
> -	return on_stack(sp, low, high, STACK_TYPE_IRQ, info);
> +	return on_stack(sp, size, low, high, STACK_TYPE_IRQ, info);
>  }
>  
>  static inline bool on_task_stack(const struct task_struct *tsk,
> -				 unsigned long sp,
> +				 unsigned long sp, unsigned long size,
>  				 struct stack_info *info)
>  {
>  	unsigned long low = (unsigned long)task_stack_page(tsk);
>  	unsigned long high = low + THREAD_SIZE;
>  
> -	return on_stack(sp, low, high, STACK_TYPE_TASK, info);
> +	return on_stack(sp, size, low, high, STACK_TYPE_TASK, info);
>  }
>  
>  #ifdef CONFIG_VMAP_STACK
>  DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack);
>  
> -static inline bool on_overflow_stack(unsigned long sp,
> +static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
>  				struct stack_info *info)
>  {
>  	unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack);
>  	unsigned long high = low + OVERFLOW_STACK_SIZE;
>  
> -	return on_stack(sp, low, high, STACK_TYPE_OVERFLOW, info);
> +	return on_stack(sp, size, low, high, STACK_TYPE_OVERFLOW, info);
>  }
>  #else
> -static inline bool on_overflow_stack(unsigned long sp,
> +static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
>  			struct stack_info *info) { return false; }
>  #endif
>  
> @@ -128,21 +128,21 @@ static inline bool on_overflow_stack(unsigned long sp,
>   * context.
>   */
>  static inline bool on_accessible_stack(const struct task_struct *tsk,
> -				       unsigned long sp,
> +				       unsigned long sp, unsigned long size,
>  				       struct stack_info *info)
>  {
>  	if (info)
>  		info->type = STACK_TYPE_UNKNOWN;
>  
> -	if (on_task_stack(tsk, sp, info))
> +	if (on_task_stack(tsk, sp, size, info))
>  		return true;
>  	if (tsk != current || preemptible())
>  		return false;
> -	if (on_irq_stack(sp, info))
> +	if (on_irq_stack(sp, size, info))
>  		return true;
> -	if (on_overflow_stack(sp, info))
> +	if (on_overflow_stack(sp, size, info))
>  		return true;
> -	if (on_sdei_stack(sp, info))
> +	if (on_sdei_stack(sp, size, info))
>  		return true;
>  
>  	return false;
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index f49b349e16a3..b44d6981decb 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -122,7 +122,7 @@ static bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr)
>  {
>  	return ((addr & ~(THREAD_SIZE - 1))  ==
>  		(kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1))) ||
> -		on_irq_stack(addr, NULL);
> +		on_irq_stack(addr, sizeof(unsigned long), NULL);
>  }
>  
>  /**
> diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
> index 793c46d6a447..ee8a13ce239b 100644
> --- a/arch/arm64/kernel/sdei.c
> +++ b/arch/arm64/kernel/sdei.c
> @@ -91,31 +91,33 @@ static int init_sdei_stacks(void)
>  	return err;
>  }
>  
> -static bool on_sdei_normal_stack(unsigned long sp, struct stack_info *info)
> +static bool on_sdei_normal_stack(unsigned long sp, unsigned long size,
> +				 struct stack_info *info)
>  {
>  	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
>  	unsigned long high = low + SDEI_STACK_SIZE;
>  
> -	return on_stack(sp, low, high, STACK_TYPE_SDEI_NORMAL, info);
> +	return on_stack(sp, size, low, high, STACK_TYPE_SDEI_NORMAL, info);
>  }
>  
> -static bool on_sdei_critical_stack(unsigned long sp, struct stack_info *info)
> +static bool on_sdei_critical_stack(unsigned long sp, unsigned long size,
> +				   struct stack_info *info)
>  {
>  	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
>  	unsigned long high = low + SDEI_STACK_SIZE;
>  
> -	return on_stack(sp, low, high, STACK_TYPE_SDEI_CRITICAL, info);
> +	return on_stack(sp, size, low, high, STACK_TYPE_SDEI_CRITICAL, info);
>  }
>  
> -bool _on_sdei_stack(unsigned long sp, struct stack_info *info)
> +bool _on_sdei_stack(unsigned long sp, unsigned long size, struct stack_info *info)
>  {
>  	if (!IS_ENABLED(CONFIG_VMAP_STACK))
>  		return false;
>  
> -	if (on_sdei_critical_stack(sp, info))
> +	if (on_sdei_critical_stack(sp, size, info))
>  		return true;
>  
> -	if (on_sdei_normal_stack(sp, info))
> +	if (on_sdei_normal_stack(sp, size, info))
>  		return true;
>  
>  	return false;
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index fa56af1a59c3..ce613e22b58b 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -50,7 +50,7 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
>  	if (!tsk)
>  		tsk = current;
>  
> -	if (!on_accessible_stack(tsk, fp, &info))
> +	if (!on_accessible_stack(tsk, fp, 16, &info))
>  		return -EINVAL;
>  
>  	if (test_bit(info.type, frame->stacks_done))
> -- 
> 2.29.2.454.gaff20da3a2-goog
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks
  2020-12-03  5:12 ` [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks Peter Collingbourne
@ 2020-12-08 17:49   ` Catalin Marinas
  2021-01-20 12:41     ` Will Deacon
  2021-01-07 15:20   ` Andrey Konovalov
  1 sibling, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2020-12-08 17:49 UTC (permalink / raw)
  To: Peter Collingbourne
  Cc: Mark Rutland, Andrey Konovalov, Mark Brown, Linux ARM,
	Will Deacon, Ard Biesheuvel, Evgenii Stepanov

On Wed, Dec 02, 2020 at 09:12:24PM -0800, Peter Collingbourne wrote:
> By using outlined checks we can achieve a significant code size
> improvement by moving the tag-based ASAN checks into separate
> functions. Unlike the existing CONFIG_KASAN_OUTLINE mode these
> functions have a custom calling convention that preserves most
> registers and is specialized to the register containing the address
> and the type of access, and as a result we can eliminate the code
> size and performance overhead of a standard calling convention such
> as AAPCS for these functions.
> 
> This change depends on a separate series of changes to Clang [1] to
> support outlined checks in the kernel, although the change works fine
> without them (we just don't get outlined checks). This is because the
> flag -mllvm -hwasan-inline-all-checks=0 has no effect until the Clang
> changes land. The flag was introduced in the Clang 9.0 timeframe as
> part of the support for outlined checks in userspace and because our
> minimum Clang version is 10.0 we can pass it unconditionally.
> 
> Outlined checks require a new runtime function with a custom calling
> convention. Add this function to arch/arm64/lib.
> 
> I measured the code size of defconfig + tag-based KASAN, as well
> as boot time (i.e. time to init launch) on a DragonBoard 845c with
> an Android arm64 GKI kernel. The results are below:
> 
>                                code size    boot time
> CONFIG_KASAN_INLINE=y before    92824064      6.18s
> CONFIG_KASAN_INLINE=y after     38822400      6.65s
> CONFIG_KASAN_OUTLINE=y          39215616     11.48s
> 
> We can see straight away that specialized outlined checks beat the
> existing CONFIG_KASAN_OUTLINE=y on both code size and boot time
> for tag-based ASAN.
> 
> As for the comparison between CONFIG_KASAN_INLINE=y before and after
> we saw similar performance numbers in userspace [2] and decided
> that since the performance overhead is minimal compared to the
> overhead of tag-based ASAN itself as well as compared to the code
> size improvements we would just replace the inlined checks with the
> specialized outlined checks without the option to select between them,
> and that is what I have implemented in this patch. But we may make a
> different decision for the kernel such as having CONFIG_KASAN_OUTLINE=y
> turn on specialized outlined checks if Clang is new enough.
> 
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/I1a30036c70ab3c3ee78d75ed9b87ef7cdc3fdb76
> Link: [1] https://reviews.llvm.org/D90426
> Link: [2] https://reviews.llvm.org/D56954
> ---
> v3:
> - adopt Mark Rutland's suggested changes
> - move frame record alignment patches behind this one
> 
> v2:
> - use calculations in the stack spills and restores
> - improve the comment at the top of the function
> - add a BTI instruction
> 
>  arch/arm64/include/asm/asm-prototypes.h |  6 ++
>  arch/arm64/include/asm/module.lds.h     | 17 +++++-
>  arch/arm64/lib/Makefile                 |  2 +
>  arch/arm64/lib/kasan_sw_tags.S          | 76 +++++++++++++++++++++++++
>  mm/kasan/tags.c                         |  7 +++
>  scripts/Makefile.kasan                  |  1 +

I can try to queue the series but this patch would need an ack on the
kasan changes.

(also, it may conflict with linux-next which renames tags.c to sw_tags.c
but that's trivial)

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks
  2020-12-03  5:12 ` [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks Peter Collingbourne
  2020-12-08 17:49   ` Catalin Marinas
@ 2021-01-07 15:20   ` Andrey Konovalov
  1 sibling, 0 replies; 8+ messages in thread
From: Andrey Konovalov @ 2021-01-07 15:20 UTC (permalink / raw)
  To: Peter Collingbourne
  Cc: Mark Rutland, Catalin Marinas, Mark Brown, Linux ARM,
	Will Deacon, Ard Biesheuvel, Evgenii Stepanov

On Thu, Dec 3, 2020 at 6:12 AM Peter Collingbourne <pcc@google.com> wrote:
>
> By using outlined checks we can achieve a significant code size
> improvement by moving the tag-based ASAN checks into separate
> functions. Unlike the existing CONFIG_KASAN_OUTLINE mode these
> functions have a custom calling convention that preserves most
> registers and is specialized to the register containing the address
> and the type of access, and as a result we can eliminate the code
> size and performance overhead of a standard calling convention such
> as AAPCS for these functions.
>
> This change depends on a separate series of changes to Clang [1] to
> support outlined checks in the kernel, although the change works fine
> without them (we just don't get outlined checks). This is because the
> flag -mllvm -hwasan-inline-all-checks=0 has no effect until the Clang
> changes land. The flag was introduced in the Clang 9.0 timeframe as
> part of the support for outlined checks in userspace and because our
> minimum Clang version is 10.0 we can pass it unconditionally.
>
> Outlined checks require a new runtime function with a custom calling
> convention. Add this function to arch/arm64/lib.
>
> I measured the code size of defconfig + tag-based KASAN, as well
> as boot time (i.e. time to init launch) on a DragonBoard 845c with
> an Android arm64 GKI kernel. The results are below:
>
>                                code size    boot time
> CONFIG_KASAN_INLINE=y before    92824064      6.18s
> CONFIG_KASAN_INLINE=y after     38822400      6.65s
> CONFIG_KASAN_OUTLINE=y          39215616     11.48s
>
> We can see straight away that specialized outlined checks beat the
> existing CONFIG_KASAN_OUTLINE=y on both code size and boot time
> for tag-based ASAN.
>
> As for the comparison between CONFIG_KASAN_INLINE=y before and after
> we saw similar performance numbers in userspace [2] and decided
> that since the performance overhead is minimal compared to the
> overhead of tag-based ASAN itself as well as compared to the code
> size improvements we would just replace the inlined checks with the
> specialized outlined checks without the option to select between them,
> and that is what I have implemented in this patch. But we may make a
> different decision for the kernel such as having CONFIG_KASAN_OUTLINE=y
> turn on specialized outlined checks if Clang is new enough.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/I1a30036c70ab3c3ee78d75ed9b87ef7cdc3fdb76
> Link: [1] https://reviews.llvm.org/D90426
> Link: [2] https://reviews.llvm.org/D56954
> ---
> v3:
> - adopt Mark Rutland's suggested changes
> - move frame record alignment patches behind this one
>
> v2:
> - use calculations in the stack spills and restores
> - improve the comment at the top of the function
> - add a BTI instruction
>
>  arch/arm64/include/asm/asm-prototypes.h |  6 ++
>  arch/arm64/include/asm/module.lds.h     | 17 +++++-
>  arch/arm64/lib/Makefile                 |  2 +
>  arch/arm64/lib/kasan_sw_tags.S          | 76 +++++++++++++++++++++++++
>  mm/kasan/tags.c                         |  7 +++
>  scripts/Makefile.kasan                  |  1 +
>  6 files changed, 107 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/lib/kasan_sw_tags.S
>
> diff --git a/arch/arm64/include/asm/asm-prototypes.h b/arch/arm64/include/asm/asm-prototypes.h
> index 1c9a3a0c5fa5..ec1d9655f885 100644
> --- a/arch/arm64/include/asm/asm-prototypes.h
> +++ b/arch/arm64/include/asm/asm-prototypes.h
> @@ -23,4 +23,10 @@ long long __ashlti3(long long a, int b);
>  long long __ashrti3(long long a, int b);
>  long long __lshrti3(long long a, int b);
>
> +/*
> + * This function uses a custom calling convention and cannot be called from C so
> + * this prototype is not entirely accurate.
> + */
> +void __hwasan_tag_mismatch(unsigned long addr, unsigned long access_info);
> +
>  #endif /* __ASM_PROTOTYPES_H */
> diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
> index 691f15af788e..4a6d717f75f3 100644
> --- a/arch/arm64/include/asm/module.lds.h
> +++ b/arch/arm64/include/asm/module.lds.h
> @@ -1,7 +1,20 @@
> -#ifdef CONFIG_ARM64_MODULE_PLTS
>  SECTIONS {
> +#ifdef CONFIG_ARM64_MODULE_PLTS
>         .plt (NOLOAD) : { BYTE(0) }
>         .init.plt (NOLOAD) : { BYTE(0) }
>         .text.ftrace_trampoline (NOLOAD) : { BYTE(0) }
> -}
>  #endif
> +
> +#ifdef CONFIG_KASAN_SW_TAGS
> +       /*
> +        * Outlined checks go into comdat-deduplicated sections named .text.hot.
> +        * Because they are in comdats they are not combined by the linker and
> +        * we otherwise end up with multiple sections with the same .text.hot
> +        * name in the .ko file. The kernel module loader warns if it sees
> +        * multiple sections with the same name so we use this sections
> +        * directive to force them into a single section and silence the
> +        * warning.
> +        */
> +       .text.hot : { *(.text.hot) }
> +#endif
> +}
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index d31e1169d9b8..8e60d76a1b47 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -18,3 +18,5 @@ obj-$(CONFIG_CRC32) += crc32.o
>  obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
>
>  obj-$(CONFIG_ARM64_MTE) += mte.o
> +
> +obj-$(CONFIG_KASAN_SW_TAGS) += kasan_sw_tags.o
> diff --git a/arch/arm64/lib/kasan_sw_tags.S b/arch/arm64/lib/kasan_sw_tags.S
> new file mode 100644
> index 000000000000..5b04464c045e
> --- /dev/null
> +++ b/arch/arm64/lib/kasan_sw_tags.S
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Google LLC
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +
> +/*
> + * Report a tag mismatch detected by tag-based KASAN.
> + *
> + * A compiler-generated thunk calls this with a non-AAPCS calling
> + * convention. Upon entry to this function, registers are as follows:
> + *
> + * x0:         fault address (see below for restore)
> + * x1:         fault description (see below for restore)
> + * x2 to x15:  callee-saved
> + * x16 to x17: safe to clobber
> + * x18 to x30: callee-saved
> + * sp:         pre-decremented by 256 bytes (see below for restore)
> + *
> + * The caller has decremented the SP by 256 bytes, and created a
> + * structure on the stack as follows:
> + *
> + * sp + 0..15:    x0 and x1 to be restored
> + * sp + 16..231:  free for use
> + * sp + 232..247: x29 and x30 (same as in GPRs)
> + * sp + 248..255: free for use
> + *
> + * Note that this is not a struct pt_regs.
> + *
> + * To call a regular AAPCS function we must save x2 to x15 (which we can
> + * store in the gaps), and create a frame record (for which we can use
> + * x29 and x30 spilled by the caller as those match the GPRs).
> + *
> + * The caller expects x0 and x1 to be restored from the structure, and
> + * for the structure to be removed from the stack (i.e. the SP must be
> + * incremented by 256 prior to return).
> + */
> +SYM_CODE_START(__hwasan_tag_mismatch)
> +#ifdef BTI_C
> +       BTI_C
> +#endif
> +       add     x29, sp, #232
> +       stp     x2, x3, [sp, #8 * 2]
> +       stp     x4, x5, [sp, #8 * 4]
> +       stp     x6, x7, [sp, #8 * 6]
> +       stp     x8, x9, [sp, #8 * 8]
> +       stp     x10, x11, [sp, #8 * 10]
> +       stp     x12, x13, [sp, #8 * 12]
> +       stp     x14, x15, [sp, #8 * 14]
> +#ifndef CONFIG_SHADOW_CALL_STACK
> +       str     x18, [sp, #8 * 18]
> +#endif
> +
> +       mov     x2, x30
> +       bl      kasan_tag_mismatch
> +
> +       ldp     x0, x1, [sp]
> +       ldp     x2, x3, [sp, #8 * 2]
> +       ldp     x4, x5, [sp, #8 * 4]
> +       ldp     x6, x7, [sp, #8 * 6]
> +       ldp     x8, x9, [sp, #8 * 8]
> +       ldp     x10, x11, [sp, #8 * 10]
> +       ldp     x12, x13, [sp, #8 * 12]
> +       ldp     x14, x15, [sp, #8 * 14]
> +#ifndef CONFIG_SHADOW_CALL_STACK
> +       ldr     x18, [sp, #8 * 18]
> +#endif
> +       ldp     x29, x30, [sp, #8 * 29]
> +
> +       /* remove the structure from the stack */
> +       add     sp, sp, #256
> +       ret
> +SYM_CODE_END(__hwasan_tag_mismatch)
> +EXPORT_SYMBOL(__hwasan_tag_mismatch)
> diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
> index e02a36a51f42..d00613956c79 100644
> --- a/mm/kasan/tags.c
> +++ b/mm/kasan/tags.c
> @@ -198,3 +198,10 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
>
>         return &alloc_meta->free_track[i];
>  }
> +
> +void kasan_tag_mismatch(unsigned long addr, unsigned long access_info,
> +                       unsigned long ret_ip)
> +{
> +       kasan_report(addr, 1 << (access_info & 0xf), access_info & 0x10,
> +                    ret_ip);
> +}
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 1e000cc2e7b4..1f2cccc264d8 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -44,6 +44,7 @@ endif
>  CFLAGS_KASAN := -fsanitize=kernel-hwaddress \
>                 -mllvm -hwasan-instrument-stack=$(CONFIG_KASAN_STACK) \
>                 -mllvm -hwasan-use-short-granules=0 \
> +               -mllvm -hwasan-inline-all-checks=0 \
>                 $(instrumentation_flags)
>
>  endif # CONFIG_KASAN_SW_TAGS
> --
> 2.29.2.454.gaff20da3a2-goog
>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks
  2020-12-08 17:49   ` Catalin Marinas
@ 2021-01-20 12:41     ` Will Deacon
  2021-05-13  2:30       ` Peter Collingbourne
  0 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2021-01-20 12:41 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Andrey Konovalov, Mark Brown, Linux ARM,
	Peter Collingbourne, Ard Biesheuvel, Evgenii Stepanov

On Tue, Dec 08, 2020 at 05:49:07PM +0000, Catalin Marinas wrote:
> On Wed, Dec 02, 2020 at 09:12:24PM -0800, Peter Collingbourne wrote:
> > By using outlined checks we can achieve a significant code size
> > improvement by moving the tag-based ASAN checks into separate
> > functions. Unlike the existing CONFIG_KASAN_OUTLINE mode these
> > functions have a custom calling convention that preserves most
> > registers and is specialized to the register containing the address
> > and the type of access, and as a result we can eliminate the code
> > size and performance overhead of a standard calling convention such
> > as AAPCS for these functions.

[...]

> >  arch/arm64/include/asm/asm-prototypes.h |  6 ++
> >  arch/arm64/include/asm/module.lds.h     | 17 +++++-
> >  arch/arm64/lib/Makefile                 |  2 +
> >  arch/arm64/lib/kasan_sw_tags.S          | 76 +++++++++++++++++++++++++
> >  mm/kasan/tags.c                         |  7 +++
> >  scripts/Makefile.kasan                  |  1 +
> 
> I can try to queue the series but this patch would need an ack on the
> kasan changes.
> 
> (also, it may conflict with linux-next which renames tags.c to sw_tags.c
> but that's trivial)

Looks like this didn't land for 5.11. Peter -- please can you send a rebased
version (on -rc4) so that I can queue it for 5.12?

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks
  2021-01-20 12:41     ` Will Deacon
@ 2021-05-13  2:30       ` Peter Collingbourne
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Collingbourne @ 2021-05-13  2:30 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, Mark Brown, Mark Rutland, Andrey Konovalov,
	Evgenii Stepanov, Ard Biesheuvel, Linux ARM

On Wed, Jan 20, 2021 at 4:41 AM Will Deacon <will@kernel.org> wrote:
>
> On Tue, Dec 08, 2020 at 05:49:07PM +0000, Catalin Marinas wrote:
> > On Wed, Dec 02, 2020 at 09:12:24PM -0800, Peter Collingbourne wrote:
> > > By using outlined checks we can achieve a significant code size
> > > improvement by moving the tag-based ASAN checks into separate
> > > functions. Unlike the existing CONFIG_KASAN_OUTLINE mode these
> > > functions have a custom calling convention that preserves most
> > > registers and is specialized to the register containing the address
> > > and the type of access, and as a result we can eliminate the code
> > > size and performance overhead of a standard calling convention such
> > > as AAPCS for these functions.
>
> [...]
>
> > >  arch/arm64/include/asm/asm-prototypes.h |  6 ++
> > >  arch/arm64/include/asm/module.lds.h     | 17 +++++-
> > >  arch/arm64/lib/Makefile                 |  2 +
> > >  arch/arm64/lib/kasan_sw_tags.S          | 76 +++++++++++++++++++++++++
> > >  mm/kasan/tags.c                         |  7 +++
> > >  scripts/Makefile.kasan                  |  1 +
> >
> > I can try to queue the series but this patch would need an ack on the
> > kasan changes.
> >
> > (also, it may conflict with linux-next which renames tags.c to sw_tags.c
> > but that's trivial)
>
> Looks like this didn't land for 5.11. Peter -- please can you send a rebased
> version (on -rc4) so that I can queue it for 5.12?

Sorry for dropping the ball on this -- I've sent out a rebased v4 that
should be ready to go into 5.14 -- I guess it's too late for 5.13.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-05-13  2:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03  5:12 [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument Peter Collingbourne
2020-12-03  5:12 ` [PATCH v3 2/3] arm64: stacktrace: Relax frame record alignment requirement to 8 bytes Peter Collingbourne
2020-12-03  5:12 ` [PATCH v3 3/3] kasan: arm64: support specialized outlined tag mismatch checks Peter Collingbourne
2020-12-08 17:49   ` Catalin Marinas
2021-01-20 12:41     ` Will Deacon
2021-05-13  2:30       ` Peter Collingbourne
2021-01-07 15:20   ` Andrey Konovalov
2020-12-08 10:20 ` [PATCH v3 1/3] arm64: Change the on_*stack functions to take a size argument Mark Rutland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).