linux-kernel - Re: [PATCH v3 1/4] kasan, arm64: Add KASAN light mode

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: <ba23ab9b-8f49-bdb7-87d8-3eb99ddf54b6@arm.com>
Date:   Sat, 16 Jan 2021 13:47:08 +0000
From:   Vincenzo Frascino <vincenzo.frascino@....com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kasan-dev@...glegroups.com, Marco Elver <elver@...gle.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Branislav Rankov <Branislav.Rankov@....com>,
        Alexander Potapenko <glider@...gle.com>,
        Evgenii Stepanov <eugenis@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Will Deacon <will@...nel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [PATCH v3 1/4] kasan, arm64: Add KASAN light mode

Hi Mark,

On 1/15/21 3:08 PM, Mark Rutland wrote:
> On Fri, Jan 15, 2021 at 12:00:40PM +0000, Vincenzo Frascino wrote:
>> Architectures supported by KASAN HW can provide a light mode of
>> execution. On an MTE enabled arm64 hw for example this can be identified
>> with the asynch mode of execution.
>> In this mode, if a tag check fault occurs, the TFSR_EL1 register is
>> updated asynchronously. The kernel checks the corresponding bits
>> periodically.
> 
> What's the expected usage of this relative to prod, given that this has
> to be chosen at boot time? When/where is this expected to be used
> relative to prod mode?
> 

IIUC the light mode is meant for low spec devices. I let Andrey comment a bit
more on this topic.

>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> index 18fce223b67b..3a7c5beb7096 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -231,7 +231,7 @@ static inline const void *__tag_set(const void *addr, u8 tag)
>>  }
>>  
>>  #ifdef CONFIG_KASAN_HW_TAGS
>> -#define arch_enable_tagging()			mte_enable_kernel()
>> +#define arch_enable_tagging(mode)		mte_enable_kernel(mode)
> 
> Rather than passing a mode in, I think it'd be better to have:
> 
> * arch_enable_tagging_prod()
> * arch_enable_tagging_light()
> 
> ... that we can map in the arch code to separate:
> 
> * mte_enable_kernel_sync()
> * mte_enable_kernel_async()
> 
> ... as by construction that avoids calls with an unhandled mode, and we
> wouldn't need the mode enum kasan_hw_tags_mode...
> 
>> +static inline int hw_init_mode(enum kasan_arg_mode mode)
>> +{
>> +	switch (mode) {
>> +	case KASAN_ARG_MODE_LIGHT:
>> +		return KASAN_HW_TAGS_ASYNC;
>> +	default:
>> +		return KASAN_HW_TAGS_SYNC;
>> +	}
>> +}
> 
> ... and we can just have a wrapper like this to call either of the two functions directly, i.e.
> 
> static inline void hw_enable_tagging_mode(enum kasan_arg_mode mode)
> {
> 	if (mode == KASAN_ARG_MODE_LIGHT)
> 		arch_enable_tagging_mode_light();
> 	else
> 		arch_enable_tagging_mode_prod();
> }
>

Fine by me, this would remove the need of adding a new enumeration as well and
reflect on the arch code. I would keep "arch_enable_tagging_mode_sync" and
"arch_enable_tagging_mode_async" though to give a clear indication in the KASAN
code of the mode we are setting. I will adapt my code accordingly for v4.

> Thanks,
> Mark.
> 

-- 
Regards,
Vincenzo