Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

On 2025-02-22 at 16:06:02 +0100, Andrey Konovalov wrote:

On Fri, Feb 21, 2025 at 2:12 PM Maciej Wieczor-Retman
[off-list ref] wrote:

quoted

quoted

Is there any reason we need this change for x86 SW_TAGS besides the
optimization benefits?

I wanted to have the shadow memory boundries aligned properly, to not waste page
table entries, so the memory map is more straight forward. This patch helps with
that, I don't think it would have worked without it.

Ok, I see - let's add this info into the commit message then.

Sure, but if you like the 0xffeffc0000000000 offset I'll just drop this part.

quoted

quoted

However, I just realized that this check is not entirely precise. When
doing the memory-to-shadow mapping, the memory address always has its
top byte set to 0xff: both the inlined compiler code and the outline
KASAN code do this

Do you mean that non-canonical addresses passed to kasan_mem_to_shadow() will
map to the same space that the canonical version would map to?

No, but non-canonical address are never passed to
kasan_mem_to_shadow(): KASAN always resets the tag before calling this
function.

quoted

What does that? Does the compiler do something more than is in
kasan_mem_to_shadow() when instrumenting functions?

Same for the compiler, it always untags the pointer first [1].

[1] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L922

quoted

quoted

                  Thus, the possible values a shadow address can
take are the result of the memory-to-shadow mapping applied to
[0xff00000000000000, 0xffffffffffffffff], not to the whole address
space. So we can make this check more precise.

In case my question above didn't lead to this: what happens to the rest of the
values if they get plugged into kasan_mem_to_shadow()?

We will get some invalid addresses. But this should never happen in
the first place.

Thanks for letting me know about the tag resets, that should make changing the
check in kasan_non_canonical_hook() easier.

-- 
Kind regards
Maciej Wieczór-Retman