linux-kernel - Re: [PATCH] mm/hugetlb: LoongArch: Return NULL from huge_pte

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: <aAqHkdXt50van31B@x1.local>
Date: Thu, 24 Apr 2025 14:48:49 -0400
From: Peter Xu <peterx@...hat.com>
To: Ming Wang <wangming01@...ngson.cn>
Cc: Huacai Chen <chenhuacai@...nel.org>, WANG Xuerui <kernel@...0n.name>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hongchen Zhang <zhanghongchen@...ngson.cn>,
	loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
	lixuefeng@...ngson.cn, gaojuxin@...ngson.cn, chenhuacai@...ngson.cn
Subject: Re: [PATCH] mm/hugetlb: LoongArch: Return NULL from
 huge_pte_offset() for none PMD

On Thu, Apr 24, 2025 at 04:30:37PM +0800, Ming Wang wrote:
> LoongArch's huge_pte_offset() currently returns a pointer to a PMD slot
> even if the underlying entry points to invalid_pte_table (indicating no
> mapping). Callers like smaps_hugetlb_range() fetch this invalid entry
> value (the address of invalid_pte_table) via this pointer.

So it's in smaps_hugetlb_range() context, then..

> 
> The generic is_swap_pte() check then incorrectly identifies this address
> as a swap entry on LoongArch, because it satisfies the !pte_present()
> && !pte_none() conditions. This misinterpretation, combined with a
> coincidental match by is_migration_entry() on the address bits, leads
> to kernel crashes in pfn_swap_entry_to_page().

.. you mentioned !pte_none() is checked.  Could you explain why it's
pte_none() rather than huge_pte_none()?  As I saw loongarch has exactly
this..

static inline int huge_pte_none(pte_t pte)
{
	unsigned long val = pte_val(pte) & ~_PAGE_GLOBAL;
	return !val || (val == (unsigned long)invalid_pte_table);
}

I'm not familiar with loongarch's invalid_pte_table, but I would expect at
least for now in hugetlb specific paths it should be reported as none even
without this patch.

One more thing to mention is the kernel (AFAIU) is trying to moving away
from hugetlb specific pgtable parsing to generic pgtable walkers.  It means
it could happen at some point where the kernel walks the hugetlb pgtables
using normal pgtable APIs.  I'm not yet sure what would happen then, but
maybe at some point the invalid_pte_table check is needed in pte_none() too
for loongarch.

Thanks,

> 
> Fix this at the architecture level by modifying huge_pte_offset() to
> check the PMD entry's content using pmd_none() before returning. If the
> entry is none (i.e., it points to invalid_pte_table), return NULL
> instead of the pointer to the slot.
> 
> Co-developed-by: Hongchen Zhang <zhanghongchen@...ngson.cn>
> Signed-off-by: Hongchen Zhang <zhanghongchen@...ngson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> Signed-off-by: Ming Wang <wangming01@...ngson.cn>
> ---
>  arch/loongarch/mm/hugetlbpage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/loongarch/mm/hugetlbpage.c b/arch/loongarch/mm/hugetlbpage.c
> index e4068906143b..cea84d7f2b91 100644
> --- a/arch/loongarch/mm/hugetlbpage.c
> +++ b/arch/loongarch/mm/hugetlbpage.c
> @@ -47,7 +47,7 @@ pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr,
>  				pmd = pmd_offset(pud, addr);
>  		}
>  	}
> -	return (pte_t *) pmd;
> +	return pmd_none(pmdp_get(pmd)) ? NULL : (pte_t *) pmd;
>  }
>  
>  uint64_t pmd_to_entrylo(unsigned long pmd_val)
> -- 
> 2.43.0
> 

-- 
Peter Xu