Re: [PATCH v13 3/3] kasan: define and use MAX_PTRS_PER_* for early shadow tables

[Christophe Leroy <christophe.leroy@xxxxxxxxxx>]

Le 16/06/2021 à 11:07, Marco Elver a écrit :
> On Wed, 16 Jun 2021 at 10:03, Daniel Axtens <dja@xxxxxxxxxx> wrote:
> [...]
> > diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> > index 768d7d342757..fd65f477ac92 100644
> > --- a/include/linux/kasan.h
> > +++ b/include/linux/kasan.h
> > @@ -40,10 +40,22 @@ struct kunit_kasan_expectation {
> >   #define PTE_HWTABLE_PTRS 0
> >   #endif
> >
> > +#ifndef MAX_PTRS_PER_PTE
> > +#define MAX_PTRS_PER_PTE PTRS_PER_PTE
> > +#endif
> > +
> > +#ifndef MAX_PTRS_PER_PMD
> > +#define MAX_PTRS_PER_PMD PTRS_PER_PMD
> > +#endif
> > +
> > +#ifndef MAX_PTRS_PER_PUD
> > +#define MAX_PTRS_PER_PUD PTRS_PER_PUD
> > +#endif
>
> This is introducing new global constants in a <linux/..> header. It
> feels like this should be in <linux/pgtable.h> together with a
> comment. Because <linux/kasan.h> is actually included in
> <linux/slab.h>, most of the kernel will get these new definitions.
> That in itself is fine, but it feels wrong that the KASAN header
> introduces these.
>
> Thoughts?
>
> Sorry for only realizing this now.

My idea here was to follow the same road as MAX_PTRS_PER_P4D, added by commit 
https://github.com/linuxppc/linux/commit/c65e774f

That commit spread MAX_PTRS_PER_P4D everywhere.

Instead of doing the same, we found that it would be better to define a fallback for when the 
architecture doesn't define MAX_PTRS_PER_PxD . Now, it can be made more global in pgtable.h, in that 
case I'd suggest to also include MAX_PTRS_PER_P4D in the dance and avoid architectures like s390 
having to define it, or even not defining it either in asm-generic/pgtable-nop4d.h

Christophe

> Thanks,
> -- Marco
>
> >   extern unsigned char kasan_early_shadow_page[PAGE_SIZE];
> > -extern pte_t kasan_early_shadow_pte[PTRS_PER_PTE + PTE_HWTABLE_PTRS];
> > -extern pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD];
> > -extern pud_t kasan_early_shadow_pud[PTRS_PER_PUD];
> > +extern pte_t kasan_early_shadow_pte[MAX_PTRS_PER_PTE + PTE_HWTABLE_PTRS];
> > +extern pmd_t kasan_early_shadow_pmd[MAX_PTRS_PER_PMD];
> > +extern pud_t kasan_early_shadow_pud[MAX_PTRS_PER_PUD];
> >   extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
> >
> >   int kasan_populate_early_shadow(const void *shadow_start,
> > diff --git a/mm/kasan/init.c b/mm/kasan/init.c
> > index 348f31d15a97..cc64ed6858c6 100644
> > --- a/mm/kasan/init.c
> > +++ b/mm/kasan/init.c
> > @@ -41,7 +41,7 @@ static inline bool kasan_p4d_table(pgd_t pgd)
> >   }
> >   #endif
> >   #if CONFIG_PGTABLE_LEVELS > 3
> > -pud_t kasan_early_shadow_pud[PTRS_PER_PUD] __page_aligned_bss;
> > +pud_t kasan_early_shadow_pud[MAX_PTRS_PER_PUD] __page_aligned_bss;
> >   static inline bool kasan_pud_table(p4d_t p4d)
> >   {
> >          return p4d_page(p4d) == virt_to_page(lm_alias(kasan_early_shadow_pud));
> > @@ -53,7 +53,7 @@ static inline bool kasan_pud_table(p4d_t p4d)
> >   }
> >   #endif
> >   #if CONFIG_PGTABLE_LEVELS > 2
> > -pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD] __page_aligned_bss;
> > +pmd_t kasan_early_shadow_pmd[MAX_PTRS_PER_PMD] __page_aligned_bss;
> >   static inline bool kasan_pmd_table(pud_t pud)
> >   {
> >          return pud_page(pud) == virt_to_page(lm_alias(kasan_early_shadow_pmd));
> > @@ -64,7 +64,7 @@ static inline bool kasan_pmd_table(pud_t pud)
> >          return false;
> >   }
> >   #endif
> > -pte_t kasan_early_shadow_pte[PTRS_PER_PTE + PTE_HWTABLE_PTRS]
> > +pte_t kasan_early_shadow_pte[MAX_PTRS_PER_PTE + PTE_HWTABLE_PTRS]
> >          __page_aligned_bss;
> >
> >   static inline bool kasan_pte_table(pmd_t pmd)
> > --
> > 2.30.2
> >
> > --
> > You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210616080244.51236-4-dja%40axtens.net.