diff mbox series

[5/5] kasan: Extend KASAN mode kernel parameter

Message ID 20210913081424.48613-6-vincenzo.frascino@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: ARMv8.7-A: MTE: Add asymm mode support | expand

Commit Message

Vincenzo Frascino Sept. 13, 2021, 8:14 a.m. UTC
Architectures supported by KASAN_HW_TAGS can provide an asymmetric mode
of execution. On an MTE enabled arm64 hw for example this can be
identified with the asymmetric tagging mode of execution. In particular,
when such a mode is present, the CPU triggers a fault on a tag mismatch
during a load operation and asynchronously updates a register when a tag
mismatch is detected during a store operation.

Extend the KASAN HW execution mode kernel command line parameter to
support asymmetric mode.

Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 Documentation/dev-tools/kasan.rst | 10 ++++++++--
 mm/kasan/hw_tags.c                | 27 ++++++++++++++++++++++-----
 mm/kasan/kasan.h                  |  5 +++++
 3 files changed, 35 insertions(+), 7 deletions(-)

Comments

Marco Elver Sept. 16, 2021, 10:43 a.m. UTC | #1
On Mon, 13 Sept 2021 at 10:14, Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Architectures supported by KASAN_HW_TAGS can provide an asymmetric mode
> of execution. On an MTE enabled arm64 hw for example this can be
> identified with the asymmetric tagging mode of execution. In particular,
> when such a mode is present, the CPU triggers a fault on a tag mismatch
> during a load operation and asynchronously updates a register when a tag
> mismatch is detected during a store operation.
>
> Extend the KASAN HW execution mode kernel command line parameter to
> support asymmetric mode.
>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  Documentation/dev-tools/kasan.rst | 10 ++++++++--
>  mm/kasan/hw_tags.c                | 27 ++++++++++++++++++++++-----
>  mm/kasan/kasan.h                  |  5 +++++
>  3 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index 21dc03bc10a4..7f43e603bfbe 100644
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -194,14 +194,20 @@ additional boot parameters that allow disabling KASAN or controlling 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``).
> +- ``kasan.mode=sync``, ``=async`` or ``=asymm`` controls whether KASAN
> +  is configured in synchronous, asynchronous or asymmetric 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.
> +  Asymmetric mode: a bad access is detected immediately when a tag
> +  check fault occurs during a load operation and its detection is
> +  delayed during a store operation. For the store operations 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/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index 05d1e9460e2e..87eb7aa13918 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -29,6 +29,7 @@ enum kasan_arg_mode {
>         KASAN_ARG_MODE_DEFAULT,
>         KASAN_ARG_MODE_SYNC,
>         KASAN_ARG_MODE_ASYNC,
> +       KASAN_ARG_MODE_ASYMM,
>  };
>
>  enum kasan_arg_stacktrace {
> @@ -49,6 +50,10 @@ EXPORT_SYMBOL(kasan_flag_enabled);
>  bool kasan_flag_async __ro_after_init;
>  EXPORT_SYMBOL_GPL(kasan_flag_async);
>
> +/* Whether the asymmetric mode is enabled. */
> +bool kasan_flag_asymm __ro_after_init;
> +EXPORT_SYMBOL_GPL(kasan_flag_asymm);
> +
>  /* Whether to collect alloc/free stack traces. */
>  DEFINE_STATIC_KEY_FALSE(kasan_flag_stacktrace);
>
> @@ -69,7 +74,7 @@ static int __init early_kasan_flag(char *arg)
>  }
>  early_param("kasan", early_kasan_flag);
>
> -/* kasan.mode=sync/async */
> +/* kasan.mode=sync/async/asymm */
>  static int __init early_kasan_mode(char *arg)
>  {
>         if (!arg)
> @@ -79,6 +84,8 @@ static int __init early_kasan_mode(char *arg)
>                 kasan_arg_mode = KASAN_ARG_MODE_SYNC;
>         else if (!strcmp(arg, "async"))
>                 kasan_arg_mode = KASAN_ARG_MODE_ASYNC;
> +       else if (!strcmp(arg, "asymm"))
> +               kasan_arg_mode = KASAN_ARG_MODE_ASYMM;
>         else
>                 return -EINVAL;
>
> @@ -116,11 +123,13 @@ void kasan_init_hw_tags_cpu(void)
>                 return;
>
>         /*
> -        * Enable async mode only when explicitly requested through
> -        * the command line.
> +        * Enable async or asymm modes only when explicitly requested
> +        * through the command line.
>          */
>         if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
>                 hw_enable_tagging_async();
> +       else if (kasan_arg_mode == KASAN_ARG_MODE_ASYMM)
> +               hw_enable_tagging_asymm();
>         else
>                 hw_enable_tagging_sync();
>  }
> @@ -143,16 +152,24 @@ void __init kasan_init_hw_tags(void)
>         case KASAN_ARG_MODE_DEFAULT:
>                 /*
>                  * Default to sync mode.
> -                * Do nothing, kasan_flag_async keeps its default value.
> +                * Do nothing, kasan_flag_async and kasan_flag_asymm keep
> +                * their default values.
>                  */
>                 break;
>         case KASAN_ARG_MODE_SYNC:
> -               /* Do nothing, kasan_flag_async keeps its default value. */
> +               /*
> +                * Do nothing, kasan_flag_async and kasan_flag_asymm keep
> +                * their default values.
> +                */
>                 break;
>         case KASAN_ARG_MODE_ASYNC:
>                 /* Async mode enabled. */
>                 kasan_flag_async = true;
>                 break;
> +       case KASAN_ARG_MODE_ASYMM:
> +               /* Asymm mode enabled. */
> +               kasan_flag_asymm = true;
> +               break;
>         }
>
>         switch (kasan_arg_stacktrace) {
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 3639e7c8bb98..a8be62058d32 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h

Shouldn't kasan.h also define kasan_asymm_mode_enabled() similar to
kasan_async_mode_enabled()?

And based on that, also use it where kasan_async_mode_enabled() is
used in tests to ensure the tests do not fail. Otherwise, there is no
purpose for kasan_flag_asymm.

Thanks,
-- Marco

> @@ -287,6 +287,9 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
>  #ifndef arch_enable_tagging_async
>  #define arch_enable_tagging_async()
>  #endif
> +#ifndef arch_enable_tagging_asymm
> +#define arch_enable_tagging_asymm()
> +#endif
>  #ifndef arch_force_async_tag_fault
>  #define arch_force_async_tag_fault()
>  #endif
> @@ -302,6 +305,7 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
>
>  #define hw_enable_tagging_sync()               arch_enable_tagging_sync()
>  #define hw_enable_tagging_async()              arch_enable_tagging_async()
> +#define hw_enable_tagging_asymm()              arch_enable_tagging_asymm()
>  #define hw_force_async_tag_fault()             arch_force_async_tag_fault()
>  #define hw_get_random_tag()                    arch_get_random_tag()
>  #define hw_get_mem_tag(addr)                   arch_get_mem_tag(addr)
> @@ -312,6 +316,7 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
>
>  #define hw_enable_tagging_sync()
>  #define hw_enable_tagging_async()
> +#define hw_enable_tagging_asymm()
>
>  #endif /* CONFIG_KASAN_HW_TAGS */
Vincenzo Frascino Sept. 20, 2021, 7:46 a.m. UTC | #2
Hi Marco,

On 9/16/21 12:43 PM, Marco Elver wrote:
>> +       case KASAN_ARG_MODE_ASYMM:
>> +               /* Asymm mode enabled. */
>> +               kasan_flag_asymm = true;
>> +               break;
>>         }
>>
>>         switch (kasan_arg_stacktrace) {
>> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
>> index 3639e7c8bb98..a8be62058d32 100644
>> --- a/mm/kasan/kasan.h
>> +++ b/mm/kasan/kasan.h
> Shouldn't kasan.h also define kasan_asymm_mode_enabled() similar to
> kasan_async_mode_enabled()?
> 
> And based on that, also use it where kasan_async_mode_enabled() is
> used in tests to ensure the tests do not fail. Otherwise, there is no
> purpose for kasan_flag_asymm.
>

I was not planning to have the tests shipped as part of this series, they will
come in a future one.

For what concerns kasan_flag_asymm, I agree with you it is meaningful only if
the tests are implemented hence I will remove it in v2.

Thanks for pointing this out.

> Thanks,
> -- Marco
>
Catalin Marinas Sept. 20, 2021, 3:32 p.m. UTC | #3
On Mon, Sep 13, 2021 at 09:14:24AM +0100, Vincenzo Frascino wrote:
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index 21dc03bc10a4..7f43e603bfbe 100644
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -194,14 +194,20 @@ additional boot parameters that allow disabling KASAN or controlling 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``).
> +- ``kasan.mode=sync``, ``=async`` or ``=asymm`` controls whether KASAN
> +  is configured in synchronous, asynchronous or asymmetric 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.
> +  Asymmetric mode: a bad access is detected immediately when a tag
> +  check fault occurs during a load operation and its detection is
> +  delayed during a store operation. For the store operations the kernel
> +  periodically checks the hardware and only reports tag faults during
> +  these checks.

Nitpick: I'd simply refer to the sync/async which already describe what
the kernel and hardware does, something like the tag checks synchronous
on reads and asynchronous on writes.

Otherwise:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Andrey Konovalov Oct. 3, 2021, 5:16 p.m. UTC | #4
On Mon, Sep 20, 2021 at 9:46 AM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> On 9/16/21 12:43 PM, Marco Elver wrote:
> >
> > Shouldn't kasan.h also define kasan_asymm_mode_enabled() similar to
> > kasan_async_mode_enabled()?
> >
> > And based on that, also use it where kasan_async_mode_enabled() is
> > used in tests to ensure the tests do not fail. Otherwise, there is no
> > purpose for kasan_flag_asymm.
> >
>
> I was not planning to have the tests shipped as part of this series, they will
> come in a future one.
>
> For what concerns kasan_flag_asymm, I agree with you it is meaningful only if
> the tests are implemented hence I will remove it in v2.

Hi Vincenzo,

Up till now, the code assumes that not having the async mode enabled
means that the sync mode is enabled. There are two callers to
kasan_async_mode_enabled(): lib/test_kasan.c and mm/kasan/report.c.
Assuming tests support will be added later, at least the second one
should be adjusted.

Maybe we should rename kasan_async_mode_enabled() to
kasan_async_fault_possible(), make it return true for both async and
asymm modes, and use that in mm/kasan/report.c. And also add
kasan_sync_fault_possible() returning true for sync and asymm, and use
that in lib/test_kasan.c. (However, it seems that the tests don't work
with async faults right now.)

Thanks!
Vincenzo Frascino Oct. 4, 2021, 3:45 p.m. UTC | #5
Hi Andrey,

On 10/3/21 7:16 PM, Andrey Konovalov wrote:
> Hi Vincenzo,
> 
> Up till now, the code assumes that not having the async mode enabled
> means that the sync mode is enabled. There are two callers to
> kasan_async_mode_enabled(): lib/test_kasan.c and mm/kasan/report.c.
> Assuming tests support will be added later, at least the second one
> should be adjusted.
> 
> Maybe we should rename kasan_async_mode_enabled() to
> kasan_async_fault_possible(), make it return true for both async and
> asymm modes, and use that in mm/kasan/report.c. And also add
> kasan_sync_fault_possible() returning true for sync and asymm, and use
> that in lib/test_kasan.c. (However, it seems that the tests don't work
> with async faults right now.)
> 

It is ok by me, I will add the changes you are mentioning in v2.

> Thanks!
diff mbox series

Patch

diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index 21dc03bc10a4..7f43e603bfbe 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -194,14 +194,20 @@  additional boot parameters that allow disabling KASAN or controlling 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``).
+- ``kasan.mode=sync``, ``=async`` or ``=asymm`` controls whether KASAN
+  is configured in synchronous, asynchronous or asymmetric 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.
+  Asymmetric mode: a bad access is detected immediately when a tag
+  check fault occurs during a load operation and its detection is
+  delayed during a store operation. For the store operations 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/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 05d1e9460e2e..87eb7aa13918 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -29,6 +29,7 @@  enum kasan_arg_mode {
 	KASAN_ARG_MODE_DEFAULT,
 	KASAN_ARG_MODE_SYNC,
 	KASAN_ARG_MODE_ASYNC,
+	KASAN_ARG_MODE_ASYMM,
 };
 
 enum kasan_arg_stacktrace {
@@ -49,6 +50,10 @@  EXPORT_SYMBOL(kasan_flag_enabled);
 bool kasan_flag_async __ro_after_init;
 EXPORT_SYMBOL_GPL(kasan_flag_async);
 
+/* Whether the asymmetric mode is enabled. */
+bool kasan_flag_asymm __ro_after_init;
+EXPORT_SYMBOL_GPL(kasan_flag_asymm);
+
 /* Whether to collect alloc/free stack traces. */
 DEFINE_STATIC_KEY_FALSE(kasan_flag_stacktrace);
 
@@ -69,7 +74,7 @@  static int __init early_kasan_flag(char *arg)
 }
 early_param("kasan", early_kasan_flag);
 
-/* kasan.mode=sync/async */
+/* kasan.mode=sync/async/asymm */
 static int __init early_kasan_mode(char *arg)
 {
 	if (!arg)
@@ -79,6 +84,8 @@  static int __init early_kasan_mode(char *arg)
 		kasan_arg_mode = KASAN_ARG_MODE_SYNC;
 	else if (!strcmp(arg, "async"))
 		kasan_arg_mode = KASAN_ARG_MODE_ASYNC;
+	else if (!strcmp(arg, "asymm"))
+		kasan_arg_mode = KASAN_ARG_MODE_ASYMM;
 	else
 		return -EINVAL;
 
@@ -116,11 +123,13 @@  void kasan_init_hw_tags_cpu(void)
 		return;
 
 	/*
-	 * Enable async mode only when explicitly requested through
-	 * the command line.
+	 * Enable async or asymm modes only when explicitly requested
+	 * through the command line.
 	 */
 	if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
 		hw_enable_tagging_async();
+	else if (kasan_arg_mode == KASAN_ARG_MODE_ASYMM)
+		hw_enable_tagging_asymm();
 	else
 		hw_enable_tagging_sync();
 }
@@ -143,16 +152,24 @@  void __init kasan_init_hw_tags(void)
 	case KASAN_ARG_MODE_DEFAULT:
 		/*
 		 * Default to sync mode.
-		 * Do nothing, kasan_flag_async keeps its default value.
+		 * Do nothing, kasan_flag_async and kasan_flag_asymm keep
+		 * their default values.
 		 */
 		break;
 	case KASAN_ARG_MODE_SYNC:
-		/* Do nothing, kasan_flag_async keeps its default value. */
+		/*
+		 * Do nothing, kasan_flag_async and kasan_flag_asymm keep
+		 * their default values.
+		 */
 		break;
 	case KASAN_ARG_MODE_ASYNC:
 		/* Async mode enabled. */
 		kasan_flag_async = true;
 		break;
+	case KASAN_ARG_MODE_ASYMM:
+		/* Asymm mode enabled. */
+		kasan_flag_asymm = true;
+		break;
 	}
 
 	switch (kasan_arg_stacktrace) {
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 3639e7c8bb98..a8be62058d32 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -287,6 +287,9 @@  static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 #ifndef arch_enable_tagging_async
 #define arch_enable_tagging_async()
 #endif
+#ifndef arch_enable_tagging_asymm
+#define arch_enable_tagging_asymm()
+#endif
 #ifndef arch_force_async_tag_fault
 #define arch_force_async_tag_fault()
 #endif
@@ -302,6 +305,7 @@  static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 
 #define hw_enable_tagging_sync()		arch_enable_tagging_sync()
 #define hw_enable_tagging_async()		arch_enable_tagging_async()
+#define hw_enable_tagging_asymm()		arch_enable_tagging_asymm()
 #define hw_force_async_tag_fault()		arch_force_async_tag_fault()
 #define hw_get_random_tag()			arch_get_random_tag()
 #define hw_get_mem_tag(addr)			arch_get_mem_tag(addr)
@@ -312,6 +316,7 @@  static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 
 #define hw_enable_tagging_sync()
 #define hw_enable_tagging_async()
+#define hw_enable_tagging_asymm()
 
 #endif /* CONFIG_KASAN_HW_TAGS */