Re: [PATCH v8 13/14] x86/kasan: Logical bit shift for kasan_mem_to_shadow

[Maciej Wieczor-Retman]

On 2026-01-13 at 02:21:22 +0100, Andrey Konovalov wrote:
>On Mon, Jan 12, 2026 at 6:28 PM Maciej Wieczor-Retman
><m.wieczorretman@xxxxx> wrote:
>>
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@xxxxxxxxx>
...
>>
>>         /*
>> -        * For Generic KASAN, kasan_mem_to_shadow() uses the logical right shift
>> +        * For Generic KASAN and Software Tag-Based mode on the x86
>> +        * architecture, kasan_mem_to_shadow() uses the logical right shift
>>          * and never overflows with the chosen KASAN_SHADOW_OFFSET values (on
>>          * both x86 and arm64). Thus, the possible shadow addresses (even for
>>          * bogus pointers) belong to a single contiguous region that is the
>>          * result of kasan_mem_to_shadow() applied to the whole address space.
>>          */
>> -       if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
>> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC) || IS_ENABLED(CONFIG_X86_64)) {
>
>Not a functionality but just a code organization related concern:
>
>Here, we embed the CONFIG_X86_64 special case in the core KASAN code,
>but the __kasan_mem_to_shadow definition to use the logical shift
>exists in the x86-64 arch code, and it just copy-pastes one of the
>cases from the core kasan_mem_to_shadow definition.
>
>Should we just move the x86-64 special case to the core KASAN code too
>then? I.e., change the kasan_mem_to_shadow definition in
>include/linux/kasan.h to check for IS_ENABLED(CONFIG_X86_64)).
>
>And we could also add a comment there explaining how using the logical
>shift for SW_TAGS benefits some architectures (just arm64 for now, but
>riscv in the future as well). And put your comment about why it's not
>worth it for x86 there as well.
>
>I don't have a strong preference, just an idea.
>
>Any thoughts?

I'm a fan of trying to keep as much arch code in the arch directories.

How about before putting a call here instead like:

	if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
		if (addr < (unsigned long)kasan_mem_to_shadow((void *)(0ULL)) ||
		    addr > (unsigned long)kasan_mem_to_shadow((void *)(~0ULL)))
			return;
	}

	arch_kasan_non_canonical_hook()
There would be the generic non-arch part above (and anything shared that might
make sense here in the future) and all the arch related code would be hidden in
the per-arch helper.

So then we could move the part below:
	if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) && IS_ENABLED(CONFIG_ARM64)) {
		if (addr < (unsigned long)kasan_mem_to_shadow((void *)(0xFFULL << 56)) ||
		    addr > (unsigned long)kasan_mem_to_shadow((void *)(~0ULL)))
			return;
	}
to /arch/arm64.

For x86 we'd need to duplicate the generic part into
arch_kasan_non_canonical_hook() call in /arch/x86. That seems quiet tidy to me,
granted the duplication isn't great but it would keep the non-arch part as
shared as possible. What do you think?

>
>>                 if (addr < (unsigned long)kasan_mem_to_shadow((void *)(0ULL)) ||
>>                     addr > (unsigned long)kasan_mem_to_shadow((void *)(~0ULL)))
>>                         return;
>
>There's also a comment lower in the function that needs to be updated
>to mention Software Tag-Based mode on arm64 specifically.

Okay, I'll add that in

>
>
>
>
>> --
>> 2.52.0
>>
>>

-- 
Kind regards
Maciej Wieczór-Retman