All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/mm: don't free p4d table when it is folded at runtime.
@ 2018-06-25 10:24 Andrey Ryabinin
  2018-06-25 11:50 ` Kirill A. Shutemov
  2018-06-26 10:33 ` [tip:x86/urgent] x86/mm: Don't free P4D " tip-bot for Andrey Ryabinin
  0 siblings, 2 replies; 6+ messages in thread
From: Andrey Ryabinin @ 2018-06-25 10:24 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86
  Cc: Kirill A. Shutemov, linux-kernel, Andrey Ryabinin

When the p4d page table layer is folded at runtime, the p4d_free()
should do nothing, the same as in <asm-generic/pgtable-nop4d.h>.

It seems this bug should cause double-free in efi_call_phys_epilog(),
but I don't know how to trigger that code path, so I can't confirm that
by testing.

Fixes: 98219dda2ab5 ("x86/mm: Fold p4d page table layer at runtime")
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 arch/x86/include/asm/pgalloc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
index ada6410fd2ec..fbd578daa66e 100644
--- a/arch/x86/include/asm/pgalloc.h
+++ b/arch/x86/include/asm/pgalloc.h
@@ -184,6 +184,9 @@ static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
 
 static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
 {
+	if (!pgtable_l5_enabled())
+		return;
+
 	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
 	free_page((unsigned long)p4d);
 }
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/mm: don't free p4d table when it is folded at runtime.
  2018-06-25 10:24 [PATCH] x86/mm: don't free p4d table when it is folded at runtime Andrey Ryabinin
@ 2018-06-25 11:50 ` Kirill A. Shutemov
  2018-06-26  7:40   ` Baoquan He
  2018-06-26 10:33 ` [tip:x86/urgent] x86/mm: Don't free P4D " tip-bot for Andrey Ryabinin
  1 sibling, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2018-06-25 11:50 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	Kirill A. Shutemov, linux-kernel, Baoquan He, Matt Fleming

On Mon, Jun 25, 2018 at 01:24:27PM +0300, Andrey Ryabinin wrote:
> When the p4d page table layer is folded at runtime, the p4d_free()
> should do nothing, the same as in <asm-generic/pgtable-nop4d.h>.
> 
> It seems this bug should cause double-free in efi_call_phys_epilog(),
> but I don't know how to trigger that code path, so I can't confirm that
> by testing.

+ Baoquan, Matt.

There's other bug in the efi_call_phys_epilog() that prevents the bug from
being triggered.

With the patch below. You can trigger the bug with efi=old_map in kernel
command line + KALSR and CONFIG_X86_5LEVEL=y:

	page:fffff6bec0000000 count:0 mapcount:1 mapping:0000000000000000 index:0x0
	flags: 0x800(reserved)
	raw: 0000000000000800 fffff6bec0000008 fffff6bec0000008 0000000000000000
	raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
	page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
	------------[ cut here ]------------
	kernel BUG at /home/kas/linux/la57/include/linux/mm.h:499!
	invalid opcode: 0000 [#1] PREEMPT SMP
	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.18.0-rc2-00037-g6f0d349d922b-dirty #58
	Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
	RIP: 0010:__free_pages+0x28/0x30
	Code: 00 00 8b 47 34 85 c0 74 15 f0 ff 4f 34 75 09 85 f6 74 06 e9 ca d8 ff ff c3 e9 64 ff ff ff 48
	RSP: 0000:ffffffff9a403e90 EFLAGS: 00000246
	RAX: 000000000000003e RBX: ffffffff9a41d000 RCX: 0000000000000002
	RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000ffffffff
	RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
	R10: 0000000000000000 R11: 0000000000000000 R12: ffff9092af089000
	R13: ffffffff9a598a80 R14: 0000000000000001 R15: 0000000000000001
	FS:  0000000000000000(0000) GS:ffff9092bfc00000(0000) knlGS:0000000000000000
	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
	CR2: ffff9092bffff000 CR3: 0000000198e1d000 CR4: 00000000000006b0
	Call Trace:
	 efi_call_phys_epilog+0x17d/0x1bb
	 efi_enter_virtual_mode+0x457/0x4ca
	 start_kernel+0x443/0x4dc
	 secondary_startup_64+0xb7/0xc0
	Modules linked in:
	---[ end trace 61e271260b11acdd ]---

I'll send patch for efi_call_phys_epilog().

> 
> Fixes: 98219dda2ab5 ("x86/mm: Fold p4d page table layer at runtime")
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: stable@vger.kernel.org # 4.17

> ---
>  arch/x86/include/asm/pgalloc.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
> index ada6410fd2ec..fbd578daa66e 100644
> --- a/arch/x86/include/asm/pgalloc.h
> +++ b/arch/x86/include/asm/pgalloc.h
> @@ -184,6 +184,9 @@ static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
>  
>  static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
>  {
> +	if (!pgtable_l5_enabled())
> +		return;
> +
>  	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
>  	free_page((unsigned long)p4d);
>  }

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index e01f7ceb9e7a..77873ce700ae 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -166,14 +166,14 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
                pgd = pgd_offset_k(pgd_idx * PGDIR_SIZE);
                set_pgd(pgd_offset_k(pgd_idx * PGDIR_SIZE), save_pgd[pgd_idx]);

-               if (!(pgd_val(*pgd) & _PAGE_PRESENT))
+               if (!pgd_present(*pgd))
                        continue;

                for (i = 0; i < PTRS_PER_P4D; i++) {
                        p4d = p4d_offset(pgd,
                                         pgd_idx * PGDIR_SIZE + i * P4D_SIZE);

-                       if (!(p4d_val(*p4d) & _PAGE_PRESENT))
+                       if (!p4d_present(*p4d))
                                continue;

                        pud = (pud_t *)p4d_page_vaddr(*p4d);
-- 
 Kirill A. Shutemov

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/mm: don't free p4d table when it is folded at runtime.
  2018-06-25 11:50 ` Kirill A. Shutemov
@ 2018-06-26  7:40   ` Baoquan He
  2018-06-26 10:00     ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Baoquan He @ 2018-06-26  7:40 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Andrey Ryabinin, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Kirill A. Shutemov, linux-kernel, Matt Fleming

Hi Kirill,

On 06/25/18 at 02:50pm, Kirill A. Shutemov wrote:
> On Mon, Jun 25, 2018 at 01:24:27PM +0300, Andrey Ryabinin wrote:
> > When the p4d page table layer is folded at runtime, the p4d_free()
> > should do nothing, the same as in <asm-generic/pgtable-nop4d.h>.
> > 
> > It seems this bug should cause double-free in efi_call_phys_epilog(),
> > but I don't know how to trigger that code path, so I can't confirm that
> > by testing.
> 
> + Baoquan, Matt.
> 
> There's other bug in the efi_call_phys_epilog() that prevents the bug from
> being triggered.
> 
> With the patch below. You can trigger the bug with efi=old_map in kernel
> command line + KALSR and CONFIG_X86_5LEVEL=y:
> 
> 	page:fffff6bec0000000 count:0 mapcount:1 mapping:0000000000000000 index:0x0
> 	flags: 0x800(reserved)
> 	raw: 0000000000000800 fffff6bec0000008 fffff6bec0000008 0000000000000000
> 	raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 	page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
> 	------------[ cut here ]------------
> 	kernel BUG at /home/kas/linux/la57/include/linux/mm.h:499!
> 	invalid opcode: 0000 [#1] PREEMPT SMP
> 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.18.0-rc2-00037-g6f0d349d922b-dirty #58
> 	Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
> 	RIP: 0010:__free_pages+0x28/0x30
> 	Code: 00 00 8b 47 34 85 c0 74 15 f0 ff 4f 34 75 09 85 f6 74 06 e9 ca d8 ff ff c3 e9 64 ff ff ff 48
> 	RSP: 0000:ffffffff9a403e90 EFLAGS: 00000246
> 	RAX: 000000000000003e RBX: ffffffff9a41d000 RCX: 0000000000000002
> 	RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000ffffffff
> 	RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> 	R10: 0000000000000000 R11: 0000000000000000 R12: ffff9092af089000
> 	R13: ffffffff9a598a80 R14: 0000000000000001 R15: 0000000000000001
> 	FS:  0000000000000000(0000) GS:ffff9092bfc00000(0000) knlGS:0000000000000000
> 	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> 	CR2: ffff9092bffff000 CR3: 0000000198e1d000 CR4: 00000000000006b0
> 	Call Trace:
> 	 efi_call_phys_epilog+0x17d/0x1bb
> 	 efi_enter_virtual_mode+0x457/0x4ca
> 	 start_kernel+0x443/0x4dc
> 	 secondary_startup_64+0xb7/0xc0
> 	Modules linked in:
> 	---[ end trace 61e271260b11acdd ]---
> 
> I'll send patch for efi_call_phys_epilog().
> 
> > 
> > Fixes: 98219dda2ab5 ("x86/mm: Fold p4d page table layer at runtime")
> > Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> 
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: stable@vger.kernel.org # 4.17
> 
> > ---
> >  arch/x86/include/asm/pgalloc.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
> > index ada6410fd2ec..fbd578daa66e 100644
> > --- a/arch/x86/include/asm/pgalloc.h
> > +++ b/arch/x86/include/asm/pgalloc.h
> > @@ -184,6 +184,9 @@ static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
> >  
> >  static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
> >  {
> > +	if (!pgtable_l5_enabled())
> > +		return;
> > +
> >  	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> >  	free_page((unsigned long)p4d);
> >  }
> 
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index e01f7ceb9e7a..77873ce700ae 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -166,14 +166,14 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
>                 pgd = pgd_offset_k(pgd_idx * PGDIR_SIZE);
>                 set_pgd(pgd_offset_k(pgd_idx * PGDIR_SIZE), save_pgd[pgd_idx]);
> 
> -               if (!(pgd_val(*pgd) & _PAGE_PRESENT))
> +               if (!pgd_present(*pgd))
>                         continue;

I may not understand boot-time p4d folding. Here p4d is folded, why
pgd_present() need always return 1 if p4d folded?

Thanks
Baoquan

> 
>                 for (i = 0; i < PTRS_PER_P4D; i++) {
>                         p4d = p4d_offset(pgd,
>                                          pgd_idx * PGDIR_SIZE + i * P4D_SIZE);
> 
> -                       if (!(p4d_val(*p4d) & _PAGE_PRESENT))
> +                       if (!p4d_present(*p4d))
>                                 continue;
> 
>                         pud = (pud_t *)p4d_page_vaddr(*p4d);
> -- 
>  Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/mm: don't free p4d table when it is folded at runtime.
  2018-06-26  7:40   ` Baoquan He
@ 2018-06-26 10:00     ` Kirill A. Shutemov
  2018-06-26 23:08       ` Baoquan He
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2018-06-26 10:00 UTC (permalink / raw)
  To: Baoquan He
  Cc: Kirill A. Shutemov, Andrey Ryabinin, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Matt Fleming

On Tue, Jun 26, 2018 at 07:40:49AM +0000, Baoquan He wrote:
> Hi Kirill,
> 
> On 06/25/18 at 02:50pm, Kirill A. Shutemov wrote:
> > On Mon, Jun 25, 2018 at 01:24:27PM +0300, Andrey Ryabinin wrote:
> > > When the p4d page table layer is folded at runtime, the p4d_free()
> > > should do nothing, the same as in <asm-generic/pgtable-nop4d.h>.
> > > 
> > > It seems this bug should cause double-free in efi_call_phys_epilog(),
> > > but I don't know how to trigger that code path, so I can't confirm that
> > > by testing.
> > 
> > + Baoquan, Matt.
> > 
> > There's other bug in the efi_call_phys_epilog() that prevents the bug from
> > being triggered.
> > 
> > With the patch below. You can trigger the bug with efi=old_map in kernel
> > command line + KALSR and CONFIG_X86_5LEVEL=y:
> > 
> > 	page:fffff6bec0000000 count:0 mapcount:1 mapping:0000000000000000 index:0x0
> > 	flags: 0x800(reserved)
> > 	raw: 0000000000000800 fffff6bec0000008 fffff6bec0000008 0000000000000000
> > 	raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > 	page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
> > 	------------[ cut here ]------------
> > 	kernel BUG at /home/kas/linux/la57/include/linux/mm.h:499!
> > 	invalid opcode: 0000 [#1] PREEMPT SMP
> > 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.18.0-rc2-00037-g6f0d349d922b-dirty #58
> > 	Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
> > 	RIP: 0010:__free_pages+0x28/0x30
> > 	Code: 00 00 8b 47 34 85 c0 74 15 f0 ff 4f 34 75 09 85 f6 74 06 e9 ca d8 ff ff c3 e9 64 ff ff ff 48
> > 	RSP: 0000:ffffffff9a403e90 EFLAGS: 00000246
> > 	RAX: 000000000000003e RBX: ffffffff9a41d000 RCX: 0000000000000002
> > 	RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000ffffffff
> > 	RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> > 	R10: 0000000000000000 R11: 0000000000000000 R12: ffff9092af089000
> > 	R13: ffffffff9a598a80 R14: 0000000000000001 R15: 0000000000000001
> > 	FS:  0000000000000000(0000) GS:ffff9092bfc00000(0000) knlGS:0000000000000000
> > 	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > 	CR2: ffff9092bffff000 CR3: 0000000198e1d000 CR4: 00000000000006b0
> > 	Call Trace:
> > 	 efi_call_phys_epilog+0x17d/0x1bb
> > 	 efi_enter_virtual_mode+0x457/0x4ca
> > 	 start_kernel+0x443/0x4dc
> > 	 secondary_startup_64+0xb7/0xc0
> > 	Modules linked in:
> > 	---[ end trace 61e271260b11acdd ]---
> > 
> > I'll send patch for efi_call_phys_epilog().
> > 
> > > 
> > > Fixes: 98219dda2ab5 ("x86/mm: Fold p4d page table layer at runtime")
> > > Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> > 
> > Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: stable@vger.kernel.org # 4.17
> > 
> > > ---
> > >  arch/x86/include/asm/pgalloc.h | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
> > > index ada6410fd2ec..fbd578daa66e 100644
> > > --- a/arch/x86/include/asm/pgalloc.h
> > > +++ b/arch/x86/include/asm/pgalloc.h
> > > @@ -184,6 +184,9 @@ static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
> > >  
> > >  static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
> > >  {
> > > +	if (!pgtable_l5_enabled())
> > > +		return;
> > > +
> > >  	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> > >  	free_page((unsigned long)p4d);
> > >  }
> > 
> > diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> > index e01f7ceb9e7a..77873ce700ae 100644
> > --- a/arch/x86/platform/efi/efi_64.c
> > +++ b/arch/x86/platform/efi/efi_64.c
> > @@ -166,14 +166,14 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
> >                 pgd = pgd_offset_k(pgd_idx * PGDIR_SIZE);
> >                 set_pgd(pgd_offset_k(pgd_idx * PGDIR_SIZE), save_pgd[pgd_idx]);
> > 
> > -               if (!(pgd_val(*pgd) & _PAGE_PRESENT))
> > +               if (!pgd_present(*pgd))
> >                         continue;
> 
> I may not understand boot-time p4d folding. Here p4d is folded, why
> pgd_present() need always return 1 if p4d folded?

Yeah. This is confusing. Basically, we pretend that p4d level consist of 1
entry, with the same value as pgd above. We say that pgd is always present
and all checks happens on p4d level.

It's not specific to boot-time folding. Compilet-time folding doing the
same. See include/asm-generic/pgtable-nop4d.h (and the rest -nop?d.h)

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] x86/mm: Don't free P4D table when it is folded at runtime
  2018-06-25 10:24 [PATCH] x86/mm: don't free p4d table when it is folded at runtime Andrey Ryabinin
  2018-06-25 11:50 ` Kirill A. Shutemov
@ 2018-06-26 10:33 ` tip-bot for Andrey Ryabinin
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Andrey Ryabinin @ 2018-06-26 10:33 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, kirill.shutemov, torvalds, tglx, hpa, mingo,
	aryabinin, peterz

Commit-ID:  0e311d237d7f3022b7dafb639b42541bfb42fe94
Gitweb:     https://git.kernel.org/tip/0e311d237d7f3022b7dafb639b42541bfb42fe94
Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
AuthorDate: Mon, 25 Jun 2018 13:24:27 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Jun 2018 09:21:48 +0200

x86/mm: Don't free P4D table when it is folded at runtime

When the P4D page table layer is folded at runtime, the p4d_free()
should do nothing, the same as in <asm-generic/pgtable-nop4d.h>.

It seems this bug should cause double-free in efi_call_phys_epilog(),
but I don't know how to trigger that code path, so I can't confirm that
by testing.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org # 4.17
Fixes: 98219dda2ab5 ("x86/mm: Fold p4d page table layer at runtime")
Link: http://lkml.kernel.org/r/20180625102427.15015-1-aryabinin@virtuozzo.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/pgalloc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
index ada6410fd2ec..fbd578daa66e 100644
--- a/arch/x86/include/asm/pgalloc.h
+++ b/arch/x86/include/asm/pgalloc.h
@@ -184,6 +184,9 @@ static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
 
 static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
 {
+	if (!pgtable_l5_enabled())
+		return;
+
 	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
 	free_page((unsigned long)p4d);
 }

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/mm: don't free p4d table when it is folded at runtime.
  2018-06-26 10:00     ` Kirill A. Shutemov
@ 2018-06-26 23:08       ` Baoquan He
  0 siblings, 0 replies; 6+ messages in thread
From: Baoquan He @ 2018-06-26 23:08 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Kirill A. Shutemov, Andrey Ryabinin, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Matt Fleming

On 06/26/18 at 01:00pm, Kirill A. Shutemov wrote:
> On Tue, Jun 26, 2018 at 07:40:49AM +0000, Baoquan He wrote:
> > Hi Kirill,
> > 
> > On 06/25/18 at 02:50pm, Kirill A. Shutemov wrote:
> > > On Mon, Jun 25, 2018 at 01:24:27PM +0300, Andrey Ryabinin wrote:
> > > > When the p4d page table layer is folded at runtime, the p4d_free()
> > > > should do nothing, the same as in <asm-generic/pgtable-nop4d.h>.
> > > > 
> > > > It seems this bug should cause double-free in efi_call_phys_epilog(),
> > > > but I don't know how to trigger that code path, so I can't confirm that
> > > > by testing.
> > > 
> > > + Baoquan, Matt.
> > > 
> > > There's other bug in the efi_call_phys_epilog() that prevents the bug from
> > > being triggered.
> > > 
> > > With the patch below. You can trigger the bug with efi=old_map in kernel
> > > command line + KALSR and CONFIG_X86_5LEVEL=y:
> > > 
> > > 	page:fffff6bec0000000 count:0 mapcount:1 mapping:0000000000000000 index:0x0
> > > 	flags: 0x800(reserved)
> > > 	raw: 0000000000000800 fffff6bec0000008 fffff6bec0000008 0000000000000000
> > > 	raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > > 	page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
> > > 	------------[ cut here ]------------
> > > 	kernel BUG at /home/kas/linux/la57/include/linux/mm.h:499!
> > > 	invalid opcode: 0000 [#1] PREEMPT SMP
> > > 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.18.0-rc2-00037-g6f0d349d922b-dirty #58
> > > 	Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
> > > 	RIP: 0010:__free_pages+0x28/0x30
> > > 	Code: 00 00 8b 47 34 85 c0 74 15 f0 ff 4f 34 75 09 85 f6 74 06 e9 ca d8 ff ff c3 e9 64 ff ff ff 48
> > > 	RSP: 0000:ffffffff9a403e90 EFLAGS: 00000246
> > > 	RAX: 000000000000003e RBX: ffffffff9a41d000 RCX: 0000000000000002
> > > 	RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000ffffffff
> > > 	RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> > > 	R10: 0000000000000000 R11: 0000000000000000 R12: ffff9092af089000
> > > 	R13: ffffffff9a598a80 R14: 0000000000000001 R15: 0000000000000001
> > > 	FS:  0000000000000000(0000) GS:ffff9092bfc00000(0000) knlGS:0000000000000000
> > > 	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > 	CR2: ffff9092bffff000 CR3: 0000000198e1d000 CR4: 00000000000006b0
> > > 	Call Trace:
> > > 	 efi_call_phys_epilog+0x17d/0x1bb
> > > 	 efi_enter_virtual_mode+0x457/0x4ca
> > > 	 start_kernel+0x443/0x4dc
> > > 	 secondary_startup_64+0xb7/0xc0
> > > 	Modules linked in:
> > > 	---[ end trace 61e271260b11acdd ]---
> > > 
> > > I'll send patch for efi_call_phys_epilog().
> > > 
> > > > 
> > > > Fixes: 98219dda2ab5 ("x86/mm: Fold p4d page table layer at runtime")
> > > > Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> > > 
> > > Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Cc: stable@vger.kernel.org # 4.17
> > > 
> > > > ---
> > > >  arch/x86/include/asm/pgalloc.h | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
> > > > index ada6410fd2ec..fbd578daa66e 100644
> > > > --- a/arch/x86/include/asm/pgalloc.h
> > > > +++ b/arch/x86/include/asm/pgalloc.h
> > > > @@ -184,6 +184,9 @@ static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
> > > >  
> > > >  static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
> > > >  {
> > > > +	if (!pgtable_l5_enabled())
> > > > +		return;
> > > > +
> > > >  	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> > > >  	free_page((unsigned long)p4d);
> > > >  }
> > > 
> > > diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> > > index e01f7ceb9e7a..77873ce700ae 100644
> > > --- a/arch/x86/platform/efi/efi_64.c
> > > +++ b/arch/x86/platform/efi/efi_64.c
> > > @@ -166,14 +166,14 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
> > >                 pgd = pgd_offset_k(pgd_idx * PGDIR_SIZE);
> > >                 set_pgd(pgd_offset_k(pgd_idx * PGDIR_SIZE), save_pgd[pgd_idx]);
> > > 
> > > -               if (!(pgd_val(*pgd) & _PAGE_PRESENT))
> > > +               if (!pgd_present(*pgd))
> > >                         continue;
> > 
> > I may not understand boot-time p4d folding. Here p4d is folded, why
> > pgd_present() need always return 1 if p4d folded?
> 
> Yeah. This is confusing. Basically, we pretend that p4d level consist of 1
> entry, with the same value as pgd above. We say that pgd is always present
> and all checks happens on p4d level.
> 
> It's not specific to boot-time folding. Compilet-time folding doing the
> same. See include/asm-generic/pgtable-nop4d.h (and the rest -nop?d.h)

Thanks for telling, Kirill. Then the next p4d_present will do the real
_PAGE_PRESENT checking. If so, I think this is a good fix.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-06-26 23:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-25 10:24 [PATCH] x86/mm: don't free p4d table when it is folded at runtime Andrey Ryabinin
2018-06-25 11:50 ` Kirill A. Shutemov
2018-06-26  7:40   ` Baoquan He
2018-06-26 10:00     ` Kirill A. Shutemov
2018-06-26 23:08       ` Baoquan He
2018-06-26 10:33 ` [tip:x86/urgent] x86/mm: Don't free P4D " tip-bot for Andrey Ryabinin

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.