Message-ID: <uov3nar7yt7p3gb76mrmtw6fjfbxm5nmurn3hl72bkz6qwsfmv@ztvxz235oggw>
Date: Tue, 25 Feb 2025 18:15:27 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
To: Andrey Konovalov <andreyknvl@...il.com>
CC: <kees@...nel.org>, <julian.stecklina@...erus-technology.de>,
	<kevinloughlin@...gle.com>, <peterz@...radead.org>, <tglx@...utronix.de>,
	<justinstitt@...gle.com>, <catalin.marinas@....com>,
	<wangkefeng.wang@...wei.com>, <bhe@...hat.com>, <ryabinin.a.a@...il.com>,
	<kirill.shutemov@...ux.intel.com>, <will@...nel.org>, <ardb@...nel.org>,
	<jason.andryuk@....com>, <dave.hansen@...ux.intel.com>,
	<pasha.tatashin@...een.com>, <ndesaulniers@...gle.com>,
	<guoweikang.kernel@...il.com>, <dwmw@...zon.co.uk>, <mark.rutland@....com>,
	<broonie@...nel.org>, <apopple@...dia.com>, <bp@...en8.de>,
	<rppt@...nel.org>, <kaleshsingh@...gle.com>, <richard.weiyang@...il.com>,
	<luto@...nel.org>, <glider@...gle.com>, <pankaj.gupta@....com>,
	<pawan.kumar.gupta@...ux.intel.com>, <kuan-ying.lee@...onical.com>,
	<tony.luck@...el.com>, <tj@...nel.org>, <jgross@...e.com>,
	<dvyukov@...gle.com>, <baohua@...nel.org>, <samuel.holland@...ive.com>,
	<dennis@...nel.org>, <akpm@...ux-foundation.org>,
	<thomas.weissschuh@...utronix.de>, <surenb@...gle.com>,
	<kbingham@...nel.org>, <ankita@...dia.com>, <nathan@...nel.org>,
	<ziy@...dia.com>, <xin@...or.com>, <rafael.j.wysocki@...el.com>,
	<andriy.shevchenko@...ux.intel.com>, <cl@...ux.com>, <jhubbard@...dia.com>,
	<hpa@...or.com>, <scott@...amperecomputing.com>, <david@...hat.com>,
	<jan.kiszka@...mens.com>, <vincenzo.frascino@....com>, <corbet@....net>,
	<maz@...nel.org>, <mingo@...hat.com>, <arnd@...db.de>, <ytcoode@...il.com>,
	<xur@...gle.com>, <morbo@...gle.com>, <thiago.bauermann@...aro.org>,
	<linux-doc@...r.kernel.org>, <kasan-dev@...glegroups.com>,
	<linux-kernel@...r.kernel.org>, <llvm@...ts.linux.dev>, <linux-mm@...ck.org>,
	<linux-arm-kernel@...ts.infradead.org>, <x86@...nel.org>
Subject: Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

On 2025-02-22 at 16:07:20 +0100, Andrey Konovalov wrote:
>On Fri, Feb 21, 2025 at 4:11 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@...el.com> wrote:
>>
>> On 2025-02-20 at 00:31:08 +0100, Andrey Konovalov wrote:
>> >On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
>> ><maciej.wieczor-retman@...el.com> wrote:
>> >>
>> >> On x86, generic KASAN is setup in a way that needs a single
>> >> KASAN_SHADOW_OFFSET value for both 4 and 5 level paging. It's required
>> >> to facilitate boot time switching and it's a compiler ABI so it can't be
>> >> changed during runtime.
>> >>
>> >> Software tag-based mode doesn't tie shadow start and end to any linear
>> >> addresses as part of the compiler ABI so it can be changed during
>> >> runtime.
>> >
>> >KASAN_SHADOW_OFFSET is passed to the compiler via
>> >hwasan-mapping-offset, see scripts/Makefile.kasan (for the INLINE
>> >mode). So while we can change its value, it has to be known at compile
>> >time. So I don't think using a runtime constant would work.
>>
>> I don't know about arm64, but this doesn't seem to work right now on x86.
>
>You mean it _does_ seem to work? Or otherwise if runtime constant
>doesn't work on x86, then we shouldn't use it?

I mean in my tests, with setting offset in runtime, everything works correctly
in inline mode. Even though hwasan-mapping-offset ends up empty and doesn't end
up in CFLAGS_KASAN. I assume this means that the inline mode is pretty much the
same as outline mode with the runtime offset setting?

I also tested if hwasan-mapping-offset does anything if I passed random values
to it by hardcoding them in the makefile and still everything seemed to work
just fine. Therefore I assumed that this option doesn't have any effect on x86.

>
>> I
>> think I recall that hwasan-mapping-offset isn't implemented on the x86 LLVM or
>> something like that? I'm sure I saw some note about it a while ago on the
>> internet but I couldn't find it today.
>
>In LLVM sources, ShadowBase gets calculated [1] based on
>Mapping.Offset [2], which is in turn taken [3] from
>hwasan-mapping-offset [4]. And then ShadowBase is used to calculate
>[5] the shadow memory address.
>
>All of this happens in the common code, so this should affect both x86
>and arm64.
>
>[1] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1305
>[2] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L761
>[3] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1863
>[4] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L171
>[5] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L899

Hmm indeed it does. Then I'm not sure why I didn't crash when I started putting
in random variables. I'll dive into assembly and see what's up in there.

>
>>
>> Anyway if KASAN_SHADOW_OFFSET is not set at compile time it defaults to nothing
>> and just doesn't get passed into kasan-params a few lines below. I assume that
>> result seems a little too makeshift for runtime const to make sense here?
>
>Sorry, I don't understand this question.
>
>If hwasan-mapping-offset is not set properly, then in the inline
>instrumentation mode, the compiler won't generate the right
>instructions to calculate the shadow memory address.

I meant that if we end up with "hwasan-mapping-offset=" because
$(KASAN_SHADOW_OFFSET) didn't return anything, then
$(call check-args, cc-param, $(kasan_params)) drops hwasan-mapping-offset and
doesn't pass it to CFLAGS_KASAN. So I guess then it isn't an issue for the
compiler and inline mode is essentially outline mode.

But anyway I have an idea how to setup the x86 offset for tag-based mode so it
works for both paging modes. I did some testing and value
	0xffeffc0000000000
seems to work fine and has at least some of the benefits I was hoping for when
doing the runtime_const thing. It works in both paging modes because in 5 levels
it's just a little bit below the 0xffe0000000000000 that I was thinking about
first and in 4 levels, because of LAM, it becomes 0xfffffc0000000000 (because in
4 level paging bits 62:48 are masked from address translation. So it's the same
as the end of generic mode shadow memory space.

The alignment doesn't fit the shadow memory size so it's not optimal but I'm not
sure it can be if we want to have the inline mode and python scripts working at
the same time. At the very least I think the KASAN_SHADOW_END won't collide with
other things in the tab-based mode in 5 level paging mode, so no extra steps are
needed (arch/x86/mm/kasan_init_64.c in kasan_init()).

Do you see any problems with this offset for x86 tag-based mode?


Btw I think kasan_check_range() can be optimized on x86 if we use
addr_has_metadata() that doesn't use KASAN_SHADOW_START. Getting rid of it from
the implementation will remove pgtable_l5_enabled() which is pretty slow so
kasan_check_range() which is called a lot would probably work much faster.
Do you see any way in which addr_has_metadata() will make sense but won't use
KASAN_SHADOW_START? Every one of my ideas ends up using pgtable_l5_enabled()
because the metadata can have 6 or 15 bits depending on paging level.

-- 
Kind regards
Maciej Wieczór-Retman

Powered by blists - more mailing lists