Re: [PATCH v5 2/6] kasan: Add KASAN mode kernel parameter

From: Andrey Konovalov
Date: Thu Jan 21 2021 - 12:38:34 EST

On Thu, Jan 21, 2021 at 5:39 PM Vincenzo Frascino
<vincenzo.frascino@xxxxxxx> 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@xxxxxxxxxx>
> Cc: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
> Cc: Alexander Potapenko <glider@xxxxxxxxxx>
> Cc: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
> ---
> Documentation/dev-tools/kasan.rst | 7 +++++++
> lib/test_kasan.c | 2 +-
> mm/kasan/hw_tags.c | 27 ++++++++++++++++++++++++++-
> mm/kasan/kasan.h | 6 ++++--
> 4 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index e022b7506e37..7e4a6e0c9f57 100644
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -161,6 +161,13 @@ 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``: an exception is triggered if a tag check fault occurs.

Synchronous mode: a bad access is detected immediately when a tag
check fault occurs.

(No need for `` here, "synchronous mode" is not an inline snippet.)

> + ``asynchronous mode``: if a tag check fault occurs, the information is stored
> + asynchronously in hardware (e.g. in the TFSR_EL1 register for arm64). The kernel
> + checks the hardware location and reports an error if the fault is detected.

Asynchronous mode: a bad access detection is delayed. When a tag check
fault occurs, the information is stored in hardware (in the TFSR_EL1
register for arm64). The kernel periodically checks the hardware and
only reports tag faults during these checks.

> +
> - ``kasan.stacktrace=off`` or ``=on`` disables or enables alloc and free stack
> traces collection (default: ``on`` for ``CONFIG_DEBUG_KERNEL=y``, otherwise
> ``off``).
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index d16ec9e66806..7285dcf9fcc1 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -97,7 +97,7 @@ static void kasan_test_exit(struct kunit *test)
> READ_ONCE(fail_data.report_found)); \
> if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \
> if (READ_ONCE(fail_data.report_found)) \
> - hw_enable_tagging(); \
> + hw_enable_tagging_sync(); \
> migrate_enable(); \
> } \
> } while (0)
> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index e529428e7a11..224a2187839c 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -25,6 +25,11 @@ enum kasan_arg {
> KASAN_ARG_ON,
> };
>
> +enum kasan_arg_mode {
> + KASAN_ARG_MODE_SYNC,
> + KASAN_ARG_MODE_ASYNC,

For other modes I explicitly added a _DEFAULT option first. It makes
sense to do this here as well for consistency.

> +};
> +
> enum kasan_arg_stacktrace {
> KASAN_ARG_STACKTRACE_DEFAULT,
> KASAN_ARG_STACKTRACE_OFF,
> @@ -38,6 +43,7 @@ enum kasan_arg_fault {
> };
>
> static enum kasan_arg kasan_arg __ro_after_init;
> +static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
> static enum kasan_arg_stacktrace kasan_arg_stacktrace __ro_after_init;
> static enum kasan_arg_fault kasan_arg_fault __ro_after_init;
>
> @@ -68,6 +74,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 +136,11 @@ void kasan_init_hw_tags_cpu(void)
> return;
>
> hw_init_tags(KASAN_TAG_MAX);
> - hw_enable_tagging();
> +

Let's add a comment:

/* Enable async mode only when explicitly requested through the command line. */

> + if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
> + hw_enable_tagging_async();
> + else
> + hw_enable_tagging_sync();
> }
>
> /* kasan_init_hw_tags() is called once on boot CPU. */
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 07ef7fc742ad..3923d9744105 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -294,7 +294,8 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
> #define arch_set_mem_tag_range(addr, size, tag) ((void *)(addr))
> #endif
>
> -#define hw_enable_tagging() arch_enable_tagging()
> +#define hw_enable_tagging_sync() arch_enable_tagging_sync()
> +#define hw_enable_tagging_async() arch_enable_tagging_async()
> #define hw_init_tags(max_tag) arch_init_tags(max_tag)
> #define hw_set_tagging_report_once(state) arch_set_tagging_report_once(state)
> #define hw_get_random_tag() arch_get_random_tag()
> @@ -303,7 +304,8 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
>
> #else /* CONFIG_KASAN_HW_TAGS */
>
> -#define hw_enable_tagging()
> +#define hw_enable_tagging_sync()
> +#define hw_enable_tagging_async()
> #define hw_set_tagging_report_once(state)
>
> #endif /* CONFIG_KASAN_HW_TAGS */
> --
> 2.30.0
>