linux-kernel - Re: [PATCH 3/3] mm: Add p{g/4}d_leaf() in asm-generic/pgtable-nop{4/u}d.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56a0340a-2534-4d2e-92e4-cf27a6358b23@csgroup.eu>
Date: Tue, 9 Jul 2024 21:48:24 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Peter Xu <peterx@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Arnd Bergmann <arnd@...db.de>,
 linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
 linux-riscv@...ts.infradead.org, Oscar Salvador <osalvador@...e.de>
Subject: Re: [PATCH 3/3] mm: Add p{g/4}d_leaf() in
 asm-generic/pgtable-nop{4/u}d.h



Le 04/07/2024 à 16:48, Peter Xu a écrit :
> On Thu, Jul 04, 2024 at 08:30:05AM +0200, Christophe Leroy wrote:
>> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related
>> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h
>>
>> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them
>> erroneously defined by architectures that do not implement the
>> related page level.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>> ---
>>   include/asm-generic/pgtable-nop4d.h | 1 +
>>   include/asm-generic/pgtable-nopud.h | 1 +
>>   include/linux/pgtable.h             | 6 +++---
>>   3 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/asm-generic/pgtable-nop4d.h b/include/asm-generic/pgtable-nop4d.h
>> index 03b7dae47dd4..75c96bbc5a96 100644
>> --- a/include/asm-generic/pgtable-nop4d.h
>> +++ b/include/asm-generic/pgtable-nop4d.h
>> @@ -21,6 +21,7 @@ typedef struct { pgd_t pgd; } p4d_t;
>>   static inline int pgd_none(pgd_t pgd)		{ return 0; }
>>   static inline int pgd_bad(pgd_t pgd)		{ return 0; }
>>   static inline int pgd_present(pgd_t pgd)	{ return 1; }
>> +static inline int pgd_leaf(pgd_t pgd)		{ return 0; }
>>   static inline void pgd_clear(pgd_t *pgd)	{ }
>>   #define p4d_ERROR(p4d)				(pgd_ERROR((p4d).pgd))
>>   
>> diff --git a/include/asm-generic/pgtable-nopud.h b/include/asm-generic/pgtable-nopud.h
>> index eb70c6d7ceff..14aeb8ef2d8a 100644
>> --- a/include/asm-generic/pgtable-nopud.h
>> +++ b/include/asm-generic/pgtable-nopud.h
>> @@ -28,6 +28,7 @@ typedef struct { p4d_t p4d; } pud_t;
>>   static inline int p4d_none(p4d_t p4d)		{ return 0; }
>>   static inline int p4d_bad(p4d_t p4d)		{ return 0; }
>>   static inline int p4d_present(p4d_t p4d)	{ return 1; }
>> +static inline int p4d_leaf(p4d_t p4d)		{ return 0; }
>>   static inline void p4d_clear(p4d_t *p4d)	{ }
>>   #define pud_ERROR(pud)				(p4d_ERROR((pud).p4d))
>>   
>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>> index 2a6a3cccfc36..b27e66f542d6 100644
>> --- a/include/linux/pgtable.h
>> +++ b/include/linux/pgtable.h
>> @@ -1882,13 +1882,13 @@ typedef unsigned int pgtbl_mod_mask;
>>    * - It should cover all kinds of huge mappings (e.g., pXd_trans_huge(),
>>    *   pXd_devmap(), or hugetlb mappings).
>>    */
>> -#ifndef pgd_leaf
>> +#if !defined(__PAGETABLE_P4D_FOLDED) && !defined(pgd_leaf)
>>   #define pgd_leaf(x)	false
>>   #endif
>> -#ifndef p4d_leaf
>> +#if !defined(__PAGETABLE_PUD_FOLDED) && !defined(p4d_leaf)
>>   #define p4d_leaf(x)	false
>>   #endif
>> -#ifndef pud_leaf
>> +#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(pud_leaf)
>>   #define pud_leaf(x)	false
>>   #endif
>>   #ifndef pmd_leaf
> 
> Is it possible to do it the other way round, so that we can still rely on
> "ifdef pxx_leaf" to decide whether to provide a fallback, and define them
> properly when needed?

What do you mean by the "other way round" ? Did I do a mistake ? I can't 
see it.

The purpose here is:
- If the architecture has the said level and implements pXd_leaf(), 
that's fine
- If the architecture has the said level and doesn't implement 
pXd_leaf(), that's also fine, a fallback is provided.
- If the architecture doesn't have the said level but implements 
pXd_leaf(), it will conflict with the definition in 
include/asm-generic/pgtable-nopXd.h and the build will fail.

The purpose is to make sure architectures don't implement pXd_leaf() at 
the wrong level, for instance:
- an architecture without PMDs shall not implement anything else than 
pmd_leaf()
- an architecture without P4Ds shall not implement pgd_leaf().


> 
> IMHO it was a neat way to avoid worrying on any macro defined; it'll be as
> simple as "if function xxx not defined, let's define a fallback for xxx".
> Per my limited experience it helped a lot on avoid compile issues here and
> there..

That will still be the case.

This patch adds: "if function xxx is defined for wrong level, break the 
build"

Christophe