Re: [PATCH] powerpc/kasan/book3s_64: warn when running with hash MMU

From: Michael Ellerman <mpe@ellerman.id.au>
To: Nathan Lynch <nathanl@linux.ibm.com>
Cc: "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	kasan-dev <kasan-dev@googlegroups.com>
Subject: Re: [PATCH] powerpc/kasan/book3s_64: warn when running with hash MMU
Date: Tue, 11 Oct 2022 21:00:25 +1100	[thread overview]
Message-ID: <87v8oqn0hy.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <8735bvbwgy.fsf@linux.ibm.com>

Nathan Lynch <nathanl@linux.ibm.com> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> + KASAN list
>>>
>>> Le 06/10/2022 à 06:10, Michael Ellerman a écrit :
>>>> Nathan Lynch <nathanl@linux.ibm.com> writes:
>>>>> kasan is known to crash at boot on book3s_64 with non-radix MMU. As
>>>>> noted in commit 41b7a347bf14 ("powerpc: Book3S 64-bit outline-only
>>>>> KASAN support"):
>>>>>
>>>>>    A kernel with CONFIG_KASAN=y will crash during boot on a machine
>>>>>    using HPT translation because not all the entry points to the
>>>>>    generic KASAN code are protected with a call to kasan_arch_is_ready().
>>>> 
>>>> I guess I thought there was some plan to fix that.
>>>
>>> I was thinking the same.
>>>
>>> Do we have a list of the said entry points to the generic code that are 
>>> lacking a call to kasan_arch_is_ready() ?
>>>
>>> Typically, the BUG dump below shows that kasan_byte_accessible() is 
>>> lacking the check. It should be straight forward to add 
>>> kasan_arch_is_ready() check to kasan_byte_accessible(), shouldn't it ?
>>
>> Yes :)
>>
>> And one other spot, but the patch below boots OK for me. I'll leave it
>> running for a while just in case there's a path I've missed.
>
> It works for me too, thanks (p8 pseries qemu).

It works but I still see the kasan shadow getting mapped, which we would
ideally avoid.

From PTDUMP:

---[ kasan shadow mem start ]---
0xc00f000000000000-0xc00f00000006ffff  0x00000000045e0000       448K         r  w       pte  valid  present        dirty  accessed
0xc00f3ffffffe0000-0xc00f3fffffffffff  0x0000000004d80000       128K         r  w       pte  valid  present        dirty  accessed

I haven't worked out how those are getting mapped.

> This avoids the boot-time oops, but kasan remains unimplemented for hash
> mmu. Raising the question: with the trivial crashes addressed, is the
> current message ('KASAN not enabled as it requires radix!') sufficient
> to notify developers (such as me, a week ago) who mean to use kasan on a
> book3s platform, unaware that it's radix-only? Would a WARN or something
> more prominent still be justified?
>
> I guess people will figure it out as soon as they think to search the
> kernel log for 'KASAN'...

Yeah, I think a warning is a bit too strong. I think that's more likely
to lead to bug reports than anything :)

cheers