[PATCH 4/5 v10] ARM: Initialize the mapping of KASan shadow memory

[Linus Walleij]

On Mon, Jun 29, 2020 at 4:37 PM Russell King - ARM Linux admin
<linux at armlinux.org.uk> wrote:
> On Mon, Jun 29, 2020 at 04:07:06PM +0200, Linus Walleij wrote:
> > Asking for help here!
> >
> > I have a problem with populating PTEs for the LPAE usecase using
> > Versatile Express Cortex A15 (TC1) in QEMU.
> >
> > In this loop of the patch:
> >
> > On Mon, Jun 15, 2020 at 11:05 AM Linus Walleij <linus.walleij at linaro.org> wrote:
> >
> > > +static void __init kasan_pte_populate(pmd_t *pmdp, unsigned long addr,
> > > +                                     unsigned long end, int node, bool early)
> > > +{
> > > +       unsigned long next;
> > > +       pte_t *ptep = pte_offset_kernel(pmdp, addr);
> >
> > (...)
> >
> > > +       do {
> > > +               next = pmd_addr_end(addr, end);
> > > +               kasan_pte_populate(pmdp, addr, next, node, early);
> > > +       } while (pmdp++, addr = next, addr != end && pmd_none(READ_ONCE(*pmdp)));
> >
> > I first populate the PMD for 0x6ee00000 .. 0x6f000000
> > and this works fine, and the PTEs are all initialized.
> > pte_offset_kernel() returns something reasonable.
> > (0x815F5000).
> >
> > Next the kernel processes the PMD for
> > 0x6f000000 .. 0x6f200000 and now I run into trouble,
> > because pte_offset_kernel() suddenly returns a NULL
> > pointer 0x00000000.
>
> That means there is no PTE table allocated which covers 0x6f000000.
>
> "pmdp" points at the previous level's table entry that points at the
> pte, and all pte_offset*() does is load that entry, convert it to a
> pte_t pointer type, and point it to the appropriate entry for the
> address.  So, pte_offset*() is an accessor that takes a pointer to
> the preceding level's entry for "addr", and returns a pointer to
> the pte_t entry in the last level of page table for "addr".
>
> It is the responsibility of the caller to pte_offset*() to ensure
> either by explicit tests, or prior knowledge, that pmd_val(*pmdp)
> is a valid PTE table entry.
>
> Since generic kernel code can't use "prior knowledge", it has to do
> the full checks (see, mm/vmalloc.c vunmap_pte_range() and higher
> levels etc using pmd_none_or_clear_bad() for example - whether you
> can use _clear_bad() depends whether you intend to clear "bad" entries.
> Beware that the 1MB sections on non-LPAE will appear as "bad" entries
> since we can't "walk" them to PTE level, and they're certainly not
> "none" entries.)

Spot on! I figured it out quickly with this hint.

Essentially I have some loops like this:

pmd_t *pmdp = pmd_offset(pudp, addr);

if (pmd_none(*pmdp)) {
    void *p = early ? kasan_early_shadow_pte :
kasan_alloc_block(PAGE_SIZE, node);
    ....
}

do {
    pmd_populate_kernel(&init_mm, pmdp, p);
    flush_pmd_entry(pmdp);
    next = pmd_addr_end(addr, end);
    kasan_pte_populate(pmdp, addr, next, node, early);
} while (pmdp++, addr = next, addr != end && pmd_none(READ_ONCE(*pmdp)));

I just had to move the i (pmd_node(*pmdp)) inside the loop and it all
starts working
fine.

What confuses me is that arm64 does it this way (checking pmdp outside the loop)
for all levels of the cache and it works (I suppose?) for them, but I
suspect it is
formally wrong.

I'll rewrite with the check inside the loop at all levels and retest
and resend, then
I hope this starts to work and look reasonable, finally.

Yours,
Linus Walleij