Messages in this thread | | | Date | Mon, 4 Apr 2022 11:51:13 +0100 | Subject | Re: [PATCH] arm64: mm: fix pmd_leaf() | From | Steven Price <> |
| |
On 04/04/2022 10:19, Will Deacon wrote: > On Sun, Apr 03, 2022 at 10:49:28AM +0800, Muchun Song wrote: >> The pmd_leaf() is used to test a leaf mapped PMD, however, it misses >> the PROT_NONE mapped PMD on arm64. Fix it. A real world issue [1] >> caused by this was reported by Qian Cai. >> >> Link: https://patchwork.kernel.org/comment/24798260/ [1] >> Fixes: 8aa82df3c123 ("arm64: mm: add p?d_leaf() definitions") >> Reported-by: Qian Cai <quic_qiancai@quicinc.com> >> Signed-off-by: Muchun Song <songmuchun@bytedance.com> >> --- >> arch/arm64/include/asm/pgtable.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >> index 94e147e5456c..09eaae46a19b 100644 >> --- a/arch/arm64/include/asm/pgtable.h >> +++ b/arch/arm64/include/asm/pgtable.h >> @@ -535,7 +535,7 @@ extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn, >> PMD_TYPE_TABLE) >> #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ >> PMD_TYPE_SECT) >> -#define pmd_leaf(pmd) pmd_sect(pmd) >> +#define pmd_leaf(pmd) (pmd_present(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) >> #define pmd_bad(pmd) (!pmd_table(pmd)) >> >> #define pmd_leaf_size(pmd) (pmd_cont(pmd) ? CONT_PMD_SIZE : PMD_SIZE) > > A bunch of the users of pmd_leaf() already check pmd_present() -- is it > documented that we need to handle this check inside the macro? afaict x86 > doesn't do this either.
The documentation is with the fallback implementations that always return 0:
> /* > * p?d_leaf() - true if this entry is a final mapping to a physical address. > * This differs from p?d_huge() by the fact that they are always available (if > * the architecture supports large pages at the appropriate level) even > * if CONFIG_HUGETLB_PAGE is not defined. > * Only meaningful when called on a valid entry. > */
I guess the term "valid entry" is a bit vague but my intention was that meant p?d_present().
I have to admit I hadn't considered PROT_NONE mappings before.
Steve
| |