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: <aJ87cZC3Cy3JJplT@e129823.arm.com>
Date: Fri, 15 Aug 2025 14:51:45 +0100
From: Yeoreum Yun <yeoreum.yun@....com>
To: Catalin Marinas <catalin.marinas@....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

Hi Cataline,

> > 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).

Yes. this is my mistake. I'll fix it..


[...]
> > +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.

Yes. I'll change this.

>
> > 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/

Thanks. I'll fix it.
>
> > + * 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.

But MTE_STORE_ONLY is defined as a SYSTEM_FEATURE.
This means that when it is called from kasan_init_hw_tags_cpu(),
the store_only mode is never set in system_capability,
so it cannot be checked using cpus_have_cap().

Although the MTE_STORE_ONLY capability is verified by
directly reading the ID register (seems ugly),
my concern is the potential for an inconsistent state across CPUs.

For example, in the case of ASYMM, which is a BOOT_CPU_FEATURE,
all CPUs operate in the same mode —
if ASYMM is not supported, either
all CPUs run in synchronous mode, or all run in asymmetric mode.

However, for MTE_STORE_ONLY, CPUs that support the feature will run in store-only mode,
while those that do not will run with full checking for all operations.

If we want to enable MTE_STORE_ONLY in kasan_init_hw_tags_cpu(),
I believe it should be reclassified as a BOOT_CPU_FEATURE.x
Otherwise, the cpu_enable_mte_store_only() function should still be called
as the enable callback for the MTE_STORE_ONLY feature.
In that case, kasan_enable_store_only() should be invoked (remove late init),
and if it returns an error, stop_machine() should be called to disable
the STORE_ONLY feature on all other CPUs
if any CPU is found to lack support for MTE_STORE_ONLY.

Am I missing something?

Thanks

--
Sincerely,
Yeoreum Yun