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

[Ard Biesheuvel <ardb@kernel.org>]

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:
> > trans_pgd_create_copy() can hit "VM_BUG_ON(mm != &init_mm)" in the
> > function pmd_populate_kernel().
>
> I think that's more important to fix than the PXN. You may want to
> change the subject to something like "Fix VM_BUG_ON ..."
>
> > p?d_populate() helpers resort to the input parameter mm_struct to decide
> > the attribute: UXN or PXN. And plus the fact, either hibernation or
> > kexec runs in privilege mode. So the bug can be fixed by assigning
> > &init_mm to the callsites of these helpers.
>
> That's the table PXN, so it's not a big deal if we miss it.
>
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Pasha Tatashin <pasha.tatashin@soleen.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Matthias Brugger <mbrugger@suse.com>
> > To: linux-arm-kernel@lists.infradead.org
>
> A Fixes: tag would be nice, we just need to decide which.
>
> > ---
> >  arch/arm64/mm/trans_pgd.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > 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.
>
> 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.

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