[PATCHv2] arm64: mm: Fix VM_BUG_ON(mm != &init_mm) for trans_pgd

Commit Message

Pingfan Liu Nov. 12, 2021, 5:22 a.m. UTC

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 <[e-mail]>
Cc: Catalin Marinas <[e-mail]>
Cc: Ard Biesheuvel <[e-mail]>
Cc: Will Deacon <[e-mail]>
Cc: Pasha Tatashin <[e-mail]>
Cc: James Morse <[e-mail]>
Cc: Matthias Brugger <[e-mail]>
To: [e-mail]
---
 arch/arm64/include/asm/pgalloc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Catalin Marinas Nov. 12, 2021, 2:26 p.m. UTC | #1

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: <[e-mail]> # 5.13.x

> Signed-off-by: Pingfan Liu <[e-mail]>
> Cc: Catalin Marinas <[e-mail]>
> Cc: Ard Biesheuvel <[e-mail]>
> Cc: Will Deacon <[e-mail]>
> Cc: Pasha Tatashin <[e-mail]>
> Cc: James Morse <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> To: [e-mail]

It looks fine to me.

Reviewed-by: Catalin Marinas <[e-mail]>

I'll leave it to Will to queue as a fix.

Thanks.

Pasha Tatashin Nov. 12, 2021, 2:37 p.m. UTC | #2

On Fri, Nov 12, 2021 at 12:22 AM Pingfan Liu <[e-mail]> 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 <[e-mail]>
> Cc: Catalin Marinas <[e-mail]>
> Cc: Ard Biesheuvel <[e-mail]>
> Cc: Will Deacon <[e-mail]>
> Cc: Pasha Tatashin <[e-mail]>
> Cc: James Morse <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> To: [e-mail]
> ---
>  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 <[e-mail]>

Pasha

Will Deacon Nov. 16, 2021, 10:39 a.m. UTC | #3

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,

Patch

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);
 }