Re: [PATCH 1/2] riscv: Fix asan-stack clang build

From: Palmer Dabbelt
Date: Thu Oct 28 2021 - 11:11:15 EST

On Thu, 28 Oct 2021 00:13:06 PDT (-0700), alexandre.ghiti@xxxxxxxxxxxxx wrote:
On Thu, Oct 28, 2021 at 8:45 AM Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:

On Wed, 27 Oct 2021 22:34:32 PDT (-0700), alexandre.ghiti@xxxxxxxxxxxxx wrote:
> On Thu, Oct 28, 2021 at 7:30 AM Alexandre Ghiti
> <alexandre.ghiti@xxxxxxxxxxxxx> wrote:
>>
>> On Thu, Oct 28, 2021 at 7:02 AM Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:
>> >
>> > On Wed, 27 Oct 2021 21:15:28 PDT (-0700), alexandre.ghiti@xxxxxxxxxxxxx wrote:
>> > > On Thu, Oct 28, 2021 at 1:06 AM Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:
>> > >>
>> > >> On Tue, 26 Oct 2021 21:58:42 PDT (-0700), alexandre.ghiti@xxxxxxxxxxxxx wrote:
>> > >> > Nathan reported that because KASAN_SHADOW_OFFSET was not defined in
>> > >> > Kconfig, it prevents asan-stack from getting disabled with clang even
>> > >> > when CONFIG_KASAN_STACK is disabled: fix this by defining the
>> > >> > corresponding config.
>> > >> >
>> > >> > Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx>
>> > >> > Signed-off-by: Alexandre Ghiti <alexandre.ghiti@xxxxxxxxxxxxx>
>> > >> > ---
>> > >> > arch/riscv/Kconfig | 6 ++++++
>> > >> > arch/riscv/include/asm/kasan.h | 3 +--
>> > >> > arch/riscv/mm/kasan_init.c | 3 +++
>> > >> > 3 files changed, 10 insertions(+), 2 deletions(-)
>> > >> >
>> > >> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > >> > index c1abbc876e5b..79250b1ed54e 100644
>> > >> > --- a/arch/riscv/Kconfig
>> > >> > +++ b/arch/riscv/Kconfig
>> > >> > @@ -162,6 +162,12 @@ config PAGE_OFFSET
>> > >> > default 0xffffffff80000000 if 64BIT && MAXPHYSMEM_2GB
>> > >> > default 0xffffffe000000000 if 64BIT && MAXPHYSMEM_128GB
>> > >> >
>> > >> > +config KASAN_SHADOW_OFFSET
>> > >> > + hex
>> > >> > + depends on KASAN_GENERIC
>> > >> > + default 0xdfffffc800000000 if 64BIT
>> > >> > + default 0xffffffff if 32BIT
>> > >>
>> > >> I thought I posted this somewhere, but this is exactly what my first
>> > >> guess was. The problem is that it's hanging on boot for me. I don't
>> > >> really have anything exotic going on, it's just a defconfig with
>> > >> CONFIG_KASAN=y running in QEMU.
>> > >>
>> > >> Does this boot for you?
>> > >
>> > > Yes with the 2nd patch of this series which fixes the issue
>> > > encountered here. And that's true I copied/pasted this part of your
>> > > patch which was better than what I had initially done, sorry I should
>> > > have mentioned you did that, please add a Codeveloped-by or something
>> > > like that.

OK, those should probably be in the opposite order (though it looks like
they're inter-dependent, which makes things a bit trickier).

>> >
>> > Not sure if I'm missing something, but it's still not booting for me.
>> > I've put what I'm testing on palmer/to-test, it's these two on top of
>> > fixes and merged into Linus' tree
>> >
>> > * 6d7d351902ff - (HEAD -> to-test, palmer/to-test) Merge remote-tracking branch 'palmer/fixes' into to-test (7 minutes ago) <Palmer Dabbelt>
>> > |\
>> > | * 782551edf8f8 - (palmer/fixes) riscv: Fix CONFIG_KASAN_STACK build (6 hours ago) <Alexandre Ghiti>
>> > | * 47383e5b3c4f - riscv: Fix asan-stack clang build (6 hours ago) <Alexandre Ghiti>
>> > | * 64a19591a293 - (riscv/fixes) riscv: fix misalgned trap vector base address (9 hours ago) <Chen Lu>
>> > * | 1fc596a56b33 - (palmer/master, linus/master, linus/HEAD, master) Merge tag 'trace-v5.15-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace (11 hours ago) <Linus Torvalds>
>> >
>> > Am I missing something else?
>>
>> Hmm, that's weird, I have just done the same: cherry-picked both my
>> commits on top of fixes (64a19591a293) and it boots fine with KASAN
>> enabled. Maybe a config thing? I pushed my branch here:
>> https://github.com/AlexGhiti/riscv-linux/tree/int/alex/kasan_stack_fixes_rebase
>
> I pushed the config I use and that boots in that branch, maybe there's
> another issue somewhere.

CONFIG_KASAN_VMALLOC=n is what's causing the failure. I'm testing both
polarities of that, looks like your config has =y. I haven't looked any
further as I'm pretty much cooked for tonight, but if you don't have
time then I'll try to find some time tomorrow.


Arf, that was obvious and just under my nose: without KASAN_VMALLOC,
kasan_populate_early_shadow is called and creates the same issue that
the second patch fixes.

I'll send a v2 today and try to swap both patches to avoid having a
non-bootable kernel commit.

Thanks.


Alex

>
>>
>> >
>> > >
>> > > Thanks,
>> > >
>> > > Alex
>> > >
>> > >>
>> > >> > +
>> > >> > config ARCH_FLATMEM_ENABLE
>> > >> > def_bool !NUMA
>> > >> >
>> > >> > diff --git a/arch/riscv/include/asm/kasan.h b/arch/riscv/include/asm/kasan.h
>> > >> > index a2b3d9cdbc86..b00f503ec124 100644
>> > >> > --- a/arch/riscv/include/asm/kasan.h
>> > >> > +++ b/arch/riscv/include/asm/kasan.h
>> > >> > @@ -30,8 +30,7 @@
>> > >> > #define KASAN_SHADOW_SIZE (UL(1) << ((CONFIG_VA_BITS - 1) - KASAN_SHADOW_SCALE_SHIFT))
>> > >> > #define KASAN_SHADOW_START KERN_VIRT_START
>> > >> > #define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
>> > >> > -#define KASAN_SHADOW_OFFSET (KASAN_SHADOW_END - (1ULL << \
>> > >> > - (64 - KASAN_SHADOW_SCALE_SHIFT)))
>> > >> > +#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
>> > >> >
>> > >> > void kasan_init(void);
>> > >> > asmlinkage void kasan_early_init(void);
>> > >> > diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c
>> > >> > index d7189c8714a9..8175e98b9073 100644
>> > >> > --- a/arch/riscv/mm/kasan_init.c
>> > >> > +++ b/arch/riscv/mm/kasan_init.c
>> > >> > @@ -17,6 +17,9 @@ asmlinkage void __init kasan_early_init(void)
>> > >> > uintptr_t i;
>> > >> > pgd_t *pgd = early_pg_dir + pgd_index(KASAN_SHADOW_START);
>> > >> >
>> > >> > + BUILD_BUG_ON(KASAN_SHADOW_OFFSET !=
>> > >> > + KASAN_SHADOW_END - (1UL << (64 - KASAN_SHADOW_SCALE_SHIFT)));
>> > >> > +
>> > >> > for (i = 0; i < PTRS_PER_PTE; ++i)
>> > >> > set_pte(kasan_early_shadow_pte + i,
>> > >> > mk_pte(virt_to_page(kasan_early_shadow_page),