Re: [PATCH] arm64: mm: assign PXN permission in trans_pgd

[Pingfan Liu <kernelfans@gmail.com>]

On Thu, Nov 11, 2021 at 06:18:17PM +0000, Catalin Marinas wrote:
> On Thu, Nov 11, 2021 at 05:13:02PM +0100, Ard Biesheuvel wrote:
> > On Thu, 11 Nov 2021 at 17:09, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > On Thu, Nov 11, 2021 at 02:03:16PM +0800, Pingfan Liu wrote:
> > > > diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
> > > > index d7da8ca40d2e..7b43d179e94d 100644
> > > > --- a/arch/arm64/mm/trans_pgd.c
> > > > +++ b/arch/arm64/mm/trans_pgd.c
> > > > @@ -67,7 +67,7 @@ static int copy_pte(struct trans_pgd_info *info, pmd_t *dst_pmdp,
> > > >       dst_ptep = trans_alloc(info);
> > > >       if (!dst_ptep)
> > > >               return -ENOMEM;
> > > > -     pmd_populate_kernel(NULL, dst_pmdp, dst_ptep);
> > > > +     pmd_populate_kernel(&init_mm, dst_pmdp, dst_ptep);
> > > >       dst_ptep = pte_offset_kernel(dst_pmdp, start);
> > >
> > > Commit 5de59884ac0e ("arm64: trans_pgd: pass NULL instead of init_mm to
> > > *_populate functions") actually replaced &init_mm with NULL. Commit
> > > 59511cfd08f3 ("arm64: mm: use XN table mapping attributes for
> > > user/kernel mappings") introduced the VM_BUG_ON.

I will include this history in V2.
> > >
> > > So we either revert Pasha's trans_pgd commit (as per your patch) or we
> > > fix Ard's commit to make it aware of a NULL mm and either skip the table
> > > UXN altogether or treat it as init_mm.
> > >
> > 
> > That code in pmd_populate_kernel() does not actually *use* the mm
> > argument at all, it just BUGs if its value != init_mm. So we can just
> > relax the BUG() to disregard mm ==  NULL.

This seems to be more reasonable solution than my V1.
> 
> We also have pud_populate() that ends up setting PUD_TABLE_PXN when mm
> is NULL. Fortunately we don't execute anything from those mappings, they
> are just a copy of the linear map.
> 
Also included to V2.

Thank both of you for kindly review.

Regards,

	Pingfan
> BTW, in line with PAGE_KERNEL, pud_populate() could set both UXN and PXN
> for init_mm mappings.
> 
> -- 
> Catalin
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel