* [PATCH] arm64: mm: assign PXN permission in trans_pgd
@ 2021-11-11 6:03 Pingfan Liu
2021-11-11 16:09 ` Catalin Marinas
2021-11-12 5:22 ` [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd Pingfan Liu
0 siblings, 2 replies; 10+ messages in thread
From: Pingfan Liu @ 2021-11-11 6:03 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Pingfan Liu, Catalin Marinas, Will Deacon, Pasha Tatashin,
James Morse, Matthias Brugger
trans_pgd_create_copy() can hit "VM_BUG_ON(mm != &init_mm)" in the
function pmd_populate_kernel().
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.
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
---
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);
src_ptep = pte_offset_kernel(src_pmdp, start);
@@ -90,7 +90,7 @@ static int copy_pmd(struct trans_pgd_info *info, pud_t *dst_pudp,
dst_pmdp = trans_alloc(info);
if (!dst_pmdp)
return -ENOMEM;
- pud_populate(NULL, dst_pudp, dst_pmdp);
+ pud_populate(&init_mm, dst_pudp, dst_pmdp);
}
dst_pmdp = pmd_offset(dst_pudp, start);
@@ -126,7 +126,7 @@ static int copy_pud(struct trans_pgd_info *info, p4d_t *dst_p4dp,
dst_pudp = trans_alloc(info);
if (!dst_pudp)
return -ENOMEM;
- p4d_populate(NULL, dst_p4dp, dst_pudp);
+ p4d_populate(&init_mm, dst_p4dp, dst_pudp);
}
dst_pudp = pud_offset(dst_p4dp, start);
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: assign PXN permission in trans_pgd
2021-11-11 6:03 [PATCH] arm64: mm: assign PXN permission in trans_pgd Pingfan Liu
@ 2021-11-11 16:09 ` Catalin Marinas
2021-11-11 16:13 ` Ard Biesheuvel
2021-11-12 5:22 ` [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd Pingfan Liu
1 sibling, 1 reply; 10+ messages in thread
From: Catalin Marinas @ 2021-11-11 16:09 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-arm-kernel, Will Deacon, Pasha Tatashin, James Morse,
Matthias Brugger, Ard Biesheuvel
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.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: assign PXN permission in trans_pgd
2021-11-11 16:09 ` Catalin Marinas
@ 2021-11-11 16:13 ` Ard Biesheuvel
2021-11-11 18:18 ` Catalin Marinas
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2021-11-11 16:13 UTC (permalink / raw)
To: Catalin Marinas
Cc: Pingfan Liu, Linux ARM, Will Deacon, Pasha Tatashin, James Morse,
Matthias Brugger
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: assign PXN permission in trans_pgd
2021-11-11 16:13 ` Ard Biesheuvel
@ 2021-11-11 18:18 ` Catalin Marinas
2021-11-11 18:24 ` Catalin Marinas
2021-11-12 5:18 ` Pingfan Liu
0 siblings, 2 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-11-11 18:18 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Pingfan Liu, Linux ARM, Will Deacon, Pasha Tatashin, James Morse,
Matthias Brugger
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.
> >
> > 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.
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.
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: assign PXN permission in trans_pgd
2021-11-11 18:18 ` Catalin Marinas
@ 2021-11-11 18:24 ` Catalin Marinas
2021-11-12 5:18 ` Pingfan Liu
1 sibling, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-11-11 18:24 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Pingfan Liu, Linux ARM, Will Deacon, Pasha Tatashin, James Morse,
Matthias Brugger
On Thu, Nov 11, 2021 at 06:18:17PM +0000, Catalin Marinas wrote:
> BTW, in line with PAGE_KERNEL, pud_populate() could set both UXN and PXN
> for init_mm mappings.
Ignore this, I got confused. The UXN/PXN choice is correct as it is.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: assign PXN permission in trans_pgd
2021-11-11 18:18 ` Catalin Marinas
2021-11-11 18:24 ` Catalin Marinas
@ 2021-11-12 5:18 ` Pingfan Liu
1 sibling, 0 replies; 10+ messages in thread
From: Pingfan Liu @ 2021-11-12 5:18 UTC (permalink / raw)
To: Catalin Marinas
Cc: Ard Biesheuvel, Linux ARM, Will Deacon, Pasha Tatashin,
James Morse, Matthias Brugger
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd
2021-11-11 6:03 [PATCH] arm64: mm: assign PXN permission in trans_pgd Pingfan Liu
2021-11-11 16:09 ` Catalin Marinas
@ 2021-11-12 5:22 ` Pingfan Liu
2021-11-12 14:26 ` Catalin Marinas
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Pingfan Liu @ 2021-11-12 5:22 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Pingfan Liu, Catalin Marinas, Ard Biesheuvel, Will Deacon,
Pasha Tatashin, James Morse, Matthias Brugger
trans_pgd_create_copy() can hit "VM_BUG_ON(mm != &init_mm)" in the
function pmd_populate_kernel().
This is the combined consequence of commit 5de59884ac0e ("arm64:
trans_pgd: pass NULL instead of init_mm to *_populate functions"), which
replaced &init_mm with NULL and commit 59511cfd08f3 ("arm64: mm: use XN
table mapping attributes for user/kernel mappings"), which introduced
the VM_BUG_ON.
Since the former sounds reasonable, it is better to work on the later.
From the perspective of trans_pgd, two groups of functions are
considered in the later one:
pmd_populate_kernel()
mm == NULL should be fixed, else it hits VM_BUG_ON()
p?d_populate()
mm == NULL means PXN, that is OK, since trans_pgd only copies a
linear map, no execution will happen on the map.
So it is good enough to just relax VM_BUG_ON() to disregard mm == NULL
Fixes: 59511cfd08f3 ("arm64: mm: use XN table mapping attributes for user/kernel mappings")
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
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
---
arch/arm64/include/asm/pgalloc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
index 8433a2058eb1..237224484d0f 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -76,7 +76,7 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t ptep,
static inline void
pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmdp, pte_t *ptep)
{
- VM_BUG_ON(mm != &init_mm);
+ VM_BUG_ON(mm && mm != &init_mm);
__pmd_populate(pmdp, __pa(ptep), PMD_TYPE_TABLE | PMD_TABLE_UXN);
}
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd
2021-11-12 5:22 ` [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd Pingfan Liu
@ 2021-11-12 14:26 ` Catalin Marinas
2021-11-12 14:37 ` Pasha Tatashin
2021-11-16 10:39 ` Will Deacon
2 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-11-12 14:26 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-arm-kernel, Ard Biesheuvel, Will Deacon, Pasha Tatashin,
James Morse, Matthias Brugger
On Fri, Nov 12, 2021 at 01:22:14PM +0800, Pingfan Liu wrote:
> trans_pgd_create_copy() can hit "VM_BUG_ON(mm != &init_mm)" in the
> function pmd_populate_kernel().
>
> This is the combined consequence of commit 5de59884ac0e ("arm64:
> trans_pgd: pass NULL instead of init_mm to *_populate functions"), which
> replaced &init_mm with NULL and commit 59511cfd08f3 ("arm64: mm: use XN
> table mapping attributes for user/kernel mappings"), which introduced
> the VM_BUG_ON.
>
> Since the former sounds reasonable, it is better to work on the later.
> From the perspective of trans_pgd, two groups of functions are
> considered in the later one:
>
> pmd_populate_kernel()
> mm == NULL should be fixed, else it hits VM_BUG_ON()
> p?d_populate()
> mm == NULL means PXN, that is OK, since trans_pgd only copies a
> linear map, no execution will happen on the map.
>
> So it is good enough to just relax VM_BUG_ON() to disregard mm == NULL
>
> Fixes: 59511cfd08f3 ("arm64: mm: use XN table mapping attributes for user/kernel mappings")
Cc: <stable@vger.kernel.org> # 5.13.x
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> 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
It looks fine to me.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
I'll leave it to Will to queue as a fix.
Thanks.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd
2021-11-12 5:22 ` [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd Pingfan Liu
2021-11-12 14:26 ` Catalin Marinas
@ 2021-11-12 14:37 ` Pasha Tatashin
2021-11-16 10:39 ` Will Deacon
2 siblings, 0 replies; 10+ messages in thread
From: Pasha Tatashin @ 2021-11-12 14:37 UTC (permalink / raw)
To: Pingfan Liu
Cc: Linux ARM, Catalin Marinas, Ard Biesheuvel, Will Deacon,
James Morse, Matthias Brugger
On Fri, Nov 12, 2021 at 12:22 AM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> trans_pgd_create_copy() can hit "VM_BUG_ON(mm != &init_mm)" in the
> function pmd_populate_kernel().
>
> This is the combined consequence of commit 5de59884ac0e ("arm64:
> trans_pgd: pass NULL instead of init_mm to *_populate functions"), which
> replaced &init_mm with NULL and commit 59511cfd08f3 ("arm64: mm: use XN
> table mapping attributes for user/kernel mappings"), which introduced
> the VM_BUG_ON.
>
> Since the former sounds reasonable, it is better to work on the later.
> From the perspective of trans_pgd, two groups of functions are
> considered in the later one:
>
> pmd_populate_kernel()
> mm == NULL should be fixed, else it hits VM_BUG_ON()
> p?d_populate()
> mm == NULL means PXN, that is OK, since trans_pgd only copies a
> linear map, no execution will happen on the map.
>
> So it is good enough to just relax VM_BUG_ON() to disregard mm == NULL
>
> Fixes: 59511cfd08f3 ("arm64: mm: use XN table mapping attributes for user/kernel mappings")
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> 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
> ---
> arch/arm64/include/asm/pgalloc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
> index 8433a2058eb1..237224484d0f 100644
> --- a/arch/arm64/include/asm/pgalloc.h
> +++ b/arch/arm64/include/asm/pgalloc.h
> @@ -76,7 +76,7 @@ static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t ptep,
> static inline void
> pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmdp, pte_t *ptep)
> {
> - VM_BUG_ON(mm != &init_mm);
> + VM_BUG_ON(mm && mm != &init_mm);
LGTM
Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Pasha
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd
2021-11-12 5:22 ` [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd Pingfan Liu
2021-11-12 14:26 ` Catalin Marinas
2021-11-12 14:37 ` Pasha Tatashin
@ 2021-11-16 10:39 ` Will Deacon
2 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2021-11-16 10:39 UTC (permalink / raw)
To: linux-arm-kernel, Pingfan Liu
Cc: catalin.marinas, kernel-team, Will Deacon, Ard Biesheuvel,
James Morse, Matthias Brugger, Pasha Tatashin
On Fri, 12 Nov 2021 13:22:14 +0800, Pingfan Liu wrote:
> trans_pgd_create_copy() can hit "VM_BUG_ON(mm != &init_mm)" in the
> function pmd_populate_kernel().
>
> This is the combined consequence of commit 5de59884ac0e ("arm64:
> trans_pgd: pass NULL instead of init_mm to *_populate functions"), which
> replaced &init_mm with NULL and commit 59511cfd08f3 ("arm64: mm: use XN
> table mapping attributes for user/kernel mappings"), which introduced
> the VM_BUG_ON.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd
https://git.kernel.org/arm64/c/d3eb70ead647
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-16 10:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11 6:03 [PATCH] arm64: mm: assign PXN permission in trans_pgd Pingfan Liu
2021-11-11 16:09 ` Catalin Marinas
2021-11-11 16:13 ` Ard Biesheuvel
2021-11-11 18:18 ` Catalin Marinas
2021-11-11 18:24 ` Catalin Marinas
2021-11-12 5:18 ` Pingfan Liu
2021-11-12 5:22 ` [PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd Pingfan Liu
2021-11-12 14:26 ` Catalin Marinas
2021-11-12 14:37 ` Pasha Tatashin
2021-11-16 10:39 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.