Re: [PATCH v11 6/6] powerpc: Book3S 64-bit outline-only KASAN support

Daniel Axtens
2021-03-22T05:52:07Z

Balbir Singh <bsingharora@gmail.com> writes:

> On Mon, Mar 22, 2021 at 11:55:08AM +1100, Daniel Axtens wrote:
>> Hi Balbir,
>>
>> > Could you highlight the changes from
>> > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20170729140901.5887-1-bsingharora@gmail.com/?
>> >
>> > Feel free to use my signed-off-by if you need to and add/update copyright
>> > headers if appropriate.
>>
>> There's not really anything in common any more:
>>
>> - ppc32 KASAN landed, so there was already a kasan.h for powerpc, the
>> explicit memcpy changes, the support for non-instrumented files,
>> prom_check.sh, etc. all already landed.
>>
>> - I locate the shadow region differently and don't resize any virtual
>> memory areas.
>>
>> - The ARCH_DEFINES_KASAN_ZERO_PTE handling changed upstream and our
>> handling for that is now handled more by patch 3.
>>
>> - The outline hook is now an inline function rather than a #define.
>>
>> - The init function has been totally rewritten as it's gone from
>> supporting real mode to not supporting real mode and back.
>>
>> - The list of non-instrumented files has grown a lot.
>>
>> - There's new stuff: stack walking is now safe, KASAN vmalloc support
>> means modules are better supported now, ptdump works, and there's
>> documentation.
>>
>> It's been a while now, but I don't think when I started this process 2
>> years ago that I directly reused much of your code. So I'm not sure that
>> a signed-off-by makes sense here? Would a different tag (Originally-by?)
>> make more sense?
>>
>
> Sure

Will do.

>
>> >> + * The shadow ends before the highest accessible address
>> >> + * because we don't need a shadow for the shadow. Instead:
>> >> + * c00e000000000000 << 3 + a80e 0000 0000 0000 000 = c00fc00000000000
>> >
>> > The comment has one extra 0 in a80e.., I did the math and had to use
>> > the data from the defines :)
>>
>> 3 extra 0s, even! Fixed.
>>
>> >> +void __init kasan_init(void)
>> >> +{
>> >> + /*
>> >> + * We want to do the following things:
>> >> + * 1) Map real memory into the shadow for all physical memblocks
>> >> + * This takes us from c000... to c008...
>> >> + * 2) Leave a hole over the shadow of vmalloc space. KASAN_VMALLOC
>> >> + * will manage this for us.
>> >> + * This takes us from c008... to c00a...
>> >> + * 3) Map the 'early shadow'/zero page over iomap and vmemmap space.
>> >> + * This takes us up to where we start at c00e...
>> >> + */
>> >> +
>> >
>> > assuming we have
>> > #define VMEMMAP_END R_VMEMMAP_END
>> > and ditto for hash we probably need
>> >
>> > BUILD_BUG_ON(VMEMMAP_END + KASAN_SHADOW_OFFSET != KASAN_SHADOW_END);
>>
>> Sorry, I'm not sure what this is supposed to be testing? In what
>> situation would this trigger?
>>
>
> I am bit concerned that we have hard coded (IIR) 0xa80e... in the
> config, any changes to VMEMMAP_END, KASAN_SHADOW_OFFSET/END
> should be guarded.
>

Ah that makes sense. I'll come up with some test that should catch any
unsynchronised changes to VMEMMAP_END, KASAN_SHADOW_OFFSET or
KASAN_SHADOW_END.

Kind regards,
Daniel Axtens

> Balbir Singh.