答复: [外部邮件] Re: [PATCH] arm64: mm: refresh stale pmd snapshot after split_contpmd()

["Li,Rongqing" <lirongqing@baidu.com>]

> 
> 
> 
> On 6/25/26 4:39 AM, lirongqing wrote:
> > From: Li RongQing <lirongqing@baidu.com>
> >
> > split_contpmd() modifies the pmd entries in-place by clearing the CONT
> > bit, but the local 'pmd' variable still holds the old snapshot with
> > CONT set. The subsequent split_pmd() call uses this stale value to
> > derive the pgprot for the new PTE entries via pmd_pgprot(), causing
> > the resulting PTEs to be populated with incorrect protection bits.
> 
> If I read the code correctly, CONT bit is cleared by split_pmd(), then the bit
> may be set again for PTEs if we want to have cont ptes. So I don't see any
> problem, did I miss something?
> 

You are right, there's no functional issue with the current code.
However, I think explicitly re-reading the pmd is the safer and clearer 
approach — it makes the intent obvious (we need the post-modification state) 
rather than relying on the implicit assumption that "CONT bit doesn't affect pgprot."

Thanks


[Li,Rongqing] 


> Thanks,
> Yang
> 
> >
> > Fix this by re-reading the pmd from memory after split_contpmd()
> > returns in both call sites: split_kernel_leaf_mapping_locked() and
> > split_to_ptes_pmd_entry().
> >
> > Fixes: a166563e7ec3 ("arm64: mm: support large block mapping when
> > rodata=full")
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >   arch/arm64/mm/mmu.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index
> > 12e862c..e510336 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -746,8 +746,10 @@ static int
> split_kernel_leaf_mapping_locked(unsigned long addr)
> >   	if (!pmd_present(pmd))
> >   		goto out;
> >   	if (pmd_leaf(pmd)) {
> > -		if (pmd_cont(pmd))
> > +		if (pmd_cont(pmd)) {
> >   			split_contpmd(pmdp);
> > +			pmd = pmdp_get(pmdp);
> > +		}
> >   		/*
> >   		 * PMD: If addr is PMD aligned then addr already describes a
> >   		 * leaf boundary. Otherwise, split to contpte.
> > @@ -891,8 +893,10 @@ static int split_to_ptes_pmd_entry(pmd_t *pmdp,
> unsigned long addr,
> >   	int ret = 0;
> >
> >   	if (pmd_leaf(pmd)) {
> > -		if (pmd_cont(pmd))
> > +		if (pmd_cont(pmd)) {
> >   			split_contpmd(pmdp);
> > +			pmd = pmdp_get(pmdp);
> > +		}
> >   		ret = split_pmd(pmdp, pmd, gfp, false);
> >
> >   		/*