linux-kernel - Re: [PATCH v3 1/2] arm64/mm: Allow __create_pgd_mapping() to propagate pgtable

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: <5280450d-e18e-4362-b48c-0f759c8d37e5@arm.com>
Date: Thu, 16 Oct 2025 11:35:20 +0530
From: Dev Jain <dev.jain@....com>
To: Kevin Brodsky <kevin.brodsky@....com>, Linu Cherian
 <linu.cherian@....com>, Catalin Marinas <catalin.marinas@....com>,
 Will Deacon <will@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
 Ryan Roberts <ryan.roberts@....com>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Cc: Anshuman Khandual <anshuman.khandual@....com>,
 Zhenhua Huang <quic_zhenhuah@...cinc.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Yang Shi <yang@...amperecomputing.com>,
 Chaitanya S Prakash <chaitanyas.prakash@....com>, stable@...r.kernel.org
Subject: Re: [PATCH v3 1/2] arm64/mm: Allow __create_pgd_mapping() to
 propagate pgtable_alloc() errors


On 15/10/25 9:35 pm, Kevin Brodsky wrote:
> On 15/10/2025 13:27, Linu Cherian wrote:
>> From: Chaitanya S Prakash <chaitanyas.prakash@....com>
>>
>> arch_add_memory() is used to hotplug memory into a system but as a part
>> of its implementation it calls __create_pgd_mapping(), which uses
>> pgtable_alloc() in order to build intermediate page tables. As this path
>> was initally only used during early boot pgtable_alloc() is designed to
>> BUG_ON() on failure. However, in the event that memory hotplug is
>> attempted when the system's memory is extremely tight and the allocation
>> were to fail, it would lead to panicking the system, which is not
>> desirable. Hence update __create_pgd_mapping and all it's callers to be
>> non void and propagate -ENOMEM on allocation failure to allow system to
>> fail gracefully.
>>
>> But during early boot if there is an allocation failure, we want the
>> system to panic, hence create a wrapper around __create_pgd_mapping()
>> called early_create_pgd_mapping() which is designed to panic, if ret
>> is non zero value. All the init calls are updated to use this wrapper
>> rather than the modified __create_pgd_mapping() to restore
>> functionality.
>>
>> Fixes: 4ab215061554 ("arm64: Add memory hotplug support")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@....com>
>> Signed-off-by: Linu Cherian <linu.cherian@....com>
> A couple more nits below (sorry I didn't catch them earlier), but otherwise:
>
> Reviewed-by: Kevin Brodsky <kevin.brodsky@....com>
>
>> ---
>> Changelog:
>>
>> v3:
>> * Fixed a maybe-uninitialized case in alloc_init_pud
>> * Added Fixes tag and CCed stable
>> * Few other trivial cleanups
>>
>>   arch/arm64/mm/mmu.c | 210 ++++++++++++++++++++++++++++----------------
>>   1 file changed, 132 insertions(+), 78 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index b8d37eb037fc..638cb4df31a9 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -49,6 +49,8 @@
>>   #define NO_CONT_MAPPINGS	BIT(1)
>>   #define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
>>   
>> +#define INVALID_PHYS_ADDR	(-1ULL)
>> +
>>   DEFINE_STATIC_KEY_FALSE(arm64_ptdump_lock_key);
>>   
>>   u64 kimage_voffset __ro_after_init;
>> @@ -194,11 +196,11 @@ static void init_pte(pte_t *ptep, unsigned long addr, unsigned long end,
>>   	} while (ptep++, addr += PAGE_SIZE, addr != end);
>>   }
>>   
>> -static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
>> -				unsigned long end, phys_addr_t phys,
>> -				pgprot_t prot,
>> -				phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>> -				int flags)
>> +static int alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
>> +			       unsigned long end, phys_addr_t phys,
>> +			       pgprot_t prot,
>> +			       phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>> +			       int flags)
>>   {
>>   	unsigned long next;
>>   	pmd_t pmd = READ_ONCE(*pmdp);
>> @@ -213,6 +215,8 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
>>   			pmdval |= PMD_TABLE_PXN;
>>   		BUG_ON(!pgtable_alloc);
>>   		pte_phys = pgtable_alloc(TABLE_PTE);
>> +		if (pte_phys == INVALID_PHYS_ADDR)
>> +			return -ENOMEM;
>>   		ptep = pte_set_fixmap(pte_phys);
>>   		init_clear_pgtable(ptep);
>>   		ptep += pte_index(addr);
>> @@ -244,12 +248,15 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
>>   	 * walker.
>>   	 */
>>   	pte_clear_fixmap();
>> +
>> +	return 0;
>>   }
>>   
>> -static void init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
>> -		     phys_addr_t phys, pgprot_t prot,
>> -		     phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags)
>> +static int init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
>> +		    phys_addr_t phys, pgprot_t prot,
>> +		    phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags)
>>   {
>> +	int ret;
> Nit: that could be added to the else block instead (makes it clearer
> it's not used for the final return, that got me confused when re-reading
> this patch).

+1.

Reviewed-by: Dev Jain <dev.jain@....com>

>
>
> [...]