Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 22 Mar 2021 13:59:34 +1100
From:   Balbir Singh <bsingharora@gmail.com>
To:     Daniel Axtens <dja@axtens.net>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        linuxppc-dev@lists.ozlabs.org, kasan-dev@googlegroups.com,
        christophe.leroy@csgroup.eu, aneesh.kumar@linux.ibm.com
Subject: Re: [PATCH v11 6/6] powerpc: Book3S 64-bit outline-only KASAN support
Message-ID: <20210322025934.GG77072@balbir-desktop>
References: <20210319144058.772525-1-dja@axtens.net>
 <20210319144058.772525-7-dja@axtens.net>
 <20210320060259.GF77072@balbir-desktop>
 <87o8fcatxv.fsf@dja-thinkpad.axtens.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87o8fcatxv.fsf@dja-thinkpad.axtens.net>
Precedence: bulk

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
 
> >> + * 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.

Balbir Singh.