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

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 16/06/2021 à 11:07, Marco Elver a écrit :
> On Wed, 16 Jun 2021 at 10:03, Daniel Axtens <dja@axtens.net> 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@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210616080244.51236-4-dja%40axtens.net.