On Wed, Sep 03, 2025 at 04:00:19PM +0100, Yeoreum Yun wrote:
> Since Armv8.9, FEATURE_MTE_STORE_ONLY feature is introduced to restrict
> raise of tag check fault on store operation only.
> Introcude KASAN write only mode based on this feature.
Typo ^^
>
> KASAN write only mode restricts KASAN checks operation for write only and
> omits the checks for fetch/read operations when accessing memory.
> So it might be used not only debugging enviroment but also normal
> enviroment to check memory safty.
>
> This features can be controlled with "kasan.write_only" arguments.
> When "kasan.write_only=on", KASAN checks write operation only otherwise
> KASAN checks all operations.
>
> This changes the MTE_STORE_ONLY feature as BOOT_CPU_FEATURE like
> ARM64_MTE_ASYMM so that makes it initialise in kasan_init_hw_tags()
> with other function together.
>
> Signed-off-by: Yeoreum Yun <yeoreum....@arm.com>
> Reviewed-by: Andrey Konovalov <andreyk...@gmail.com>
> Reviewed-by: Catalin Marinas <catalin.mari...@arm.com>
> ---
> Documentation/dev-tools/kasan.rst | 3 ++
> arch/arm64/include/asm/memory.h | 1 +
> arch/arm64/include/asm/mte-kasan.h | 6 +++
> arch/arm64/kernel/cpufeature.c | 2 +-
> arch/arm64/kernel/mte.c | 18 ++++++++
> mm/kasan/hw_tags.c | 70 +++++++++++++++++++++++++++++-
> mm/kasan/kasan.h | 7 +++
> 7 files changed, 104 insertions(+), 3 deletions(-)
[...]
> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index 9a6927394b54..d5b5fb47d52b 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -41,9 +41,16 @@ enum kasan_arg_vmalloc {
> KASAN_ARG_VMALLOC_ON,
> };
>
> +enum kasan_arg_write_only {
> + KASAN_ARG_WRITE_ONLY_DEFAULT,
> + KASAN_ARG_WRITE_ONLY_OFF,
> + KASAN_ARG_WRITE_ONLY_ON,
> +};
> +
> static enum kasan_arg kasan_arg __ro_after_init;
> static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
> static enum kasan_arg_vmalloc kasan_arg_vmalloc __initdata;
> +static enum kasan_arg_write_only kasan_arg_write_only __ro_after_init;
>
> /*
> * Whether KASAN is enabled at all.
> @@ -67,6 +74,9 @@ DEFINE_STATIC_KEY_FALSE(kasan_flag_vmalloc);
> #endif
> EXPORT_SYMBOL_GPL(kasan_flag_vmalloc);
>
> +/* Whether to check write accesses only. */
> +static bool kasan_flag_write_only = false;
> +
> #define PAGE_ALLOC_SAMPLE_DEFAULT 1
> #define PAGE_ALLOC_SAMPLE_ORDER_DEFAULT 3
>
> @@ -141,6 +151,23 @@ static int __init early_kasan_flag_vmalloc(char *arg)
> }
> early_param("kasan.vmalloc", early_kasan_flag_vmalloc);
>
> +/* kasan.write_only=off/on */
> +static int __init early_kasan_flag_write_only(char *arg)
> +{
> + if (!arg)
> + return -EINVAL;
> +
> + if (!strcmp(arg, "off"))
> + kasan_arg_write_only = KASAN_ARG_WRITE_ONLY_OFF;
> + else if (!strcmp(arg, "on"))
> + kasan_arg_write_only = KASAN_ARG_WRITE_ONLY_ON;
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +early_param("kasan.write_only", early_kasan_flag_write_only);
> +
> static inline const char *kasan_mode_info(void)
> {
> if (kasan_mode == KASAN_MODE_ASYNC)
> @@ -257,15 +284,28 @@ void __init kasan_init_hw_tags(void)
> break;
> }
>
> + switch (kasan_arg_write_only) {
> + case KASAN_ARG_WRITE_ONLY_DEFAULT:
> + /* Default is specified by kasan_flag_write_only definition. */
> + break;
> + case KASAN_ARG_WRITE_ONLY_OFF:
> + kasan_flag_write_only = false;
> + break;
> + case KASAN_ARG_WRITE_ONLY_ON:
> + kasan_flag_write_only = true;
> + break;
> + }
> +
> kasan_init_tags();
I'm probably missing something here, but why have 'enum
kasan_arg_write_only' at all? What stops you from setting
'kasan_flag_write_only' directly from early_kasan_flag_write_only()?
This all looks weirdly over-engineered, as though 'kasan_flag_write_only'
is expected to be statically initialised to something other than 'false'.
> /* KASAN is now initialized, enable it. */
> static_branch_enable(&kasan_flag_enabled);
>
> - pr_info("KernelAddressSanitizer initialized (hw-tags, mode=%s,
> vmalloc=%s, stacktrace=%s)\n",
> + pr_info("KernelAddressSanitizer initialized (hw-tags, mode=%s,
> vmalloc=%s, stacktrace=%s, write_only=%s)\n",
> kasan_mode_info(),
> str_on_off(kasan_vmalloc_enabled()),
> - str_on_off(kasan_stack_collection_enabled()));
> + str_on_off(kasan_stack_collection_enabled()),
> + str_on_off(kasan_arg_write_only));
It's also confusing, because now you appear to be passing the funny new
'enum' type to str_on_off(), which expects a bool.
Will
