linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
@ 2021-01-26 13:45 Vincenzo Frascino
  2021-01-26 13:46 ` [PATCH v9 1/4] arm64: mte: Add asynchronous " Vincenzo Frascino
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-26 13:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Andrew Morton, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

This patchset implements the asynchronous mode support for ARMv8.5-A
Memory Tagging Extension (MTE), which is a debugging feature that allows
to detect with the help of the architecture the C and C++ programmatic
memory errors like buffer overflow, use-after-free, use-after-return, etc.

MTE is built on top of the AArch64 v8.0 virtual address tagging TBI
(Top Byte Ignore) feature and allows a task to set a 4 bit tag on any
subset of its address space that is multiple of a 16 bytes granule. MTE
is based on a lock-key mechanism where the lock is the tag associated to
the physical memory and the key is the tag associated to the virtual
address.
When MTE is enabled and tags are set for ranges of address space of a task,
the PE will compare the tag related to the physical memory with the tag
related to the virtual address (tag check operation). Access to the memory
is granted only if the two tags match. In case of mismatch the PE will raise
an exception.

The exception can be handled synchronously or asynchronously. When the
asynchronous mode is enabled:
  - Upon fault the PE updates the TFSR_EL1 register.
  - The kernel detects the change during one of the following:
    - Context switching
    - Return to user/EL0
    - Kernel entry from EL1
    - Kernel exit to EL1
  - If the register has been updated by the PE the kernel clears it and
    reports the error.

The series is based on linux-next/akpm.

To simplify the testing a tree with the new patches on top has been made
available at [1].

[1] https://git.gitlab.arm.com/linux-arm/linux-vf.git mte/v10.async.akpm

Changes:
--------
v9:
  - Rebase on the latest linux-next/akpm
  - Address review comments.
v8:
  - Address review comments.
v7:
  - Fix a warning reported by kernel test robot. This
    time for real.
v6:
  - Drop patches that forbid KASAN KUNIT tests when async
    mode is enabled.
  - Fix a warning reported by kernel test robot.
  - Address review comments.
v5:
  - Rebase the series on linux-next/akpm.
  - Forbid execution for KASAN KUNIT tests when async
    mode is enabled.
  - Dropped patch to inline mte_assign_mem_tag_range().
  - Address review comments.
v4:
  - Added support for kasan.mode (sync/async) kernel
    command line parameter.
  - Addressed review comments.
v3:
  - Exposed kasan_hw_tags_mode to convert the internal
    KASAN represenetation.
  - Added dsb() for kernel exit paths in arm64.
  - Addressed review comments.
v2:
  - Fixed a compilation issue reported by krobot.
  - General cleanup.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Evgenii Stepanov <eugenis@google.com>
Cc: Branislav Rankov <Branislav.Rankov@arm.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

Vincenzo Frascino (4):
  arm64: mte: Add asynchronous mode support
  kasan: Add KASAN mode kernel parameter
  kasan: Add report for async mode
  arm64: mte: Enable async tag check fault

 Documentation/dev-tools/kasan.rst  |  9 +++++
 arch/arm64/include/asm/memory.h    |  3 +-
 arch/arm64/include/asm/mte-kasan.h |  9 ++++-
 arch/arm64/include/asm/mte.h       | 32 ++++++++++++++++
 arch/arm64/kernel/entry-common.c   |  6 +++
 arch/arm64/kernel/mte.c            | 60 +++++++++++++++++++++++++++++-
 include/linux/kasan.h              |  6 +++
 lib/test_kasan.c                   |  2 +-
 mm/kasan/hw_tags.c                 | 32 +++++++++++++++-
 mm/kasan/kasan.h                   |  6 ++-
 mm/kasan/report.c                  | 13 +++++++
 11 files changed, 169 insertions(+), 9 deletions(-)

-- 
2.30.0


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

* [PATCH v9 1/4] arm64: mte: Add asynchronous mode support
  2021-01-26 13:45 [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
@ 2021-01-26 13:46 ` Vincenzo Frascino
  2021-01-26 13:46 ` [PATCH v9 2/4] kasan: Add KASAN mode kernel parameter Vincenzo Frascino
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-26 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Andrew Morton, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

MTE provides an asynchronous mode for detecting tag exceptions. In
particular instead of triggering a fault the arm64 core updates a
register which is checked by the kernel after the asynchronous tag
check fault has occurred.

Add support for MTE asynchronous mode.

The exception handling mechanism will be added with a future patch.

Note: KASAN HW activates async mode via kasan.mode kernel parameter.
The default mode is set to synchronous.
The code that verifies the status of TFSR_EL1 will be added with a
future patch.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/memory.h    |  3 ++-
 arch/arm64/include/asm/mte-kasan.h |  9 +++++++--
 arch/arm64/kernel/mte.c            | 16 ++++++++++++++--
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index cedfc9e97bcc..df96b9c10b81 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -231,7 +231,8 @@ static inline const void *__tag_set(const void *addr, u8 tag)
 }
 
 #ifdef CONFIG_KASAN_HW_TAGS
-#define arch_enable_tagging()			mte_enable_kernel()
+#define arch_enable_tagging_sync()		mte_enable_kernel_sync()
+#define arch_enable_tagging_async()		mte_enable_kernel_async()
 #define arch_set_tagging_report_once(state)	mte_set_report_once(state)
 #define arch_init_tags(max_tag)			mte_init_tags(max_tag)
 #define arch_get_random_tag()			mte_get_random_tag()
diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
index 3748d5bb88c0..8ad981069afb 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -29,7 +29,8 @@ u8 mte_get_mem_tag(void *addr);
 u8 mte_get_random_tag(void);
 void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag);
 
-void mte_enable_kernel(void);
+void mte_enable_kernel_sync(void);
+void mte_enable_kernel_async(void);
 void mte_init_tags(u64 max_tag);
 
 void mte_set_report_once(bool state);
@@ -55,7 +56,11 @@ static inline void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
 	return addr;
 }
 
-static inline void mte_enable_kernel(void)
+static inline void mte_enable_kernel_sync(void)
+{
+}
+
+static inline void mte_enable_kernel_async(void)
 {
 }
 
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index c63b3d7a3cd9..92078e1eb627 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -153,11 +153,23 @@ void mte_init_tags(u64 max_tag)
 	write_sysreg_s(SYS_GCR_EL1_RRND | gcr_kernel_excl, SYS_GCR_EL1);
 }
 
-void mte_enable_kernel(void)
+static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
 {
 	/* Enable MTE Sync Mode for EL1. */
-	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, tcf);
 	isb();
+
+	pr_info_once("MTE: enabled in %s mode at EL1\n", mode);
+}
+
+void mte_enable_kernel_sync(void)
+{
+	__mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC);
+}
+
+void mte_enable_kernel_async(void)
+{
+	__mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
 }
 
 void mte_set_report_once(bool state)
-- 
2.30.0


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

* [PATCH v9 2/4] kasan: Add KASAN mode kernel parameter
  2021-01-26 13:45 [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  2021-01-26 13:46 ` [PATCH v9 1/4] arm64: mte: Add asynchronous " Vincenzo Frascino
@ 2021-01-26 13:46 ` Vincenzo Frascino
  2021-01-26 13:46 ` [PATCH v9 3/4] kasan: Add report for async mode Vincenzo Frascino
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-26 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Andrew Morton, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

Architectures supported by KASAN_HW_TAGS can provide a sync or async mode
of execution. On an MTE enabled arm64 hw for example this can be identified
with the synchronous or asynchronous tagging mode of execution.
In synchronous mode, an exception is triggered if a tag check fault occurs.
In asynchronous mode, if a tag check fault occurs, the TFSR_EL1 register is
updated asynchronously. The kernel checks the corresponding bits
periodically.

KASAN requires a specific kernel command line parameter to make use of this
hw features.

Add KASAN HW execution mode kernel command line parameter.

Note: This patch adds the kasan.mode kernel parameter and the
sync/async kernel command line options to enable the described features.

Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 Documentation/dev-tools/kasan.rst |  9 +++++++++
 lib/test_kasan.c                  |  2 +-
 mm/kasan/hw_tags.c                | 32 ++++++++++++++++++++++++++++++-
 mm/kasan/kasan.h                  |  6 ++++--
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index e022b7506e37..e3dca4d1f2a7 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -161,6 +161,15 @@ particular KASAN features.
 
 - ``kasan=off`` or ``=on`` controls whether KASAN is enabled (default: ``on``).
 
+- ``kasan.mode=sync`` or ``=async`` controls whether KASAN is configured in
+  synchronous or asynchronous mode of execution (default: ``sync``).
+  Synchronous mode: a bad access is detected immediately when a tag
+  check fault occurs.
+  Asynchronous mode: a bad access detection is delayed. When a tag check
+  fault occurs, the information is stored in hardware (in the TFSR_EL1
+  register for arm64). The kernel periodically checks the hardware and
+  only reports tag faults during these checks.
+
 - ``kasan.stacktrace=off`` or ``=on`` disables or enables alloc and free stack
   traces collection (default: ``on`` for ``CONFIG_DEBUG_KERNEL=y``, otherwise
   ``off``).
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index d16ec9e66806..7285dcf9fcc1 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -97,7 +97,7 @@ static void kasan_test_exit(struct kunit *test)
 			READ_ONCE(fail_data.report_found));	\
 	if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) {			\
 		if (READ_ONCE(fail_data.report_found))		\
-			hw_enable_tagging();			\
+			hw_enable_tagging_sync();		\
 		migrate_enable();				\
 	}							\
 } while (0)
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index e529428e7a11..308a879a3798 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -25,6 +25,12 @@ enum kasan_arg {
 	KASAN_ARG_ON,
 };
 
+enum kasan_arg_mode {
+	KASAN_ARG_MODE_DEFAULT,
+	KASAN_ARG_MODE_SYNC,
+	KASAN_ARG_MODE_ASYNC,
+};
+
 enum kasan_arg_stacktrace {
 	KASAN_ARG_STACKTRACE_DEFAULT,
 	KASAN_ARG_STACKTRACE_OFF,
@@ -38,6 +44,7 @@ enum kasan_arg_fault {
 };
 
 static enum kasan_arg kasan_arg __ro_after_init;
+static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
 static enum kasan_arg_stacktrace kasan_arg_stacktrace __ro_after_init;
 static enum kasan_arg_fault kasan_arg_fault __ro_after_init;
 
@@ -68,6 +75,21 @@ static int __init early_kasan_flag(char *arg)
 }
 early_param("kasan", early_kasan_flag);
 
+/* kasan.mode=sync/async */
+static int __init early_kasan_mode(char *arg)
+{
+	/* If arg is not set the default mode is sync */
+	if ((!arg) || !strcmp(arg, "sync"))
+		kasan_arg_mode = KASAN_ARG_MODE_SYNC;
+	else if (!strcmp(arg, "async"))
+		kasan_arg_mode = KASAN_ARG_MODE_ASYNC;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+early_param("kasan.mode", early_kasan_mode);
+
 /* kasan.stacktrace=off/on */
 static int __init early_kasan_flag_stacktrace(char *arg)
 {
@@ -115,7 +137,15 @@ void kasan_init_hw_tags_cpu(void)
 		return;
 
 	hw_init_tags(KASAN_TAG_MAX);
-	hw_enable_tagging();
+
+	/*
+	 * Enable async mode only when explicitly requested through
+	 * the command line.
+	 */
+	if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
+		hw_enable_tagging_async();
+	else
+		hw_enable_tagging_sync();
 }
 
 /* kasan_init_hw_tags() is called once on boot CPU. */
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 07ef7fc742ad..3923d9744105 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -294,7 +294,8 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 #define arch_set_mem_tag_range(addr, size, tag) ((void *)(addr))
 #endif
 
-#define hw_enable_tagging()			arch_enable_tagging()
+#define hw_enable_tagging_sync()		arch_enable_tagging_sync()
+#define hw_enable_tagging_async()		arch_enable_tagging_async()
 #define hw_init_tags(max_tag)			arch_init_tags(max_tag)
 #define hw_set_tagging_report_once(state)	arch_set_tagging_report_once(state)
 #define hw_get_random_tag()			arch_get_random_tag()
@@ -303,7 +304,8 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 
 #else /* CONFIG_KASAN_HW_TAGS */
 
-#define hw_enable_tagging()
+#define hw_enable_tagging_sync()
+#define hw_enable_tagging_async()
 #define hw_set_tagging_report_once(state)
 
 #endif /* CONFIG_KASAN_HW_TAGS */
-- 
2.30.0


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

* [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-26 13:45 [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  2021-01-26 13:46 ` [PATCH v9 1/4] arm64: mte: Add asynchronous " Vincenzo Frascino
  2021-01-26 13:46 ` [PATCH v9 2/4] kasan: Add KASAN mode kernel parameter Vincenzo Frascino
@ 2021-01-26 13:46 ` Vincenzo Frascino
  2021-01-29 17:40   ` Andrey Konovalov
  2021-01-26 13:46 ` [PATCH v9 4/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
  2021-01-27 20:00 ` [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Andrey Konovalov
  4 siblings, 1 reply; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-26 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Andrew Morton, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

KASAN provides an asynchronous mode of execution.

Add reporting functionality for this mode.

Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 include/linux/kasan.h |  6 ++++++
 mm/kasan/report.c     | 13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index bb862d1f0e15..b6c502dad54d 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -360,6 +360,12 @@ static inline void *kasan_reset_tag(const void *addr)
 
 #endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS*/
 
+#ifdef CONFIG_KASAN_HW_TAGS
+
+void kasan_report_async(void);
+
+#endif /* CONFIG_KASAN_HW_TAGS */
+
 #ifdef CONFIG_KASAN_SW_TAGS
 void __init kasan_init_sw_tags(void);
 #else
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 87b271206163..69bad9c01aed 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -360,6 +360,19 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
 	end_report(&flags, (unsigned long)object);
 }
 
+#ifdef CONFIG_KASAN_HW_TAGS
+void kasan_report_async(void)
+{
+	unsigned long flags;
+
+	start_report(&flags);
+	pr_err("BUG: KASAN: invalid-access\n");
+	pr_err("Asynchronous mode enabled: no access details available\n");
+	dump_stack();
+	end_report(&flags);
+}
+#endif /* CONFIG_KASAN_HW_TAGS */
+
 static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 				unsigned long ip)
 {
-- 
2.30.0


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

* [PATCH v9 4/4] arm64: mte: Enable async tag check fault
  2021-01-26 13:45 [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
                   ` (2 preceding siblings ...)
  2021-01-26 13:46 ` [PATCH v9 3/4] kasan: Add report for async mode Vincenzo Frascino
@ 2021-01-26 13:46 ` Vincenzo Frascino
  2021-01-27 20:00 ` [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Andrey Konovalov
  4 siblings, 0 replies; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-26 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Andrew Morton, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

MTE provides a mode that asynchronously updates the TFSR_EL1 register
when a tag check exception is detected.

To take advantage of this mode the kernel has to verify the status of
the register at:
  1. Context switching
  2. Return to user/EL0 (Not required in entry from EL0 since the kernel
  did not run)
  3. Kernel entry from EL1
  4. Kernel exit to EL1

If the register is non-zero a trace is reported.

Add the required features for EL1 detection and reporting.

Note: ITFSB bit is set in the SCTLR_EL1 register hence it guaranties that
the indirect writes to TFSR_EL1 are synchronized at exception entry to
EL1. On the context switch path the synchronization is guarantied by the
dsb() in __switch_to().
The dsb(nsh) in mte_check_tfsr_exit() is provisional pending
confirmation by the architects.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/mte.h     | 32 +++++++++++++++++++++++
 arch/arm64/kernel/entry-common.c |  6 +++++
 arch/arm64/kernel/mte.c          | 44 ++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index d02aff9f493d..237bb2f7309d 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -92,5 +92,37 @@ static inline void mte_assign_mem_tag_range(void *addr, size_t size)
 
 #endif /* CONFIG_ARM64_MTE */
 
+#ifdef CONFIG_KASAN_HW_TAGS
+void mte_check_tfsr_el1(void);
+
+static inline void mte_check_tfsr_entry(void)
+{
+	mte_check_tfsr_el1();
+}
+
+static inline void mte_check_tfsr_exit(void)
+{
+	/*
+	 * The asynchronous faults are sync'ed automatically with
+	 * TFSR_EL1 on kernel entry but for exit an explicit dsb()
+	 * is required.
+	 */
+	dsb(nsh);
+	isb();
+
+	mte_check_tfsr_el1();
+}
+#else
+static inline void mte_check_tfsr_el1(void)
+{
+}
+static inline void mte_check_tfsr_entry(void)
+{
+}
+static inline void mte_check_tfsr_exit(void)
+{
+}
+#endif /* CONFIG_KASAN_HW_TAGS */
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_MTE_H  */
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 5346953e4382..31666511ba67 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -37,6 +37,8 @@ static void noinstr enter_from_kernel_mode(struct pt_regs *regs)
 	lockdep_hardirqs_off(CALLER_ADDR0);
 	rcu_irq_enter_check_tick();
 	trace_hardirqs_off_finish();
+
+	mte_check_tfsr_entry();
 }
 
 /*
@@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
 {
 	lockdep_assert_irqs_disabled();
 
+	mte_check_tfsr_exit();
+
 	if (interrupts_enabled(regs)) {
 		if (regs->exit_rcu) {
 			trace_hardirqs_on_prepare();
@@ -243,6 +247,8 @@ asmlinkage void noinstr enter_from_user_mode(void)
 
 asmlinkage void noinstr exit_to_user_mode(void)
 {
+	mte_check_tfsr_exit();
+
 	trace_hardirqs_on_prepare();
 	lockdep_hardirqs_on_prepare(CALLER_ADDR0);
 	user_enter_irqoff();
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 92078e1eb627..7763ac1f2917 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -182,6 +182,37 @@ bool mte_report_once(void)
 	return READ_ONCE(report_fault_once);
 }
 
+#ifdef CONFIG_KASAN_HW_TAGS
+void mte_check_tfsr_el1(void)
+{
+	u64 tfsr_el1;
+
+	if (!system_supports_mte())
+		return;
+
+	tfsr_el1 = read_sysreg_s(SYS_TFSR_EL1);
+
+	/*
+	 * The kernel should never trigger an asynchronous fault on a
+	 * TTBR0 address, so we should never see TF0 set.
+	 * For futexes we disable checks via PSTATE.TCO.
+	 */
+	WARN_ONCE(tfsr_el1 & SYS_TFSR_EL1_TF0,
+		  "Kernel async tag fault on TTBR0 address");
+
+	if (unlikely(tfsr_el1 & SYS_TFSR_EL1_TF1)) {
+		/*
+		 * Note: isb() is not required after this direct write
+		 * because there is no indirect read subsequent to it
+		 * (per ARM DDI 0487F.c table D13-1).
+		 */
+		write_sysreg_s(0, SYS_TFSR_EL1);
+
+		kasan_report_async();
+	}
+}
+#endif
+
 static void update_sctlr_el1_tcf0(u64 tcf0)
 {
 	/* ISB required for the kernel uaccess routines */
@@ -247,6 +278,19 @@ void mte_thread_switch(struct task_struct *next)
 	/* avoid expensive SCTLR_EL1 accesses if no change */
 	if (current->thread.sctlr_tcf0 != next->thread.sctlr_tcf0)
 		update_sctlr_el1_tcf0(next->thread.sctlr_tcf0);
+	else
+		isb();
+
+	/*
+	 * Check if an async tag exception occurred at EL1.
+	 *
+	 * Note: On the context switch path we rely on the dsb() present
+	 * in __switch_to() to guarantee that the indirect writes to TFSR_EL1
+	 * are synchronized before this point.
+	 * isb() above is required for the same reason.
+	 *
+	 */
+	mte_check_tfsr_el1();
 }
 
 void mte_suspend_exit(void)
-- 
2.30.0


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

* Re: [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
  2021-01-26 13:45 [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
                   ` (3 preceding siblings ...)
  2021-01-26 13:46 ` [PATCH v9 4/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
@ 2021-01-27 20:00 ` Andrey Konovalov
  2021-01-27 22:19   ` Will Deacon
  4 siblings, 1 reply; 19+ messages in thread
From: Andrey Konovalov @ 2021-01-27 20:00 UTC (permalink / raw)
  To: Vincenzo Frascino, Andrew Morton
  Cc: Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov

On Tue, Jan 26, 2021 at 2:46 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> This patchset implements the asynchronous mode support for ARMv8.5-A
> Memory Tagging Extension (MTE), which is a debugging feature that allows
> to detect with the help of the architecture the C and C++ programmatic
> memory errors like buffer overflow, use-after-free, use-after-return, etc.
>
> MTE is built on top of the AArch64 v8.0 virtual address tagging TBI
> (Top Byte Ignore) feature and allows a task to set a 4 bit tag on any
> subset of its address space that is multiple of a 16 bytes granule. MTE
> is based on a lock-key mechanism where the lock is the tag associated to
> the physical memory and the key is the tag associated to the virtual
> address.
> When MTE is enabled and tags are set for ranges of address space of a task,
> the PE will compare the tag related to the physical memory with the tag
> related to the virtual address (tag check operation). Access to the memory
> is granted only if the two tags match. In case of mismatch the PE will raise
> an exception.
>
> The exception can be handled synchronously or asynchronously. When the
> asynchronous mode is enabled:
>   - Upon fault the PE updates the TFSR_EL1 register.
>   - The kernel detects the change during one of the following:
>     - Context switching
>     - Return to user/EL0
>     - Kernel entry from EL1
>     - Kernel exit to EL1
>   - If the register has been updated by the PE the kernel clears it and
>     reports the error.
>
> The series is based on linux-next/akpm.
>
> To simplify the testing a tree with the new patches on top has been made
> available at [1].
>
> [1] https://git.gitlab.arm.com/linux-arm/linux-vf.git mte/v10.async.akpm
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Marco Elver <elver@google.com>
> Cc: Evgenii Stepanov <eugenis@google.com>
> Cc: Branislav Rankov <Branislav.Rankov@arm.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

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

> Vincenzo Frascino (4):
>   arm64: mte: Add asynchronous mode support
>   kasan: Add KASAN mode kernel parameter
>   kasan: Add report for async mode
>   arm64: mte: Enable async tag check fault

Andrew, could you pick this up into mm? The whole series will need to
go through mm due to dependencies on the patches that are already
there.

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

* Re: [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
  2021-01-27 20:00 ` [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Andrey Konovalov
@ 2021-01-27 22:19   ` Will Deacon
  2021-01-29 17:54     ` Vincenzo Frascino
  0 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-01-27 22:19 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Vincenzo Frascino, Andrew Morton, Linux ARM, LKML, kasan-dev,
	Catalin Marinas, Dmitry Vyukov, Andrey Ryabinin,
	Alexander Potapenko, Marco Elver, Evgenii Stepanov,
	Branislav Rankov

On Wed, Jan 27, 2021 at 09:00:17PM +0100, Andrey Konovalov wrote:
> On Tue, Jan 26, 2021 at 2:46 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
> >
> > This patchset implements the asynchronous mode support for ARMv8.5-A
> > Memory Tagging Extension (MTE), which is a debugging feature that allows
> > to detect with the help of the architecture the C and C++ programmatic
> > memory errors like buffer overflow, use-after-free, use-after-return, etc.
> >
> > MTE is built on top of the AArch64 v8.0 virtual address tagging TBI
> > (Top Byte Ignore) feature and allows a task to set a 4 bit tag on any
> > subset of its address space that is multiple of a 16 bytes granule. MTE
> > is based on a lock-key mechanism where the lock is the tag associated to
> > the physical memory and the key is the tag associated to the virtual
> > address.
> > When MTE is enabled and tags are set for ranges of address space of a task,
> > the PE will compare the tag related to the physical memory with the tag
> > related to the virtual address (tag check operation). Access to the memory
> > is granted only if the two tags match. In case of mismatch the PE will raise
> > an exception.
> >
> > The exception can be handled synchronously or asynchronously. When the
> > asynchronous mode is enabled:
> >   - Upon fault the PE updates the TFSR_EL1 register.
> >   - The kernel detects the change during one of the following:
> >     - Context switching
> >     - Return to user/EL0
> >     - Kernel entry from EL1
> >     - Kernel exit to EL1
> >   - If the register has been updated by the PE the kernel clears it and
> >     reports the error.
> >
> > The series is based on linux-next/akpm.
> >
> > To simplify the testing a tree with the new patches on top has been made
> > available at [1].
> >
> > [1] https://git.gitlab.arm.com/linux-arm/linux-vf.git mte/v10.async.akpm
> >
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Dmitry Vyukov <dvyukov@google.com>
> > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> > Cc: Alexander Potapenko <glider@google.com>
> > Cc: Marco Elver <elver@google.com>
> > Cc: Evgenii Stepanov <eugenis@google.com>
> > Cc: Branislav Rankov <Branislav.Rankov@arm.com>
> > Cc: Andrey Konovalov <andreyknvl@google.com>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> 
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> 
> > Vincenzo Frascino (4):
> >   arm64: mte: Add asynchronous mode support
> >   kasan: Add KASAN mode kernel parameter
> >   kasan: Add report for async mode
> >   arm64: mte: Enable async tag check fault
> 
> Andrew, could you pick this up into mm? The whole series will need to
> go through mm due to dependencies on the patches that are already
> there.

Please can you check that it doesn't conflict with the arm64 for-next/core
branch first?

Will

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-26 13:46 ` [PATCH v9 3/4] kasan: Add report for async mode Vincenzo Frascino
@ 2021-01-29 17:40   ` Andrey Konovalov
  2021-01-29 17:48     ` Vincenzo Frascino
  2021-01-29 18:00     ` Vincenzo Frascino
  0 siblings, 2 replies; 19+ messages in thread
From: Andrey Konovalov @ 2021-01-29 17:40 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov

On Tue, Jan 26, 2021 at 2:46 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> KASAN provides an asynchronous mode of execution.
>
> Add reporting functionality for this mode.
>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Reviewed-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  include/linux/kasan.h |  6 ++++++
>  mm/kasan/report.c     | 13 +++++++++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index bb862d1f0e15..b6c502dad54d 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -360,6 +360,12 @@ static inline void *kasan_reset_tag(const void *addr)
>
>  #endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS*/
>
> +#ifdef CONFIG_KASAN_HW_TAGS
> +
> +void kasan_report_async(void);
> +
> +#endif /* CONFIG_KASAN_HW_TAGS */
> +
>  #ifdef CONFIG_KASAN_SW_TAGS
>  void __init kasan_init_sw_tags(void);
>  #else
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 87b271206163..69bad9c01aed 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -360,6 +360,19 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
>         end_report(&flags, (unsigned long)object);
>  }
>
> +#ifdef CONFIG_KASAN_HW_TAGS
> +void kasan_report_async(void)
> +{
> +       unsigned long flags;
> +
> +       start_report(&flags);
> +       pr_err("BUG: KASAN: invalid-access\n");
> +       pr_err("Asynchronous mode enabled: no access details available\n");
> +       dump_stack();
> +       end_report(&flags);

This conflicts with "kasan: use error_report_end tracepoint" that's in mm.

I suggest to call end_report(&flags, 0) here and check addr !=0 in
end_report() before calling trace_error_report_end().

> +}
> +#endif /* CONFIG_KASAN_HW_TAGS */
> +
>  static void __kasan_report(unsigned long addr, size_t size, bool is_write,
>                                 unsigned long ip)
>  {
> --
> 2.30.0
>

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 17:40   ` Andrey Konovalov
@ 2021-01-29 17:48     ` Vincenzo Frascino
  2021-01-29 17:56       ` Andrey Konovalov
  2021-01-29 18:00     ` Vincenzo Frascino
  1 sibling, 1 reply; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-29 17:48 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov



On 1/29/21 5:40 PM, Andrey Konovalov wrote:
> On Tue, Jan 26, 2021 at 2:46 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> KASAN provides an asynchronous mode of execution.
>>
>> Add reporting functionality for this mode.
>>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
>> Cc: Alexander Potapenko <glider@google.com>
>> Cc: Andrey Konovalov <andreyknvl@google.com>
>> Reviewed-by: Andrey Konovalov <andreyknvl@google.com>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> ---
>>  include/linux/kasan.h |  6 ++++++
>>  mm/kasan/report.c     | 13 +++++++++++++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>> index bb862d1f0e15..b6c502dad54d 100644
>> --- a/include/linux/kasan.h
>> +++ b/include/linux/kasan.h
>> @@ -360,6 +360,12 @@ static inline void *kasan_reset_tag(const void *addr)
>>
>>  #endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS*/
>>
>> +#ifdef CONFIG_KASAN_HW_TAGS
>> +
>> +void kasan_report_async(void);
>> +
>> +#endif /* CONFIG_KASAN_HW_TAGS */
>> +
>>  #ifdef CONFIG_KASAN_SW_TAGS
>>  void __init kasan_init_sw_tags(void);
>>  #else
>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>> index 87b271206163..69bad9c01aed 100644
>> --- a/mm/kasan/report.c
>> +++ b/mm/kasan/report.c
>> @@ -360,6 +360,19 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
>>         end_report(&flags, (unsigned long)object);
>>  }
>>
>> +#ifdef CONFIG_KASAN_HW_TAGS
>> +void kasan_report_async(void)
>> +{
>> +       unsigned long flags;
>> +
>> +       start_report(&flags);
>> +       pr_err("BUG: KASAN: invalid-access\n");
>> +       pr_err("Asynchronous mode enabled: no access details available\n");
>> +       dump_stack();
>> +       end_report(&flags);
> 
> This conflicts with "kasan: use error_report_end tracepoint" that's in mm.
> 
> I suggest to call end_report(&flags, 0) here and check addr !=0 in
> end_report() before calling trace_error_report_end().
> 

I just noticed and about to post a rebased version with end_report(&flags, 0).


>> +}
>> +#endif /* CONFIG_KASAN_HW_TAGS */
>> +
>>  static void __kasan_report(unsigned long addr, size_t size, bool is_write,
>>                                 unsigned long ip)
>>  {
>> --
>> 2.30.0
>>

-- 
Regards,
Vincenzo

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

* Re: [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
  2021-01-27 22:19   ` Will Deacon
@ 2021-01-29 17:54     ` Vincenzo Frascino
  0 siblings, 0 replies; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-29 17:54 UTC (permalink / raw)
  To: Will Deacon, Andrey Konovalov
  Cc: Andrew Morton, Linux ARM, LKML, kasan-dev, Catalin Marinas,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov

Hi Will,

On 1/27/21 10:19 PM, Will Deacon wrote:
>> Andrew, could you pick this up into mm? The whole series will need to
>> go through mm due to dependencies on the patches that are already
>> there.
> Please can you check that it doesn't conflict with the arm64 for-next/core
> branch first?

I just merged for-next/core and has of today there are no conflicts.

I notice though that with the introduction of:

ceca664b9581 ("kasan: use error_report_end tracepoint")

the interface of end_report() is changed hence my patches do not build anymore.

I will send v10 shortly.

-- 
Regards,
Vincenzo

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 17:48     ` Vincenzo Frascino
@ 2021-01-29 17:56       ` Andrey Konovalov
  2021-01-29 18:01         ` Vincenzo Frascino
  0 siblings, 1 reply; 19+ messages in thread
From: Andrey Konovalov @ 2021-01-29 17:56 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov

On Fri, Jan 29, 2021 at 6:44 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
>
>
> On 1/29/21 5:40 PM, Andrey Konovalov wrote:
> > On Tue, Jan 26, 2021 at 2:46 PM Vincenzo Frascino
> > <vincenzo.frascino@arm.com> wrote:
> >>
> >> KASAN provides an asynchronous mode of execution.
> >>
> >> Add reporting functionality for this mode.
> >>
> >> Cc: Dmitry Vyukov <dvyukov@google.com>
> >> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> >> Cc: Alexander Potapenko <glider@google.com>
> >> Cc: Andrey Konovalov <andreyknvl@google.com>
> >> Reviewed-by: Andrey Konovalov <andreyknvl@google.com>
> >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> >> ---
> >>  include/linux/kasan.h |  6 ++++++
> >>  mm/kasan/report.c     | 13 +++++++++++++
> >>  2 files changed, 19 insertions(+)
> >>
> >> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> >> index bb862d1f0e15..b6c502dad54d 100644
> >> --- a/include/linux/kasan.h
> >> +++ b/include/linux/kasan.h
> >> @@ -360,6 +360,12 @@ static inline void *kasan_reset_tag(const void *addr)
> >>
> >>  #endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS*/
> >>
> >> +#ifdef CONFIG_KASAN_HW_TAGS
> >> +
> >> +void kasan_report_async(void);
> >> +
> >> +#endif /* CONFIG_KASAN_HW_TAGS */
> >> +
> >>  #ifdef CONFIG_KASAN_SW_TAGS
> >>  void __init kasan_init_sw_tags(void);
> >>  #else
> >> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> >> index 87b271206163..69bad9c01aed 100644
> >> --- a/mm/kasan/report.c
> >> +++ b/mm/kasan/report.c
> >> @@ -360,6 +360,19 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
> >>         end_report(&flags, (unsigned long)object);
> >>  }
> >>
> >> +#ifdef CONFIG_KASAN_HW_TAGS
> >> +void kasan_report_async(void)
> >> +{
> >> +       unsigned long flags;
> >> +
> >> +       start_report(&flags);
> >> +       pr_err("BUG: KASAN: invalid-access\n");
> >> +       pr_err("Asynchronous mode enabled: no access details available\n");

Could you also add an empty line here before the stack trace while at it?

> >> +       dump_stack();
> >> +       end_report(&flags);
> >
> > This conflicts with "kasan: use error_report_end tracepoint" that's in mm.
> >
> > I suggest to call end_report(&flags, 0) here and check addr !=0 in
> > end_report() before calling trace_error_report_end().
> >
>
> I just noticed and about to post a rebased version with end_report(&flags, 0).
>
>
> >> +}
> >> +#endif /* CONFIG_KASAN_HW_TAGS */
> >> +
> >>  static void __kasan_report(unsigned long addr, size_t size, bool is_write,
> >>                                 unsigned long ip)
> >>  {
> >> --
> >> 2.30.0
> >>
>
> --
> Regards,
> Vincenzo

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 17:40   ` Andrey Konovalov
  2021-01-29 17:48     ` Vincenzo Frascino
@ 2021-01-29 18:00     ` Vincenzo Frascino
  2021-01-29 18:09       ` Andrey Konovalov
  1 sibling, 1 reply; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-29 18:00 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov

Hi Andrey,

On 1/29/21 5:40 PM, Andrey Konovalov wrote:
> I suggest to call end_report(&flags, 0) here and check addr !=0 in
> end_report() before calling trace_error_report_end().
> 

Probably this is better as:

if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))

Because that condition passes always addr == 0.

What do you think?

-- 
Regards,
Vincenzo

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 17:56       ` Andrey Konovalov
@ 2021-01-29 18:01         ` Vincenzo Frascino
  2021-01-29 18:10           ` Andrey Konovalov
  0 siblings, 1 reply; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-29 18:01 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov



On 1/29/21 5:56 PM, Andrey Konovalov wrote:
> On Fri, Jan 29, 2021 at 6:44 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>>
>>
>> On 1/29/21 5:40 PM, Andrey Konovalov wrote:
>>> On Tue, Jan 26, 2021 at 2:46 PM Vincenzo Frascino
>>> <vincenzo.frascino@arm.com> wrote:
>>>>
>>>> KASAN provides an asynchronous mode of execution.
>>>>
>>>> Add reporting functionality for this mode.
>>>>
>>>> Cc: Dmitry Vyukov <dvyukov@google.com>
>>>> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
>>>> Cc: Alexander Potapenko <glider@google.com>
>>>> Cc: Andrey Konovalov <andreyknvl@google.com>
>>>> Reviewed-by: Andrey Konovalov <andreyknvl@google.com>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>> ---
>>>>  include/linux/kasan.h |  6 ++++++
>>>>  mm/kasan/report.c     | 13 +++++++++++++
>>>>  2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>>>> index bb862d1f0e15..b6c502dad54d 100644
>>>> --- a/include/linux/kasan.h
>>>> +++ b/include/linux/kasan.h
>>>> @@ -360,6 +360,12 @@ static inline void *kasan_reset_tag(const void *addr)
>>>>
>>>>  #endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS*/
>>>>
>>>> +#ifdef CONFIG_KASAN_HW_TAGS
>>>> +
>>>> +void kasan_report_async(void);
>>>> +
>>>> +#endif /* CONFIG_KASAN_HW_TAGS */
>>>> +
>>>>  #ifdef CONFIG_KASAN_SW_TAGS
>>>>  void __init kasan_init_sw_tags(void);
>>>>  #else
>>>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>>>> index 87b271206163..69bad9c01aed 100644
>>>> --- a/mm/kasan/report.c
>>>> +++ b/mm/kasan/report.c
>>>> @@ -360,6 +360,19 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
>>>>         end_report(&flags, (unsigned long)object);
>>>>  }
>>>>
>>>> +#ifdef CONFIG_KASAN_HW_TAGS
>>>> +void kasan_report_async(void)
>>>> +{
>>>> +       unsigned long flags;
>>>> +
>>>> +       start_report(&flags);
>>>> +       pr_err("BUG: KASAN: invalid-access\n");
>>>> +       pr_err("Asynchronous mode enabled: no access details available\n");
> 
> Could you also add an empty line here before the stack trace while at it?
> 

Sure no problem.

>>>> +       dump_stack();
>>>> +       end_report(&flags);
>>>
>>> This conflicts with "kasan: use error_report_end tracepoint" that's in mm.
>>>
>>> I suggest to call end_report(&flags, 0) here and check addr !=0 in
>>> end_report() before calling trace_error_report_end().
>>>
>>
>> I just noticed and about to post a rebased version with end_report(&flags, 0).
>>
>>
>>>> +}
>>>> +#endif /* CONFIG_KASAN_HW_TAGS */
>>>> +
>>>>  static void __kasan_report(unsigned long addr, size_t size, bool is_write,
>>>>                                 unsigned long ip)
>>>>  {
>>>> --
>>>> 2.30.0
>>>>
>>
>> --
>> Regards,
>> Vincenzo

-- 
Regards,
Vincenzo

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 18:00     ` Vincenzo Frascino
@ 2021-01-29 18:09       ` Andrey Konovalov
  2021-01-29 18:16         ` Vincenzo Frascino
  0 siblings, 1 reply; 19+ messages in thread
From: Andrey Konovalov @ 2021-01-29 18:09 UTC (permalink / raw)
  To: Vincenzo Frascino, Alexander Potapenko
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Marco Elver,
	Evgenii Stepanov, Branislav Rankov

On Fri, Jan 29, 2021 at 6:56 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrey,
>
> On 1/29/21 5:40 PM, Andrey Konovalov wrote:
> > I suggest to call end_report(&flags, 0) here and check addr !=0 in
> > end_report() before calling trace_error_report_end().
> >
>
> Probably this is better as:
>
> if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
>
> Because that condition passes always addr == 0.

Not sure I understand. Call report_end(&flags, 0) and then there do:

if (addr) trace_error_report_end(...);

Although maybe it makes sense to still trace all async bugs to address
0. Or to some magic address.

Alex, WDYT?

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 18:01         ` Vincenzo Frascino
@ 2021-01-29 18:10           ` Andrey Konovalov
  2021-01-29 18:18             ` Vincenzo Frascino
  0 siblings, 1 reply; 19+ messages in thread
From: Andrey Konovalov @ 2021-01-29 18:10 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov

On Fri, Jan 29, 2021 at 6:57 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
> >>>> +#ifdef CONFIG_KASAN_HW_TAGS
> >>>> +void kasan_report_async(void)
> >>>> +{
> >>>> +       unsigned long flags;
> >>>> +
> >>>> +       start_report(&flags);
> >>>> +       pr_err("BUG: KASAN: invalid-access\n");
> >>>> +       pr_err("Asynchronous mode enabled: no access details available\n");
> >
> > Could you also add an empty line here before the stack trace while at it?
> >
>
> Sure no problem.

Just to be clear: I mean adding an empty line into the report itself
via pr_err("\n") :)

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 18:09       ` Andrey Konovalov
@ 2021-01-29 18:16         ` Vincenzo Frascino
  2021-01-29 18:46           ` Vincenzo Frascino
  0 siblings, 1 reply; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-29 18:16 UTC (permalink / raw)
  To: Andrey Konovalov, Alexander Potapenko
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Marco Elver,
	Evgenii Stepanov, Branislav Rankov



On 1/29/21 6:09 PM, Andrey Konovalov wrote:
> On Fri, Jan 29, 2021 at 6:56 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> Hi Andrey,
>>
>> On 1/29/21 5:40 PM, Andrey Konovalov wrote:
>>> I suggest to call end_report(&flags, 0) here and check addr !=0 in
>>> end_report() before calling trace_error_report_end().
>>>
>>
>> Probably this is better as:
>>
>> if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
>>
>> Because that condition passes always addr == 0.
> 
> Not sure I understand. Call report_end(&flags, 0) and then there do:
> 
> if (addr) trace_error_report_end(...);
> 
> Although maybe it makes sense to still trace all async bugs to address
> 0. Or to some magic address.
> 
> Alex, WDYT?
> 

What I meant is instead of:

if (addr) trace_error_report_end(...);

you might want to do:

if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS)) trace_error_report_end(...);

because, could make sense to trace 0 in other cases?

I could not find the implementation of trace_error_report_end() hence I am not
really sure on what it does.

-- 
Regards,
Vincenzo

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 18:10           ` Andrey Konovalov
@ 2021-01-29 18:18             ` Vincenzo Frascino
  0 siblings, 0 replies; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-29 18:18 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Linux ARM, LKML, kasan-dev, Andrew Morton, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov



On 1/29/21 6:10 PM, Andrey Konovalov wrote:
> On Fri, Jan 29, 2021 at 6:57 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>>>>> +#ifdef CONFIG_KASAN_HW_TAGS
>>>>>> +void kasan_report_async(void)
>>>>>> +{
>>>>>> +       unsigned long flags;
>>>>>> +
>>>>>> +       start_report(&flags);
>>>>>> +       pr_err("BUG: KASAN: invalid-access\n");
>>>>>> +       pr_err("Asynchronous mode enabled: no access details available\n");
>>>
>>> Could you also add an empty line here before the stack trace while at it?
>>>
>>
>> Sure no problem.
> 
> Just to be clear: I mean adding an empty line into the report itself
> via pr_err("\n") :)
> 

Yes I got it ;) It is late here but I am not completely asleep yet ;)

-- 
Regards,
Vincenzo

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 18:46           ` Vincenzo Frascino
@ 2021-01-29 18:44             ` Andrey Konovalov
  0 siblings, 0 replies; 19+ messages in thread
From: Andrey Konovalov @ 2021-01-29 18:44 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: Alexander Potapenko, Branislav Rankov, Marco Elver,
	Catalin Marinas, Evgenii Stepanov, LKML, kasan-dev,
	Dmitry Vyukov, Andrey Ryabinin, Andrew Morton, Will Deacon,
	Linux ARM

On Fri, Jan 29, 2021 at 7:42 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrey,
>
> On 1/29/21 6:16 PM, Vincenzo Frascino wrote:
> > What I meant is instead of:
> >
> > if (addr) trace_error_report_end(...);
> >
> > you might want to do:
> >
> > if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS)) trace_error_report_end(...);
> >
> > because, could make sense to trace 0 in other cases?
> >
> > I could not find the implementation of trace_error_report_end() hence I am not
> > really sure on what it does.
>
> I figured it out how trace_error_report_end() works.

It's intended for collecting crashes for CONFIG_KASAN_HW_TAGS.

> And in doing that I
> realized that the problem is sync vs async, hence I agree with what you are
> proposing:
>
> if (addr) trace_error_report_end(...);
>
> I will post v10 shortly. If we want to trace the async mode we can improve it in
> -rc1.

Sounds good, thanks!

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

* Re: [PATCH v9 3/4] kasan: Add report for async mode
  2021-01-29 18:16         ` Vincenzo Frascino
@ 2021-01-29 18:46           ` Vincenzo Frascino
  2021-01-29 18:44             ` Andrey Konovalov
  0 siblings, 1 reply; 19+ messages in thread
From: Vincenzo Frascino @ 2021-01-29 18:46 UTC (permalink / raw)
  To: Andrey Konovalov, Alexander Potapenko
  Cc: Branislav Rankov, Marco Elver, Catalin Marinas, Evgenii Stepanov,
	LKML, kasan-dev, Dmitry Vyukov, Andrey Ryabinin, Andrew Morton,
	Will Deacon, Linux ARM

Hi Andrey,

On 1/29/21 6:16 PM, Vincenzo Frascino wrote:
> What I meant is instead of:
> 
> if (addr) trace_error_report_end(...);
> 
> you might want to do:
> 
> if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS)) trace_error_report_end(...);
> 
> because, could make sense to trace 0 in other cases?
> 
> I could not find the implementation of trace_error_report_end() hence I am not
> really sure on what it does.

I figured it out how trace_error_report_end() works. And in doing that I
realized that the problem is sync vs async, hence I agree with what you are
proposing:

if (addr) trace_error_report_end(...);

I will post v10 shortly. If we want to trace the async mode we can improve it in
-rc1.

-- 
Regards,
Vincenzo

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

end of thread, other threads:[~2021-01-29 18:45 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 13:45 [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-01-26 13:46 ` [PATCH v9 1/4] arm64: mte: Add asynchronous " Vincenzo Frascino
2021-01-26 13:46 ` [PATCH v9 2/4] kasan: Add KASAN mode kernel parameter Vincenzo Frascino
2021-01-26 13:46 ` [PATCH v9 3/4] kasan: Add report for async mode Vincenzo Frascino
2021-01-29 17:40   ` Andrey Konovalov
2021-01-29 17:48     ` Vincenzo Frascino
2021-01-29 17:56       ` Andrey Konovalov
2021-01-29 18:01         ` Vincenzo Frascino
2021-01-29 18:10           ` Andrey Konovalov
2021-01-29 18:18             ` Vincenzo Frascino
2021-01-29 18:00     ` Vincenzo Frascino
2021-01-29 18:09       ` Andrey Konovalov
2021-01-29 18:16         ` Vincenzo Frascino
2021-01-29 18:46           ` Vincenzo Frascino
2021-01-29 18:44             ` Andrey Konovalov
2021-01-26 13:46 ` [PATCH v9 4/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
2021-01-27 20:00 ` [PATCH v9 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Andrey Konovalov
2021-01-27 22:19   ` Will Deacon
2021-01-29 17:54     ` Vincenzo Frascino

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).