linux-kernel - Re: [PATCH v2 1/2] kasan/hw-tags: introduce kasan.store

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aJ8WTyRJVznC9v4K@arm.com>
Date: Fri, 15 Aug 2025 12:13:19 +0100
From: Catalin Marinas <catalin.marinas@....com>
To: Yeoreum Yun <yeoreum.yun@....com>
Cc: ryabinin.a.a@...il.com, glider@...gle.com, andreyknvl@...il.com,
	dvyukov@...gle.com, vincenzo.frascino@....com, corbet@....net,
	will@...nel.org, akpm@...ux-foundation.org,
	scott@...amperecomputing.com, jhubbard@...dia.com,
	pankaj.gupta@....com, leitao@...ian.org, kaleshsingh@...gle.com,
	maz@...nel.org, broonie@...nel.org, oliver.upton@...ux.dev,
	james.morse@....com, ardb@...nel.org,
	hardevsinh.palaniya@...iconsignals.io, david@...hat.com,
	yang@...amperecomputing.com, kasan-dev@...glegroups.com,
	workflows@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mm@...ck.org
Subject: Re: [PATCH v2 1/2] kasan/hw-tags: introduce kasan.store_only option

On Wed, Aug 13, 2025 at 06:53:34PM +0100, Yeoreum Yun wrote:
> diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
> index 2e98028c1965..3e1cc341d47a 100644
> --- a/arch/arm64/include/asm/mte-kasan.h
> +++ b/arch/arm64/include/asm/mte-kasan.h
> @@ -200,6 +200,7 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag,
>  void mte_enable_kernel_sync(void);
>  void mte_enable_kernel_async(void);
>  void mte_enable_kernel_asymm(void);
> +int mte_enable_kernel_store_only(void);
>  
>  #else /* CONFIG_ARM64_MTE */
>  
> @@ -251,6 +252,11 @@ static inline void mte_enable_kernel_asymm(void)
>  {
>  }
>  
> +static inline int mte_enable_kenrel_store_only(void)
				^^^^^^
This won't build with MTE disabled (check spelling).

> +{
> +	return -EINVAL;
> +}
> +
>  #endif /* CONFIG_ARM64_MTE */
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9ad065f15f1d..7b724fcf20a7 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2404,6 +2404,11 @@ static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)
>  
>  	kasan_init_hw_tags_cpu();
>  }
> +
> +static void cpu_enable_mte_store_only(struct arm64_cpu_capabilities const *cap)
> +{
> +	kasan_late_init_hw_tags_cpu();
> +}
>  #endif /* CONFIG_ARM64_MTE */
>  
>  static void user_feature_fixup(void)
> @@ -2922,6 +2927,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.capability = ARM64_MTE_STORE_ONLY,
>  		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
>  		.matches = has_cpuid_feature,
> +		.cpu_enable = cpu_enable_mte_store_only,

I don't think we should add this, see below.

>  		ARM64_CPUID_FIELDS(ID_AA64PFR2_EL1, MTESTOREONLY, IMP)
>  	},
>  #endif /* CONFIG_ARM64_MTE */
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index e5e773844889..8eb1f66f2ccd 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -157,6 +157,20 @@ void mte_enable_kernel_asymm(void)
>  		mte_enable_kernel_sync();
>  	}
>  }
> +
> +int mte_enable_kernel_store_only(void)
> +{
> +	if (!cpus_have_cap(ARM64_MTE_STORE_ONLY))
> +		return -EINVAL;
> +
> +	sysreg_clear_set(sctlr_el1, SCTLR_EL1_TCSO_MASK,
> +			 SYS_FIELD_PREP(SCTLR_EL1, TCSO, 1));
> +	isb();
> +
> +	pr_info_once("MTE: enabled stonly mode at EL1\n");
> +
> +	return 0;
> +}
>  #endif

If we do something like mte_enable_kernel_asymm(), that one doesn't
return any error, just fall back to the default mode.

> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index 9a6927394b54..c2f90c06076e 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -219,6 +246,20 @@ void kasan_init_hw_tags_cpu(void)
>  	kasan_enable_hw_tags();
>  }
>  
> +/*
> + * kasan_late_init_hw_tags_cpu_post() is called for each CPU after
> + * all cpus are bring-up at boot.

Nit: s/bring-up/brought up/

> + * Not marked as __init as a CPU can be hot-plugged after boot.
> + */
> +void kasan_late_init_hw_tags_cpu(void)
> +{
> +	/*
> +	 * Enable stonly mode only when explicitly requested through the command line.
> +	 * If system doesn't support, kasan checks all operation.
> +	 */
> +	kasan_enable_store_only();
> +}

There's nothing late about this. We have kasan_init_hw_tags_cpu()
already and I'd rather have it all handled via this function. It's not
that different from how we added asymmetric support, though store-only
is complementary to the sync vs async checking.

Like we do in mte_enable_kernel_asymm(), if the feature is not available
just fall back to checking both reads and writes in the chosen
async/sync/asymm way. You can add some pr_info() to inform the user of
the chosen kasan mode. It's really mostly an performance choice.

-- 
Catalin