Re: [PATCH 6/8] sh/mm: Stop using [p4d|pud|pmd]_ERROR()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] `__get_pte_phys()` can return `NULL` on error, but its callers (`set_pte_phys()` and `clear_pte_phys()`) immediately dereference the returned pointer without checking, leading to a guaranteed kernel crash.
- [High] `__get_pte_phys()` uses allocation functions (`p4d_alloc`, `pud_alloc`, `pmd_alloc`) passing `NULL` for the `mm` argument, which will cause a kernel crash if allocation is actually needed.
--

commit 0098935d861a1c9cf1496880ca35e1a49aecdf51
Author: Anshuman Khandual <anshuman.khandual@arm.com>

sh/mm: Stop using [p4d|pud|pmd]_ERROR()

This commit replaces the use of [p4d|pud|pmd]_ERROR() with pr_err() in
__get_pte_phys() when page table allocations fail. This avoids a NULL
pointer dereference when attempting to pass the failed allocation result
to the error macros.

> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index 9466ae6f9f164..8d65e60688dcb 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -59,19 +59,19 @@ static pte_t *__get_pte_phys(unsigned long addr)
>  
>  	p4d = p4d_alloc(NULL, pgd, addr);

[Severity: High]
This isn't a bug introduced by this patch, but passing NULL for the mm
argument to p4d_alloc() will cause a kernel panic if allocation is actually
needed. The slow path requires a valid mm_struct to acquire the page table
lock. Should this be passing &init_mm instead?

>  	if (unlikely(!p4d)) {
> -		p4d_ERROR(*p4d);
> +		pr_err("allocating p4d table failed\n");
>  		return NULL;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while this fixes a NULL
pointer dereference here in the callee, the callers still unconditionally
dereference the returned pointer.

For example, in set_pte_phys():

    pte = __get_pte_phys(addr);
    if (!pte_none(*pte)) {

And in clear_pte_phys():

    pte = __get_pte_phys(addr);
    
    if (pgprot_val(prot) & _PAGE_WIRED)
        tlb_unwire_entry();
        
    set_pte(pte, pfn_pte(0, __pgprot(0)));

Could these callers be updated to check if the returned pte is NULL
before dereferencing it to avoid guaranteed crashes on the error paths?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831054331.625505-1-anshuman.khandual@arm.com?part=6