[PATCH v13 0/7] arm64: ARMv8.5-A: MTE: Add async mode support

[PATCH v13 0/7] arm64: ARMv8.5-A: MTE: Add async mode support

[Vincenzo Frascino]

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/v11.async.akpm

Changes:
--------
v13:
  - Rebase on the latest linux-next/akpm.
  - Address review comments.
v12:
  - Fixed a bug affecting kernel functions allowed to read
    beyond buffer boundaries.
  - Added support for save/restore of TFSR_EL1 register
    during suspend/resume operations.
  - Rebased on latest linux-next/akpm.
v11:
  - Added patch that disables KUNIT tests in async mode
v10:
  - Rebase on the latest linux-next/akpm
  - Address review comments.
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>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

Andrey Konovalov (1):
  kasan: don't run tests in async mode

Vincenzo Frascino (6):
  arm64: mte: Add asynchronous mode support
  kasan: Add KASAN mode kernel parameter
  kasan: Add report for async mode
  arm64: mte: Enable TCO in functions that can read beyond buffer limits
  arm64: mte: Enable async tag check fault
  arm64: mte: Report async tag faults before suspend

 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            | 36 ++++++++++
 arch/arm64/include/asm/uaccess.h        | 24 +++++++
 arch/arm64/include/asm/word-at-a-time.h |  4 ++
 arch/arm64/kernel/entry-common.c        |  6 ++
 arch/arm64/kernel/mte.c                 | 91 ++++++++++++++++++++++++-
 arch/arm64/kernel/suspend.c             |  3 +
 include/linux/kasan.h                   |  6 ++
 lib/test_kasan.c                        |  6 +-
 mm/kasan/hw_tags.c                      | 52 +++++++++++++-
 mm/kasan/kasan.h                        |  7 +-
 mm/kasan/report.c                       | 17 ++++-
 14 files changed, 262 insertions(+), 11 deletions(-)

-- 
2.30.0

[PATCH v13 1/7] arm64: mte: Add asynchronous mode support

[Vincenzo Frascino]

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            | 19 ++++++++++++++++---
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index c759faf7a1ff..91515383d763 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -243,7 +243,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 7ab500e2ad17..4acf8bf41cad 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -77,7 +77,8 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
 	} while (curr != end);
 }
 
-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);
@@ -104,7 +105,11 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
 {
 }
 
-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 a66c2806fc4d..706b7ab75f31 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -107,13 +107,26 @@ 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);
+}
+EXPORT_SYMBOL_GPL(mte_enable_kernel_sync);
+
+void mte_enable_kernel_async(void)
+{
+	__mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
 }
-EXPORT_SYMBOL_GPL(mte_enable_kernel);
+EXPORT_SYMBOL_GPL(mte_enable_kernel_async);
 
 void mte_set_report_once(bool state)
 {
-- 
2.30.0

Re: [PATCH v13 1/7] arm64: mte: Add asynchronous mode support

[Andrey Konovalov]

On Thu, Feb 11, 2021 at 4:34 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> 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            | 19 ++++++++++++++++---
>  3 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index c759faf7a1ff..91515383d763 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -243,7 +243,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()

We need to update KASAN usage of arch_enable_tagging() to
arch_enable_tagging_sync() in this patch as well. Otherwise, this
leaves KASAN broken between this patch and the next one.


>  #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 7ab500e2ad17..4acf8bf41cad 100644
> --- a/arch/arm64/include/asm/mte-kasan.h
> +++ b/arch/arm64/include/asm/mte-kasan.h
> @@ -77,7 +77,8 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
>         } while (curr != end);
>  }
>
> -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);
> @@ -104,7 +105,11 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
>  {
>  }
>
> -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 a66c2806fc4d..706b7ab75f31 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -107,13 +107,26 @@ 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);
> +}
> +EXPORT_SYMBOL_GPL(mte_enable_kernel_sync);
> +
> +void mte_enable_kernel_async(void)
> +{
> +       __mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
>  }
> -EXPORT_SYMBOL_GPL(mte_enable_kernel);
> +EXPORT_SYMBOL_GPL(mte_enable_kernel_async);
>
>  void mte_set_report_once(bool state)
>  {
> --
> 2.30.0
>

Re: [PATCH v13 1/7] arm64: mte: Add asynchronous mode support

[Vincenzo Frascino]

On 2/12/21 9:21 PM, Andrey Konovalov wrote:
> On Thu, Feb 11, 2021 at 4:34 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> 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            | 19 ++++++++++++++++---
>>  3 files changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> index c759faf7a1ff..91515383d763 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -243,7 +243,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()
> 
> We need to update KASAN usage of arch_enable_tagging() to
> arch_enable_tagging_sync() in this patch as well. Otherwise, this
> leaves KASAN broken between this patch and the next one.
>

Yes you are right, still can't explain why it did bysect cleanly though.
I will introduce temporarily here:

#define arch_enable_tagging() arch_enable_tagging_sync()

and remove it again in the respective kasan patch.

> 
>>  #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 7ab500e2ad17..4acf8bf41cad 100644
>> --- a/arch/arm64/include/asm/mte-kasan.h
>> +++ b/arch/arm64/include/asm/mte-kasan.h
>> @@ -77,7 +77,8 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
>>         } while (curr != end);
>>  }
>>
>> -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);
>> @@ -104,7 +105,11 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
>>  {
>>  }
>>
>> -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 a66c2806fc4d..706b7ab75f31 100644
>> --- a/arch/arm64/kernel/mte.c
>> +++ b/arch/arm64/kernel/mte.c
>> @@ -107,13 +107,26 @@ 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);
>> +}
>> +EXPORT_SYMBOL_GPL(mte_enable_kernel_sync);
>> +
>> +void mte_enable_kernel_async(void)
>> +{
>> +       __mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
>>  }
>> -EXPORT_SYMBOL_GPL(mte_enable_kernel);
>> +EXPORT_SYMBOL_GPL(mte_enable_kernel_async);
>>
>>  void mte_set_report_once(bool state)
>>  {
>> --
>> 2.30.0
>>

-- 
Regards,
Vincenzo

[PATCH v13 2/7] kasan: Add KASAN mode kernel parameter

[Vincenzo Frascino]

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>
[ Add a new var instead of exposing kasan_arg_mode to be consistent with
  flags for other command line arguments. ]
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 Documentation/dev-tools/kasan.rst |  9 ++++++
 lib/test_kasan.c                  |  2 +-
 mm/kasan/hw_tags.c                | 52 ++++++++++++++++++++++++++++++-
 mm/kasan/kasan.h                  |  7 +++--
 4 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index ddf4239a5890..6f6ab3ed7b79 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``).
 
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 1328c468fdb5..f8c72d3aed64 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 1dfe4f62a89e..bd249d1f6cdc 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;
 
@@ -45,6 +52,10 @@ static enum kasan_arg_fault kasan_arg_fault __ro_after_init;
 DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
 EXPORT_SYMBOL(kasan_flag_enabled);
 
+/* Whether the asynchronous mode is enabled. */
+bool kasan_flag_async __ro_after_init;
+EXPORT_SYMBOL_GPL(kasan_flag_async);
+
 /* Whether to collect alloc/free stack traces. */
 DEFINE_STATIC_KEY_FALSE(kasan_flag_stacktrace);
 
@@ -68,6 +79,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 +141,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. */
@@ -132,6 +166,22 @@ void __init kasan_init_hw_tags(void)
 	/* Enable KASAN. */
 	static_branch_enable(&kasan_flag_enabled);
 
+	switch (kasan_arg_mode) {
+	case KASAN_ARG_MODE_DEFAULT:
+		/*
+		 * Default to sync mode.
+		 * Do nothing, kasan_flag_async keeps its default value.
+		 */
+		break;
+	case KASAN_ARG_MODE_SYNC:
+		/* Do nothing, kasan_flag_async keeps its default value. */
+		break;
+	case KASAN_ARG_MODE_ASYNC:
+		/* Async mode enabled. */
+		kasan_flag_async = true;
+		break;
+	}
+
 	switch (kasan_arg_stacktrace) {
 	case KASAN_ARG_STACKTRACE_DEFAULT:
 		/* Default to enabling stack trace collection. */
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index cc787ba47e1b..98f70ffc9e1c 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -21,6 +21,7 @@ static inline bool kasan_stack_collection_enabled(void)
 #endif
 
 extern bool kasan_flag_panic __ro_after_init;
+extern bool kasan_flag_async __ro_after_init;
 
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
 #define KASAN_GRANULE_SIZE	(1UL << KASAN_SHADOW_SCALE_SHIFT)
@@ -294,7 +295,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 +305,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

Re: [PATCH v13 2/7] kasan: Add KASAN mode kernel parameter

[Andrey Konovalov]

On Thu, Feb 11, 2021 at 4:34 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> 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>
> [ Add a new var instead of exposing kasan_arg_mode to be consistent with
>   flags for other command line arguments. ]
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
>  Documentation/dev-tools/kasan.rst |  9 ++++++
>  lib/test_kasan.c                  |  2 +-
>  mm/kasan/hw_tags.c                | 52 ++++++++++++++++++++++++++++++-
>  mm/kasan/kasan.h                  |  7 +++--
>  4 files changed, 66 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index ddf4239a5890..6f6ab3ed7b79 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``).
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 1328c468fdb5..f8c72d3aed64 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 1dfe4f62a89e..bd249d1f6cdc 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;
>
> @@ -45,6 +52,10 @@ static enum kasan_arg_fault kasan_arg_fault __ro_after_init;
>  DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
>  EXPORT_SYMBOL(kasan_flag_enabled);
>
> +/* Whether the asynchronous mode is enabled. */
> +bool kasan_flag_async __ro_after_init;
> +EXPORT_SYMBOL_GPL(kasan_flag_async);
> +
>  /* Whether to collect alloc/free stack traces. */
>  DEFINE_STATIC_KEY_FALSE(kasan_flag_stacktrace);
>
> @@ -68,6 +79,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"))

Let's default to KASAN_ARG_MODE_DEFAULT like for other args:

if (!arg)
  return -EINVAL;

kasan_init_hw_tags_cpu()/kasan_init_hw_tags() already handle
KASAN_ARG_MODE_DEFAULT properly.

> +               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 +141,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. */
> @@ -132,6 +166,22 @@ void __init kasan_init_hw_tags(void)
>         /* Enable KASAN. */
>         static_branch_enable(&kasan_flag_enabled);
>
> +       switch (kasan_arg_mode) {
> +       case KASAN_ARG_MODE_DEFAULT:
> +               /*
> +                * Default to sync mode.
> +                * Do nothing, kasan_flag_async keeps its default value.
> +                */
> +               break;
> +       case KASAN_ARG_MODE_SYNC:
> +               /* Do nothing, kasan_flag_async keeps its default value. */
> +               break;
> +       case KASAN_ARG_MODE_ASYNC:
> +               /* Async mode enabled. */
> +               kasan_flag_async = true;
> +               break;
> +       }
> +
>         switch (kasan_arg_stacktrace) {
>         case KASAN_ARG_STACKTRACE_DEFAULT:
>                 /* Default to enabling stack trace collection. */
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index cc787ba47e1b..98f70ffc9e1c 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -21,6 +21,7 @@ static inline bool kasan_stack_collection_enabled(void)
>  #endif
>
>  extern bool kasan_flag_panic __ro_after_init;
> +extern bool kasan_flag_async __ro_after_init;
>
>  #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
>  #define KASAN_GRANULE_SIZE     (1UL << KASAN_SHADOW_SCALE_SHIFT)
> @@ -294,7 +295,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 +305,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
>

Re: [PATCH v13 2/7] kasan: Add KASAN mode kernel parameter

[Vincenzo Frascino]

On 2/11/21 5:50 PM, Andrey Konovalov wrote:
> Let's default to KASAN_ARG_MODE_DEFAULT like for other args:
> 
> if (!arg)
>   return -EINVAL;
> 
> kasan_init_hw_tags_cpu()/kasan_init_hw_tags() already handle
> KASAN_ARG_MODE_DEFAULT properly.

Ok, no problem, I will take care of it in the next version.

-- 
Regards,
Vincenzo

[PATCH v13 3/7] kasan: Add report for async mode

[Vincenzo Frascino]

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>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 include/linux/kasan.h |  6 ++++++
 mm/kasan/report.c     | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 14f72ec96492..d53ea3c047bc 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -376,6 +376,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..f147633f1f2b 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -87,7 +87,8 @@ static void start_report(unsigned long *flags)
 
 static void end_report(unsigned long *flags, unsigned long addr)
 {
-	trace_error_report_end(ERROR_DETECTOR_KASAN, addr);
+	if (!kasan_flag_async)
+		trace_error_report_end(ERROR_DETECTOR_KASAN, addr);
 	pr_err("==================================================================\n");
 	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
 	spin_unlock_irqrestore(&report_lock, *flags);
@@ -360,6 +361,20 @@ 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");
+	pr_err("\n");
+	dump_stack();
+	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

Re: [PATCH v13 3/7] kasan: Add report for async mode

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

Hi Vincenzo,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20210211]
[cannot apply to arm64/for-next/core xlnx/master arm/for-next soc/for-next kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc7 v5.11-rc6 v5.11-rc5 v5.11-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
base:    671176b0016c80b3943cb5387312c886aba3308d
config: riscv-randconfig-s031-20210209 (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-215-g0fb77bb6-dirty
        # https://github.com/0day-ci/linux/commit/df25c9583cd523a49f2407e0aeee55bdec24a14e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
        git checkout df25c9583cd523a49f2407e0aeee55bdec24a14e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   riscv64-linux-ld: mm/kasan/report.o: in function `.L0 ':
   report.c:(.text+0x480): undefined reference to `kasan_flag_async'
>> riscv64-linux-ld: report.c:(.text+0x5c4): undefined reference to `kasan_flag_async'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38226 bytes --]

Re: [PATCH v13 3/7] kasan: Add report for async mode

[Andrey Konovalov]

On Thu, Feb 11, 2021 at 9:04 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Vincenzo,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on next-20210211]
> [cannot apply to arm64/for-next/core xlnx/master arm/for-next soc/for-next kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc7 v5.11-rc6 v5.11-rc5 v5.11-rc7]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
> base:    671176b0016c80b3943cb5387312c886aba3308d
> config: riscv-randconfig-s031-20210209 (attached as .config)
> compiler: riscv64-linux-gcc (GCC) 9.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.3-215-g0fb77bb6-dirty
>         # https://github.com/0day-ci/linux/commit/df25c9583cd523a49f2407e0aeee55bdec24a14e
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
>         git checkout df25c9583cd523a49f2407e0aeee55bdec24a14e
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=riscv
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    riscv64-linux-ld: mm/kasan/report.o: in function `.L0 ':
>    report.c:(.text+0x480): undefined reference to `kasan_flag_async'
> >> riscv64-linux-ld: report.c:(.text+0x5c4): undefined reference to `kasan_flag_async'

Let's do something like this (untested):

https://github.com/xairy/linux/commit/91354d34b30ceedbc1b6417f1ff253de90618a97

Re: [PATCH v13 3/7] kasan: Add report for async mode

[Vincenzo Frascino]

Hi Andrey,

On 2/11/21 8:13 PM, Andrey Konovalov wrote:
>>>> riscv64-linux-ld: report.c:(.text+0x5c4): undefined reference to `kasan_flag_async'
> Let's do something like this (untested):
> 
> https://github.com/xairy/linux/commit/91354d34b30ceedbc1b6417f1ff253de90618a97

Could you reproduce this? I tried yesterday before posting the patches and my
conclusion was that kbuild robot is testing on the wrong tree.

I give it another go today, if you have more details based on your testing feel
free to share.

-- 
Regards,
Vincenzo

Re: [PATCH v13 3/7] kasan: Add report for async mode

[Andrey Konovalov]

On Fri, Feb 12, 2021 at 12:21 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrey,
>
> On 2/11/21 8:13 PM, Andrey Konovalov wrote:
> >>>> riscv64-linux-ld: report.c:(.text+0x5c4): undefined reference to `kasan_flag_async'
> > Let's do something like this (untested):
> >
> > https://github.com/xairy/linux/commit/91354d34b30ceedbc1b6417f1ff253de90618a97
>
> Could you reproduce this? I tried yesterday before posting the patches and my
> conclusion was that kbuild robot is testing on the wrong tree.
>
> I give it another go today, if you have more details based on your testing feel
> free to share.

I haven't tried reproducing, but the error makes sense. There's no
definition of kasan_flag_async for KASAN modes other than HW_TAGS.

Re: [PATCH v13 3/7] kasan: Add report for async mode

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

Hi Vincenzo,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20210211]
[cannot apply to arm64/for-next/core xlnx/master arm/for-next soc/for-next kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc7 v5.11-rc6 v5.11-rc5 v5.11-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
base:    671176b0016c80b3943cb5387312c886aba3308d
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/df25c9583cd523a49f2407e0aeee55bdec24a14e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
        git checkout df25c9583cd523a49f2407e0aeee55bdec24a14e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   aarch64-linux-ld: mm/kasan/report.o: in function `end_report':
   report.c:(.text+0x18): undefined reference to `kasan_flag_async'
   aarch64-linux-ld: mm/kasan/report.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `kasan_flag_async' which may bind externally can not be used when making a shared object; recompile with -fPIC
   report.c:(.text+0x18): dangerous relocation: unsupported relocation
>> aarch64-linux-ld: report.c:(.text+0x1c): undefined reference to `kasan_flag_async'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 76858 bytes --]

[PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Vincenzo Frascino]

load_unaligned_zeropad() and __get/put_kernel_nofault() functions can
read passed some buffer limits which may include some MTE granule with a
different tag.

When MTE async mode is enable, the load operation crosses the boundaries
and the next granule has a different tag the PE sets the TFSR_EL1.TF1 bit
as if an asynchronous tag fault is happened.

Enable Tag Check Override (TCO) in these functions  before the load and
disable it afterwards to prevent this to happen.

Note: The same condition can be hit in MTE sync mode but we deal with it
through the exception handling.
In the current implementation, mte_async_mode flag is set only at boot
time but in future kasan might acquire some runtime features that
that change the mode dynamically, hence we disable it when sync mode is
selected for future proof.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Reported-by: Branislav Rankov <Branislav.Rankov@arm.com>
Tested-by: Branislav Rankov <Branislav.Rankov@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/uaccess.h        | 24 ++++++++++++++++++++++++
 arch/arm64/include/asm/word-at-a-time.h |  4 ++++
 arch/arm64/kernel/mte.c                 | 16 ++++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 0deb88467111..a857f8f82aeb 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -188,6 +188,26 @@ static inline void __uaccess_enable_tco(void)
 				 ARM64_MTE, CONFIG_KASAN_HW_TAGS));
 }
 
+/* Whether the MTE asynchronous mode is enabled. */
+DECLARE_STATIC_KEY_FALSE(mte_async_mode);
+
+/*
+ * These functions disable tag checking only if in MTE async mode
+ * since the sync mode generates exceptions synchronously and the
+ * nofault or load_unaligned_zeropad can handle them.
+ */
+static inline void __uaccess_disable_tco_async(void)
+{
+	if (static_branch_unlikely(&mte_async_mode))
+		 __uaccess_disable_tco();
+}
+
+static inline void __uaccess_enable_tco_async(void)
+{
+	if (static_branch_unlikely(&mte_async_mode))
+		__uaccess_enable_tco();
+}
+
 static inline void uaccess_disable_privileged(void)
 {
 	__uaccess_disable_tco();
@@ -307,8 +327,10 @@ do {									\
 do {									\
 	int __gkn_err = 0;						\
 									\
+	__uaccess_enable_tco_async();					\
 	__raw_get_mem("ldr", *((type *)(dst)),				\
 		      (__force type *)(src), __gkn_err);		\
+	__uaccess_disable_tco_async();					\
 	if (unlikely(__gkn_err))					\
 		goto err_label;						\
 } while (0)
@@ -380,8 +402,10 @@ do {									\
 do {									\
 	int __pkn_err = 0;						\
 									\
+	__uaccess_enable_tco_async();					\
 	__raw_put_mem("str", *((type *)(src)),				\
 		      (__force type *)(dst), __pkn_err);		\
+	__uaccess_disable_tco_async();					\
 	if (unlikely(__pkn_err))					\
 		goto err_label;						\
 } while(0)
diff --git a/arch/arm64/include/asm/word-at-a-time.h b/arch/arm64/include/asm/word-at-a-time.h
index 3333950b5909..c62d9fa791aa 100644
--- a/arch/arm64/include/asm/word-at-a-time.h
+++ b/arch/arm64/include/asm/word-at-a-time.h
@@ -55,6 +55,8 @@ static inline unsigned long load_unaligned_zeropad(const void *addr)
 {
 	unsigned long ret, offset;
 
+	__uaccess_enable_tco_async();
+
 	/* Load word from unaligned pointer addr */
 	asm(
 	"1:	ldr	%0, %3\n"
@@ -76,6 +78,8 @@ static inline unsigned long load_unaligned_zeropad(const void *addr)
 	: "=&r" (ret), "=&r" (offset)
 	: "r" (addr), "Q" (*(unsigned long *)addr));
 
+	__uaccess_disable_tco_async();
+
 	return ret;
 }
 
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 706b7ab75f31..65ecb86dd886 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -26,6 +26,10 @@ u64 gcr_kernel_excl __ro_after_init;
 
 static bool report_fault_once = true;
 
+/* Whether the MTE asynchronous mode is enabled. */
+DEFINE_STATIC_KEY_FALSE(mte_async_mode);
+EXPORT_SYMBOL_GPL(mte_async_mode);
+
 static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool check_swap)
 {
 	pte_t old_pte = READ_ONCE(*ptep);
@@ -119,12 +123,24 @@ static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
 void mte_enable_kernel_sync(void)
 {
 	__mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC);
+
+	/*
+	 * This function is called on each active smp core at boot
+	 * time, hence we do not need to take cpu_hotplug_lock again.
+	 */
+	static_branch_disable_cpuslocked(&mte_async_mode);
 }
 EXPORT_SYMBOL_GPL(mte_enable_kernel_sync);
 
 void mte_enable_kernel_async(void)
 {
 	__mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
+
+	/*
+	 * This function is called on each active smp core at boot
+	 * time, hence we do not need to take cpu_hotplug_lock again.
+	 */
+	static_branch_enable_cpuslocked(&mte_async_mode);
 }
 EXPORT_SYMBOL_GPL(mte_enable_kernel_async);
 
-- 
2.30.0

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Catalin Marinas]

On Thu, Feb 11, 2021 at 03:33:50PM +0000, Vincenzo Frascino wrote:
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 706b7ab75f31..65ecb86dd886 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -26,6 +26,10 @@ u64 gcr_kernel_excl __ro_after_init;
>  
>  static bool report_fault_once = true;
>  
> +/* Whether the MTE asynchronous mode is enabled. */
> +DEFINE_STATIC_KEY_FALSE(mte_async_mode);
> +EXPORT_SYMBOL_GPL(mte_async_mode);
> +
>  static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool check_swap)
>  {
>  	pte_t old_pte = READ_ONCE(*ptep);
> @@ -119,12 +123,24 @@ static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
>  void mte_enable_kernel_sync(void)
>  {
>  	__mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC);
> +
> +	/*
> +	 * This function is called on each active smp core at boot
> +	 * time, hence we do not need to take cpu_hotplug_lock again.
> +	 */
> +	static_branch_disable_cpuslocked(&mte_async_mode);
>  }
>  EXPORT_SYMBOL_GPL(mte_enable_kernel_sync);
>  
>  void mte_enable_kernel_async(void)
>  {
>  	__mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
> +
> +	/*
> +	 * This function is called on each active smp core at boot
> +	 * time, hence we do not need to take cpu_hotplug_lock again.
> +	 */
> +	static_branch_enable_cpuslocked(&mte_async_mode);
>  }

Sorry, I missed the cpuslocked aspect before. Is there any reason you
need to use this API here? I suggested to add it to the
mte_enable_kernel_sync() because kasan may at some point do this
dynamically at run-time, so the boot-time argument doesn't hold. But
it's also incorrect as this function will be called for hot-plugged
CPUs as well after boot.

The only reason for static_branch_*_cpuslocked() is if it's called from
a region that already invoked cpus_read_lock() which I don't think is
the case here.

-- 
Catalin

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Vincenzo Frascino]

On 2/12/21 5:21 PM, Catalin Marinas wrote:
>> +
>> +	/*
>> +	 * This function is called on each active smp core at boot
>> +	 * time, hence we do not need to take cpu_hotplug_lock again.
>> +	 */
>> +	static_branch_enable_cpuslocked(&mte_async_mode);
>>  }
> Sorry, I missed the cpuslocked aspect before. Is there any reason you
> need to use this API here? I suggested to add it to the
> mte_enable_kernel_sync() because kasan may at some point do this
> dynamically at run-time, so the boot-time argument doesn't hold. But
> it's also incorrect as this function will be called for hot-plugged
> CPUs as well after boot.
> 
> The only reason for static_branch_*_cpuslocked() is if it's called from
> a region that already invoked cpus_read_lock() which I don't think is
> the case here.

I agree with your analysis on why static_branch_*_cpuslocked() is needed, in
fact cpus_read_lock() takes cpu_hotplug_lock as per comment on top of the line
of code.

If I try to take that lock when enabling the secondary cores I end up in the
situation below:

[    0.283402] smp: Bringing up secondary CPUs ...
....
[    5.890963] Call trace:
[    5.891050]  dump_backtrace+0x0/0x19c
[    5.891212]  show_stack+0x18/0x70
[    5.891373]  dump_stack+0xd0/0x12c
[    5.891531]  dequeue_task_idle+0x28/0x40
[    5.891686]  __schedule+0x45c/0x6c0
[    5.891851]  schedule+0x70/0x104
[    5.892010]  percpu_rwsem_wait+0xe8/0x104
[    5.892174]  __percpu_down_read+0x5c/0x90
[    5.892332]  percpu_down_read.constprop.0+0xbc/0xd4
[    5.892497]  cpus_read_lock+0x10/0x1c
[    5.892660]  static_key_enable+0x18/0x3c
[    5.892823]  mte_enable_kernel_async+0x40/0x70
[    5.892988]  kasan_init_hw_tags_cpu+0x50/0x60
[    5.893144]  cpu_enable_mte+0x24/0x70
[    5.893304]  verify_local_cpu_caps+0x58/0x120
[    5.893465]  check_local_cpu_capabilities+0x18/0x1f0
[    5.893626]  secondary_start_kernel+0xe0/0x190
[    5.893790]  0x0
[    5.893975] bad: scheduling from the idle thread!
[    5.894065] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G        W
5.11.0-rc7-10587-g22cd50bcfcf-dirty #6

and the kernel panics.

Note: there is a look of msg drop in between enabling the secondary and the
first clean stack trace.

-- 
Regards,
Vincenzo

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Catalin Marinas]

On Mon, Feb 22, 2021 at 12:08:07PM +0000, Vincenzo Frascino wrote:
> On 2/12/21 5:21 PM, Catalin Marinas wrote:
> >> +
> >> +	/*
> >> +	 * This function is called on each active smp core at boot
> >> +	 * time, hence we do not need to take cpu_hotplug_lock again.
> >> +	 */
> >> +	static_branch_enable_cpuslocked(&mte_async_mode);
> >>  }
> > Sorry, I missed the cpuslocked aspect before. Is there any reason you
> > need to use this API here? I suggested to add it to the
> > mte_enable_kernel_sync() because kasan may at some point do this
> > dynamically at run-time, so the boot-time argument doesn't hold. But
> > it's also incorrect as this function will be called for hot-plugged
> > CPUs as well after boot.
> > 
> > The only reason for static_branch_*_cpuslocked() is if it's called from
> > a region that already invoked cpus_read_lock() which I don't think is
> > the case here.
> 
> I agree with your analysis on why static_branch_*_cpuslocked() is needed, in
> fact cpus_read_lock() takes cpu_hotplug_lock as per comment on top of the line
> of code.
> 
> If I try to take that lock when enabling the secondary cores I end up in the
> situation below:
> 
> [    0.283402] smp: Bringing up secondary CPUs ...
> ....
> [    5.890963] Call trace:
> [    5.891050]  dump_backtrace+0x0/0x19c
> [    5.891212]  show_stack+0x18/0x70
> [    5.891373]  dump_stack+0xd0/0x12c
> [    5.891531]  dequeue_task_idle+0x28/0x40
> [    5.891686]  __schedule+0x45c/0x6c0
> [    5.891851]  schedule+0x70/0x104
> [    5.892010]  percpu_rwsem_wait+0xe8/0x104
> [    5.892174]  __percpu_down_read+0x5c/0x90
> [    5.892332]  percpu_down_read.constprop.0+0xbc/0xd4
> [    5.892497]  cpus_read_lock+0x10/0x1c
> [    5.892660]  static_key_enable+0x18/0x3c
> [    5.892823]  mte_enable_kernel_async+0x40/0x70
> [    5.892988]  kasan_init_hw_tags_cpu+0x50/0x60
> [    5.893144]  cpu_enable_mte+0x24/0x70
> [    5.893304]  verify_local_cpu_caps+0x58/0x120
> [    5.893465]  check_local_cpu_capabilities+0x18/0x1f0
> [    5.893626]  secondary_start_kernel+0xe0/0x190
> [    5.893790]  0x0
> [    5.893975] bad: scheduling from the idle thread!
> [    5.894065] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G        W
> 5.11.0-rc7-10587-g22cd50bcfcf-dirty #6
> 
> and the kernel panics.

That's because cpu_hotplug_lock is not a spinlock but a semaphore which
implies sleeping. I don't think avoiding taking this semaphore
altogether (as in the *_cpuslocked functions) is the correct workaround.

The mte_enable_kernel_async() function is called on each secondary CPU
but we don't really need to attempt to toggle the static key on each of
them as they all have the same configuration. Maybe do something like:

	if (!static_branch_unlikely(&mte_async_mode)))
		static_branch_enable(&mte_async_mode);

so that it's only set on the boot CPU.

The alternative is to use a per-CPU mask/variable instead of static
branches but it's probably too expensive for those functions that were
meant to improve performance.

We'll still have an issue with dynamically switching the async/sync mode
at run-time. Luckily kasan doesn't do this now. The problem is that
until the last CPU have been switched from async to sync, we can't
toggle the static label. When switching from sync to async, we need
to do it on the first CPU being switched.

So, I think currently we can set the mte_async_mode label to true in
mte_enable_kernel_async(), with the 'if' check above. For
mte_enable_kernel_sync(), don't bother with setting the key to false but
place a WARN_ONCE if the mte_async_mode is true. We can revisit it if
kasan ever gains this run-time switch mode.

-- 
Catalin

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Vincenzo Frascino]

On 2/22/21 5:58 PM, Catalin Marinas wrote:
> That's because cpu_hotplug_lock is not a spinlock but a semaphore which
> implies sleeping. I don't think avoiding taking this semaphore
> altogether (as in the *_cpuslocked functions) is the correct workaround.
>

Thinking at it a second time I agree, it is not a good idea avoiding to take the
semaphore in this case.

> The mte_enable_kernel_async() function is called on each secondary CPU
> but we don't really need to attempt to toggle the static key on each of
> them as they all have the same configuration. Maybe do something like:
> 
> 	if (!static_branch_unlikely(&mte_async_mode)))
> 		static_branch_enable(&mte_async_mode);
> 
> so that it's only set on the boot CPU.
> 

This should work, maybe with a comment that if we plan to introduce runtime
switching in between async and sync in future we need to revisit our strategy.

> The alternative is to use a per-CPU mask/variable instead of static
> branches but it's probably too expensive for those functions that were
> meant to improve performance.
> 

I would not go for this approach because the reason why we introduced static
branches instead of having a normal variable saving the state was performances.

> We'll still have an issue with dynamically switching the async/sync mode
> at run-time. Luckily kasan doesn't do this now. The problem is that
> until the last CPU have been switched from async to sync, we can't
> toggle the static label. When switching from sync to async, we need
> to do it on the first CPU being switched.
> 

I totally agree on this point. In the case of runtime switching we might need
the rethink completely the strategy and depends a lot on what we want to allow
and what not. For the kernel I imagine we will need to expose something in sysfs
that affects all the cores and then maybe stop_machine() to propagate it to all
the cores. Do you think having some of the cores running in sync mode and some
in async is a viable solution?

Probably it is worth to discuss it further once we cross that bridge.

> So, I think currently we can set the mte_async_mode label to true in
> mte_enable_kernel_async(), with the 'if' check above. For
> mte_enable_kernel_sync(), don't bother with setting the key to false but
> place a WARN_ONCE if the mte_async_mode is true. We can revisit it if
> kasan ever gains this run-time switch mode.

Indeed, this should work for now.

-- 
Regards,
Vincenzo

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Catalin Marinas]

On Tue, Feb 23, 2021 at 10:56:46AM +0000, Vincenzo Frascino wrote:
> On 2/22/21 5:58 PM, Catalin Marinas wrote:
> > We'll still have an issue with dynamically switching the async/sync mode
> > at run-time. Luckily kasan doesn't do this now. The problem is that
> > until the last CPU have been switched from async to sync, we can't
> > toggle the static label. When switching from sync to async, we need
> > to do it on the first CPU being switched.
> 
> I totally agree on this point. In the case of runtime switching we might need
> the rethink completely the strategy and depends a lot on what we want to allow
> and what not. For the kernel I imagine we will need to expose something in sysfs
> that affects all the cores and then maybe stop_machine() to propagate it to all
> the cores. Do you think having some of the cores running in sync mode and some
> in async is a viable solution?

stop_machine() is an option indeed. I think it's still possible to run
some cores in async while others in sync but the static key here would
only be toggled when no async CPUs are left.

> Probably it is worth to discuss it further once we cross that bridge.

Yes. For now, a warning should do so that we don't forget.

-- 
Catalin

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Vincenzo Frascino]

On 2/23/21 12:05 PM, Catalin Marinas wrote:
>> I totally agree on this point. In the case of runtime switching we might need
>> the rethink completely the strategy and depends a lot on what we want to allow
>> and what not. For the kernel I imagine we will need to expose something in sysfs
>> that affects all the cores and then maybe stop_machine() to propagate it to all
>> the cores. Do you think having some of the cores running in sync mode and some
>> in async is a viable solution?
> stop_machine() is an option indeed. I think it's still possible to run
> some cores in async while others in sync but the static key here would
> only be toggled when no async CPUs are left.
> 

In such a case we might need to track the state based on cpuid() and have a mask
that tells us when cpus are all sync.
Not as expensive as stop_machine() but still requires a valid use case to be
introduced according to me.

-- 
Regards,
Vincenzo

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Will Deacon]

On Tue, Feb 23, 2021 at 12:05:32PM +0000, Catalin Marinas wrote:
> On Tue, Feb 23, 2021 at 10:56:46AM +0000, Vincenzo Frascino wrote:
> > On 2/22/21 5:58 PM, Catalin Marinas wrote:
> > > We'll still have an issue with dynamically switching the async/sync mode
> > > at run-time. Luckily kasan doesn't do this now. The problem is that
> > > until the last CPU have been switched from async to sync, we can't
> > > toggle the static label. When switching from sync to async, we need
> > > to do it on the first CPU being switched.
> > 
> > I totally agree on this point. In the case of runtime switching we might need
> > the rethink completely the strategy and depends a lot on what we want to allow
> > and what not. For the kernel I imagine we will need to expose something in sysfs
> > that affects all the cores and then maybe stop_machine() to propagate it to all
> > the cores. Do you think having some of the cores running in sync mode and some
> > in async is a viable solution?
> 
> stop_machine() is an option indeed. I think it's still possible to run
> some cores in async while others in sync but the static key here would
> only be toggled when no async CPUs are left.

Just as a general point, but if we expose stop_machine() via sysfs we
probably want to limit that to privileged users so you can't DoS the system
by spamming into the file.

Will

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Catalin Marinas]

On Tue, Feb 23, 2021 at 12:49:52PM +0000, Will Deacon wrote:
> On Tue, Feb 23, 2021 at 12:05:32PM +0000, Catalin Marinas wrote:
> > On Tue, Feb 23, 2021 at 10:56:46AM +0000, Vincenzo Frascino wrote:
> > > On 2/22/21 5:58 PM, Catalin Marinas wrote:
> > > > We'll still have an issue with dynamically switching the async/sync mode
> > > > at run-time. Luckily kasan doesn't do this now. The problem is that
> > > > until the last CPU have been switched from async to sync, we can't
> > > > toggle the static label. When switching from sync to async, we need
> > > > to do it on the first CPU being switched.
> > > 
> > > I totally agree on this point. In the case of runtime switching we might need
> > > the rethink completely the strategy and depends a lot on what we want to allow
> > > and what not. For the kernel I imagine we will need to expose something in sysfs
> > > that affects all the cores and then maybe stop_machine() to propagate it to all
> > > the cores. Do you think having some of the cores running in sync mode and some
> > > in async is a viable solution?
> > 
> > stop_machine() is an option indeed. I think it's still possible to run
> > some cores in async while others in sync but the static key here would
> > only be toggled when no async CPUs are left.
> 
> Just as a general point, but if we expose stop_machine() via sysfs we
> probably want to limit that to privileged users so you can't DoS the system
> by spamming into the file.

Definitely. Anyway, that's a later kasan feature if they'd find it
useful. Currently the mode is set at boot from cmdline.

-- 
Catalin

Re: [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits

[Vincenzo Frascino]

On 2/23/21 12:49 PM, Will Deacon wrote:
>>> I totally agree on this point. In the case of runtime switching we might need
>>> the rethink completely the strategy and depends a lot on what we want to allow
>>> and what not. For the kernel I imagine we will need to expose something in sysfs
>>> that affects all the cores and then maybe stop_machine() to propagate it to all
>>> the cores. Do you think having some of the cores running in sync mode and some
>>> in async is a viable solution?
>> stop_machine() is an option indeed. I think it's still possible to run
>> some cores in async while others in sync but the static key here would
>> only be toggled when no async CPUs are left.
> Just as a general point, but if we expose stop_machine() via sysfs we
> probably want to limit that to privileged users so you can't DoS the system
> by spamming into the file.

I agree, if we ever introduce the runtime switching and go for this option we
should make sure that we do it safely.

-- 
Regards,
Vincenzo

[PATCH v13 5/7] arm64: mte: Enable async tag check fault

[Vincenzo Frascino]

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          | 36 ++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index 9b557a457f24..43169b978cd3 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -90,5 +90,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 9d3588450473..a1ec351c36bd 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();
@@ -293,6 +297,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 65ecb86dd886..f5aa5bea6dfe 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -155,6 +155,29 @@ 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);
+
+	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 */
@@ -220,6 +243,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

[PATCH v13 6/7] arm64: mte: Report async tag faults before suspend

[Vincenzo Frascino]

When MTE async mode is enabled TFSR_EL1 contains the accumulative
asynchronous tag check faults for EL1 and EL0.

During the suspend/resume operations the firmware might perform some
operations that could change the state of the register resulting in
a spurious tag check fault report.

Report asynchronous tag faults before suspend and clear the TFSR_EL1
register after resume to prevent this to happen.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/mte.h |  4 ++++
 arch/arm64/kernel/mte.c      | 20 ++++++++++++++++++++
 arch/arm64/kernel/suspend.c  |  3 +++
 3 files changed, 27 insertions(+)

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index 43169b978cd3..33e88a470357 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -41,6 +41,7 @@ void mte_sync_tags(pte_t *ptep, pte_t pte);
 void mte_copy_page_tags(void *kto, const void *kfrom);
 void flush_mte_state(void);
 void mte_thread_switch(struct task_struct *next);
+void mte_suspend_enter(void);
 void mte_suspend_exit(void);
 long set_mte_ctrl(struct task_struct *task, unsigned long arg);
 long get_mte_ctrl(struct task_struct *task);
@@ -66,6 +67,9 @@ static inline void flush_mte_state(void)
 static inline void mte_thread_switch(struct task_struct *next)
 {
 }
+static inline void mte_suspend_enter(void)
+{
+}
 static inline void mte_suspend_exit(void)
 {
 }
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index f5aa5bea6dfe..de905102245a 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -258,12 +258,32 @@ void mte_thread_switch(struct task_struct *next)
 	mte_check_tfsr_el1();
 }
 
+void mte_suspend_enter(void)
+{
+	if (!system_supports_mte())
+		return;
+
+	/*
+	 * The barriers are required to guarantee that the indirect writes
+	 * to TFSR_EL1 are synchronized before we report the state.
+	 */
+	dsb(nsh);
+	isb();
+
+	/* Report SYS_TFSR_EL1 before suspend entry */
+	mte_check_tfsr_el1();
+}
+
 void mte_suspend_exit(void)
 {
 	if (!system_supports_mte())
 		return;
 
 	update_gcr_el1_excl(gcr_kernel_excl);
+
+	/* Clear SYS_TFSR_EL1 after suspend exit */
+	write_sysreg_s(0, SYS_TFSR_EL1);
+
 }
 
 long set_mte_ctrl(struct task_struct *task, unsigned long arg)
diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
index a67b37a7a47e..25a02926ad88 100644
--- a/arch/arm64/kernel/suspend.c
+++ b/arch/arm64/kernel/suspend.c
@@ -91,6 +91,9 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 	unsigned long flags;
 	struct sleep_stack_data state;
 
+	/* Report any MTE async fault before going to suspend */
+	mte_suspend_enter();
+
 	/*
 	 * From this point debug exceptions are disabled to prevent
 	 * updates to mdscr register (saved and restored along with
-- 
2.30.0

Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend

[Lorenzo Pieralisi]

On Thu, Feb 11, 2021 at 03:33:52PM +0000, Vincenzo Frascino wrote:
> When MTE async mode is enabled TFSR_EL1 contains the accumulative
> asynchronous tag check faults for EL1 and EL0.
> 
> During the suspend/resume operations the firmware might perform some
> operations that could change the state of the register resulting in
> a spurious tag check fault report.
> 
> Report asynchronous tag faults before suspend and clear the TFSR_EL1
> register after resume to prevent this to happen.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  arch/arm64/include/asm/mte.h |  4 ++++
>  arch/arm64/kernel/mte.c      | 20 ++++++++++++++++++++
>  arch/arm64/kernel/suspend.c  |  3 +++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 43169b978cd3..33e88a470357 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -41,6 +41,7 @@ void mte_sync_tags(pte_t *ptep, pte_t pte);
>  void mte_copy_page_tags(void *kto, const void *kfrom);
>  void flush_mte_state(void);
>  void mte_thread_switch(struct task_struct *next);
> +void mte_suspend_enter(void);
>  void mte_suspend_exit(void);
>  long set_mte_ctrl(struct task_struct *task, unsigned long arg);
>  long get_mte_ctrl(struct task_struct *task);
> @@ -66,6 +67,9 @@ static inline void flush_mte_state(void)
>  static inline void mte_thread_switch(struct task_struct *next)
>  {
>  }
> +static inline void mte_suspend_enter(void)
> +{
> +}
>  static inline void mte_suspend_exit(void)
>  {
>  }
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index f5aa5bea6dfe..de905102245a 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -258,12 +258,32 @@ void mte_thread_switch(struct task_struct *next)
>  	mte_check_tfsr_el1();
>  }
>  
> +void mte_suspend_enter(void)
> +{
> +	if (!system_supports_mte())
> +		return;
> +
> +	/*
> +	 * The barriers are required to guarantee that the indirect writes
> +	 * to TFSR_EL1 are synchronized before we report the state.
> +	 */
> +	dsb(nsh);
> +	isb();
> +
> +	/* Report SYS_TFSR_EL1 before suspend entry */
> +	mte_check_tfsr_el1();
> +}
> +
>  void mte_suspend_exit(void)
>  {
>  	if (!system_supports_mte())
>  		return;
>  
>  	update_gcr_el1_excl(gcr_kernel_excl);
> +
> +	/* Clear SYS_TFSR_EL1 after suspend exit */
> +	write_sysreg_s(0, SYS_TFSR_EL1);

AFAICS it is not needed, it is done already in __cpu_setup() (that is
called by cpu_resume on return from cpu_suspend() from firmware).

However, I have a question. We are relying on context switch to set
sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
low power switches context we are running with SCTLR_EL1_TCF0 not
reflecting the actual value.

Just making sure that I understand it correctly, I need to check the
resume from suspend-to-RAM path, it is something that came up with perf
save/restore already in the past.

Lorenzo

> +
>  }
>  
>  long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index a67b37a7a47e..25a02926ad88 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -91,6 +91,9 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
>  	unsigned long flags;
>  	struct sleep_stack_data state;
>  
> +	/* Report any MTE async fault before going to suspend */
> +	mte_suspend_enter();
> +
>  	/*
>  	 * From this point debug exceptions are disabled to prevent
>  	 * updates to mdscr register (saved and restored along with
> -- 
> 2.30.0
> 

Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend

[Lorenzo Pieralisi]

On Fri, Feb 12, 2021 at 12:00:15PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Feb 11, 2021 at 03:33:52PM +0000, Vincenzo Frascino wrote:
> > When MTE async mode is enabled TFSR_EL1 contains the accumulative
> > asynchronous tag check faults for EL1 and EL0.
> > 
> > During the suspend/resume operations the firmware might perform some
> > operations that could change the state of the register resulting in
> > a spurious tag check fault report.
> > 
> > Report asynchronous tag faults before suspend and clear the TFSR_EL1
> > register after resume to prevent this to happen.
> > 
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > ---
> >  arch/arm64/include/asm/mte.h |  4 ++++
> >  arch/arm64/kernel/mte.c      | 20 ++++++++++++++++++++
> >  arch/arm64/kernel/suspend.c  |  3 +++
> >  3 files changed, 27 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> > index 43169b978cd3..33e88a470357 100644
> > --- a/arch/arm64/include/asm/mte.h
> > +++ b/arch/arm64/include/asm/mte.h
> > @@ -41,6 +41,7 @@ void mte_sync_tags(pte_t *ptep, pte_t pte);
> >  void mte_copy_page_tags(void *kto, const void *kfrom);
> >  void flush_mte_state(void);
> >  void mte_thread_switch(struct task_struct *next);
> > +void mte_suspend_enter(void);
> >  void mte_suspend_exit(void);
> >  long set_mte_ctrl(struct task_struct *task, unsigned long arg);
> >  long get_mte_ctrl(struct task_struct *task);
> > @@ -66,6 +67,9 @@ static inline void flush_mte_state(void)
> >  static inline void mte_thread_switch(struct task_struct *next)
> >  {
> >  }
> > +static inline void mte_suspend_enter(void)
> > +{
> > +}
> >  static inline void mte_suspend_exit(void)
> >  {
> >  }
> > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > index f5aa5bea6dfe..de905102245a 100644
> > --- a/arch/arm64/kernel/mte.c
> > +++ b/arch/arm64/kernel/mte.c
> > @@ -258,12 +258,32 @@ void mte_thread_switch(struct task_struct *next)
> >  	mte_check_tfsr_el1();
> >  }
> >  
> > +void mte_suspend_enter(void)
> > +{
> > +	if (!system_supports_mte())
> > +		return;
> > +
> > +	/*
> > +	 * The barriers are required to guarantee that the indirect writes
> > +	 * to TFSR_EL1 are synchronized before we report the state.
> > +	 */
> > +	dsb(nsh);
> > +	isb();
> > +
> > +	/* Report SYS_TFSR_EL1 before suspend entry */
> > +	mte_check_tfsr_el1();
> > +}
> > +
> >  void mte_suspend_exit(void)
> >  {
> >  	if (!system_supports_mte())
> >  		return;
> >  
> >  	update_gcr_el1_excl(gcr_kernel_excl);
> > +
> > +	/* Clear SYS_TFSR_EL1 after suspend exit */
> > +	write_sysreg_s(0, SYS_TFSR_EL1);
> 
> AFAICS it is not needed, it is done already in __cpu_setup() (that is
> called by cpu_resume on return from cpu_suspend() from firmware).
> 
> However, I have a question. We are relying on context switch to set
> sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
> low power switches context we are running with SCTLR_EL1_TCF0 not
> reflecting the actual value.

Forget this, we obviously restore sctlr_el1 on resume (cpu_do_resume()).

With the line above removed:

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> Just making sure that I understand it correctly, I need to check the
> resume from suspend-to-RAM path, it is something that came up with perf
> save/restore already in the past.
> 
> Lorenzo
> 
> > +
> >  }
> >  
> >  long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> > diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> > index a67b37a7a47e..25a02926ad88 100644
> > --- a/arch/arm64/kernel/suspend.c
> > +++ b/arch/arm64/kernel/suspend.c
> > @@ -91,6 +91,9 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
> >  	unsigned long flags;
> >  	struct sleep_stack_data state;
> >  
> > +	/* Report any MTE async fault before going to suspend */
> > +	mte_suspend_enter();
> > +
> >  	/*
> >  	 * From this point debug exceptions are disabled to prevent
> >  	 * updates to mdscr register (saved and restored along with
> > -- 
> > 2.30.0
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend

[Vincenzo Frascino]

On 2/12/21 12:30 PM, Lorenzo Pieralisi wrote:
>> However, I have a question. We are relying on context switch to set
>> sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
>> low power switches context we are running with SCTLR_EL1_TCF0 not
>> reflecting the actual value.
> Forget this, we obviously restore sctlr_el1 on resume (cpu_do_resume()).
> 
> With the line above removed:
> 
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> 

Thanks Lorenzo, I will remove the register write in the next version and add
your tag.

-- 
Regards,
Vincenzo

Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend

[Catalin Marinas]

On Fri, Feb 12, 2021 at 12:00:15PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Feb 11, 2021 at 03:33:52PM +0000, Vincenzo Frascino wrote:
> > +void mte_suspend_enter(void)
> > +{
> > +	if (!system_supports_mte())
> > +		return;
> > +
> > +	/*
> > +	 * The barriers are required to guarantee that the indirect writes
> > +	 * to TFSR_EL1 are synchronized before we report the state.
> > +	 */
> > +	dsb(nsh);
> > +	isb();
> > +
> > +	/* Report SYS_TFSR_EL1 before suspend entry */
> > +	mte_check_tfsr_el1();
> > +}
> > +
> >  void mte_suspend_exit(void)
> >  {
> >  	if (!system_supports_mte())
> >  		return;
> >  
> >  	update_gcr_el1_excl(gcr_kernel_excl);
> > +
> > +	/* Clear SYS_TFSR_EL1 after suspend exit */
> > +	write_sysreg_s(0, SYS_TFSR_EL1);
> 
> AFAICS it is not needed, it is done already in __cpu_setup() (that is
> called by cpu_resume on return from cpu_suspend() from firmware).
> 
> However, I have a question. We are relying on context switch to set
> sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
> low power switches context we are running with SCTLR_EL1_TCF0 not
> reflecting the actual value.

I think you have a point here, though not for SCTLR_EL1 as it is already
restored. GCR_EL1 is only updated after some C code has run and may mess
up stack tagging when/if we ever support it. Anyway, something to worry
about later, I think even the boot path gets this wrong.

-- 
Catalin

Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend

[Catalin Marinas]

On Thu, Feb 11, 2021 at 03:33:52PM +0000, Vincenzo Frascino wrote:
>  void mte_suspend_exit(void)
>  {
>  	if (!system_supports_mte())
>  		return;
>  
>  	update_gcr_el1_excl(gcr_kernel_excl);
> +
> +	/* Clear SYS_TFSR_EL1 after suspend exit */
> +	write_sysreg_s(0, SYS_TFSR_EL1);
> +
>  }

As per Lorenzo's comment, with this hunk removed:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH v13 7/7] kasan: don't run tests in async mode

[Vincenzo Frascino]

From: Andrey Konovalov <andreyknvl@google.com>

Asynchronous KASAN mode doesn't guarantee that a tag fault will be
detected immediately and causes tests to fail. Forbid running them
in asynchronous mode.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 lib/test_kasan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index f8c72d3aed64..77a60592d350 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -51,6 +51,10 @@ static int kasan_test_init(struct kunit *test)
 		kunit_err(test, "can't run KASAN tests with KASAN disabled");
 		return -1;
 	}
+	if (kasan_flag_async) {
+		kunit_err(test, "can't run KASAN tests in async mode");
+		return -1;
+	}
 
 	multishot = kasan_save_enable_multi_shot();
 	hw_set_tagging_report_once(false);
-- 
2.30.0

Re: [PATCH v13 7/7] kasan: don't run tests in async mode

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2734 bytes --]

Hi Vincenzo,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20210211]
[cannot apply to arm64/for-next/core xlnx/master arm/for-next soc/for-next kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc7 v5.11-rc6 v5.11-rc5 v5.11-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
base:    671176b0016c80b3943cb5387312c886aba3308d
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0ea84467dcc53cac5c8d4e636c4d6f24e6c58728
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210212-004947
        git checkout 0ea84467dcc53cac5c8d4e636c4d6f24e6c58728
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   aarch64-linux-ld: mm/kasan/report.o: in function `end_report':
   report.c:(.text+0x18): undefined reference to `kasan_flag_async'
   aarch64-linux-ld: mm/kasan/report.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `kasan_flag_async' which may bind externally can not be used when making a shared object; recompile with -fPIC
   report.c:(.text+0x18): dangerous relocation: unsupported relocation
   aarch64-linux-ld: report.c:(.text+0x1c): undefined reference to `kasan_flag_async'
   aarch64-linux-ld: lib/test_kasan.o: in function `kasan_test_init':
   test_kasan.c:(.text+0xccf0): undefined reference to `kasan_flag_async'
   aarch64-linux-ld: lib/test_kasan.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `kasan_flag_async' which may bind externally can not be used when making a shared object; recompile with -fPIC
   test_kasan.c:(.text+0xccf0): dangerous relocation: unsupported relocation
>> aarch64-linux-ld: test_kasan.c:(.text+0xccfc): undefined reference to `kasan_flag_async'
   aarch64-linux-ld: test_kasan.c:(.text+0xcd04): undefined reference to `kasan_flag_async'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 76858 bytes --]

Re: [PATCH v13 7/7] kasan: don't run tests in async mode

[Catalin Marinas]

On Thu, Feb 11, 2021 at 03:33:53PM +0000, Vincenzo Frascino wrote:
> From: Andrey Konovalov <andreyknvl@google.com>
> 
> Asynchronous KASAN mode doesn't guarantee that a tag fault will be
> detected immediately and causes tests to fail. Forbid running them
> in asynchronous mode.
> 
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  lib/test_kasan.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index f8c72d3aed64..77a60592d350 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -51,6 +51,10 @@ static int kasan_test_init(struct kunit *test)
>  		kunit_err(test, "can't run KASAN tests with KASAN disabled");
>  		return -1;
>  	}
> +	if (kasan_flag_async) {
> +		kunit_err(test, "can't run KASAN tests in async mode");
> +		return -1;
> +	}

I think we have time to fix this properly ;), so I'd rather not add this
patch at all.

-- 
Catalin

Re: [PATCH v13 7/7] kasan: don't run tests in async mode

[Andrey Konovalov]

On Fri, Feb 12, 2021 at 6:22 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Thu, Feb 11, 2021 at 03:33:53PM +0000, Vincenzo Frascino wrote:
> > From: Andrey Konovalov <andreyknvl@google.com>
> >
> > Asynchronous KASAN mode doesn't guarantee that a tag fault will be
> > detected immediately and causes tests to fail. Forbid running them
> > in asynchronous mode.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > ---
> >  lib/test_kasan.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> > index f8c72d3aed64..77a60592d350 100644
> > --- a/lib/test_kasan.c
> > +++ b/lib/test_kasan.c
> > @@ -51,6 +51,10 @@ static int kasan_test_init(struct kunit *test)
> >               kunit_err(test, "can't run KASAN tests with KASAN disabled");
> >               return -1;
> >       }
> > +     if (kasan_flag_async) {
> > +             kunit_err(test, "can't run KASAN tests in async mode");
> > +             return -1;
> > +     }
>
> I think we have time to fix this properly ;), so I'd rather not add this
> patch at all.

Yeah, this patch can be dropped.

I have a prototype of async support for tests working. I'll apply it
on top of the next version Vincenzo posts and share the patch.

Vincenzo, when you post the next version, please make sure you rebase
on top of the mm tree version that includes "kasan: export HW_TAGS
symbols for KUnit tests" (linux-next/akpm doesn't yet have it).

Re: [PATCH v13 7/7] kasan: don't run tests in async mode

[Vincenzo Frascino]

On 2/12/21 9:44 PM, Andrey Konovalov wrote:
>> I think we have time to fix this properly ;), so I'd rather not add this
>> patch at all.
> Yeah, this patch can be dropped.
> 
> I have a prototype of async support for tests working. I'll apply it
> on top of the next version Vincenzo posts and share the patch.
> 
> Vincenzo, when you post the next version, please make sure you rebase
> on top of the mm tree version that includes "kasan: export HW_TAGS
> symbols for KUnit tests" (linux-next/akpm doesn't yet have it).

Fine by me, I will drop this patch when I will repost in -rc1.

@Andrey: If you want me to test the series all together, you can send me your
tree before I repost and then I can send the patches as single series. What do
you think?

-- 
Regards,
Vincenzo

Re: [PATCH v13 7/7] kasan: don't run tests in async mode

[Andrey Konovalov]

On Mon, Feb 22, 2021 at 12:13 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> On 2/12/21 9:44 PM, Andrey Konovalov wrote:
> >> I think we have time to fix this properly ;), so I'd rather not add this
> >> patch at all.
> > Yeah, this patch can be dropped.
> >
> > I have a prototype of async support for tests working. I'll apply it
> > on top of the next version Vincenzo posts and share the patch.
> >
> > Vincenzo, when you post the next version, please make sure you rebase
> > on top of the mm tree version that includes "kasan: export HW_TAGS
> > symbols for KUnit tests" (linux-next/akpm doesn't yet have it).
>
> Fine by me, I will drop this patch when I will repost in -rc1.
>
> @Andrey: If you want me to test the series all together, you can send me your
> tree before I repost and then I can send the patches as single series. What do
> you think?

I'll need a rebased version of your patches first, otherwise conflicts
will be a pain.

Thanks!

Re: [PATCH v13 0/7] arm64: ARMv8.5-A: MTE: Add async mode support

[Vincenzo Frascino]

On 2/11/21 3:33 PM, Vincenzo Frascino wrote:
> 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/v11.async.akpm

akpm tree seems currently broken due to [1]. If you want to test my patches a
possible workaround is to remove manually the content of $KBUILD_OUTPUT and then
do the usual:

make defconfig && make menuconfig (to enable KASAN HW TAGS) && make -j<n>

[1] https://www.spinics.net/lists/netdev/msg721547.html

Thanks!

-- 
Regards,
Vincenzo