On 2025-02-22 at 16:06:02 +0100, Andrey Konovalov wrote: >On Fri, Feb 21, 2025 at 2:12 PM Maciej Wieczor-Retman ><maciej.wieczor-retman@xxxxxxxxx> wrote: >> >> >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. > >> >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. > >> 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 > >> > 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
