Messages in this thread | | | Date | Wed, 13 Apr 2022 11:19:41 +0100 | From | Will Deacon <> | Subject | Re: [PATCH v2] arm64: mm: fix pmd_leaf() |
| |
On Mon, Apr 11, 2022 at 08:26:53PM +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> > --- > v2: > - Replace pmd_present() with pmd_val() since we expect pmd_leaf() works > well on non-present pmd case. > > 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 ad9b221963d4..00cdd2d895d3 100644 > --- a/arch/arm64/include/asm/pgtable.h > +++ b/arch/arm64/include/asm/pgtable.h > @@ -551,7 +551,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_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) > #define pmd_bad(pmd) (!pmd_table(pmd))
I'm still trying to get my head around the desired semantics here.
If we want to fix the original report, then we need to take PROT_NONE entries into account. The easiest way to do that is, as you originally suggested, by using pmd_present():
#define pmd_leaf(pmd) (pmd_present(pmd) && !pmd_table(pmd))
But now you seem to be saying that !pmd_present() entries should also be considered as pmd_leaf() -- is there a real need for that?
If so, then I think this simply becomes:
#define pmd_leaf(pmd) (!pmd_table(pmd))
which is, amusingly, identical to pmd_bad().
The documentation/comment that Steven referred to also desperately needs clarifying as it currently states:
"Only meaningful when called on a valid entry."
whatever that means.
Finally, if this has implications beyond PROT_NONE (as I think you're suggesting in your v2) then pud_leaf() probably needs similar treatment. And we can remove pmd_sect() altogether if we no longer need it.
Will
| |