Unhandled page fault in vmemmap_populate on x86_64

Unhandled page fault in vmemmap_populate on x86_64

[Adrian-Ken Rueegsegger]

Hello,

While running Linux 5.15.32/x86_64 (with some out-of-tree patches) on top of
Muen [1], I came across a BUG/page fault triggered in vmemmap_populate:

[    0.000000] BUG: unable to handle page fault for address: ffffea0001e00000
[    0.000000] #PF: supervisor write access in kernel mode
[    0.000000] #PF: error_code(0x0002) - not-present page
[    0.000000] PGD 1003a067 P4D 1003a067 PUD 10039067 PMD 0 
[    0.000000] Oops: 0002 [#1] SMP PTI
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.32-muen #1
[    0.000000] RIP: 0010:vmemmap_populate+0x181/0x218
[    0.000000] Code: 00 a9 ff ff 1f 00 0f 84 a1 00 00 00 e8 91 f7 ff ff b9 0e 00 00 00 31 c0 48 89 ef f3 ab 48 85 f6 74 0a b0 fd 48 89 ef 48 89 f1 <f3> aa 4d 85 c0 74 7c 48 89 1d 2e e2 05 00 eb 73 48 83 3c 24 00 0f
[    0.000000] RSP: 0000:ffffffff82003e00 EFLAGS: 00010006
[    0.000000] RAX: 00000000000000fd RBX: ffffea0001e00000 RCX: 0000000000180000
[    0.000000] RDX: ffffea0000540000 RSI: 00000000001c0000 RDI: ffffea0001e00000
[    0.000000] RBP: ffffea0001dc0000 R08: 0000000000000000 R09: 0000000088000000
[    0.000000] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[    0.000000] R13: ffffea0001f80000 R14: ffffea0001dc0000 R15: ffff888010039070
[    0.000000] FS:  0000000000000000(0000) GS:ffffffff823ea000(0000) knlGS:0000000000000000
[    0.000000] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.000000] CR2: ffffea0001e00000 CR3: 000000000200a000 CR4: 00000000000406b0
[    0.000000] DR0: 000000000000003a DR1: 0000000000000003 DR2: 0000000000000000
[    0.000000] DR3: ffffea0001e00000 DR6: 000000000200a000 DR7: ffffffff82003d58
[    0.000000] Call Trace:
[    0.000000]  <TASK>
[    0.000000]  ? __populate_section_memmap+0x3a/0x47
[    0.000000]  ? sparse_init_nid+0xc9/0x174
[    0.000000]  ? sparse_init+0x1c1/0x1d2
[    0.000000]  ? paging_init+0x5/0xa
[    0.000000]  ? setup_arch+0x740/0x810
[    0.000000]  ? start_kernel+0x43/0x5bb
[    0.000000]  ? secondary_startup_64_no_verify+0xb0/0xbb
[    0.000000]  </TASK>
[    0.000000] Modules linked in:
[    0.000000] CR2: ffffea0001e00000
[    0.000000] random: get_random_bytes called from init_oops_id+0x1d/0x2c with crng_init=0
[    0.000000] ---[ end trace 44fe402cfef775de ]---

Announcing an available RAM region at 0x88000000 to Linux (via e820) triggered
the issue while placing it at 0x70000000 did not hit the bug. Since the problem
had not been observed with 5.10, I did a bisect which pointed to commit
8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") as the culprit.
Further debugging showed that the #PF originates from vmemmap_use_new_sub_pmd
in arch/x86/mm/init_64.c. In the error case the condition
!IS_ALIGNED(start, PMD_SIZE) evaluates to true and the page-fault is caused by
the memset marking the preceding region as unused:

    if (!IS_ALIGNED(start, PMD_SIZE))
        memset((void *)start, PAGE_UNUSED,
               start - ALIGN_DOWN(start, PMD_SIZE));

If I am not mistaken, the start variable is the wrong address to use here,
since it points to the beginning of the range that is to be *used*. Instead the
"start" of the PMD should be used, i.e. ALIGN_DOWN(start, PMD_SIZE). Looking at
arch/s390/mm/vmem.c, vmemmap_use_new_sub_pmd seems to confirm this. Is the
above analysis correct or did I misread the code?

The attached patch fixes the observed issue for me.

Regards,
Adrian

PS: When replying please include my address as to/cc since I am not subscribed
to LKML, thanks!

[1] - https://muen.sk

[PATCH] x86/mm: Fix marking of unused sub-pmd ranges

[Adrian-Ken Rueegsegger]

The unused part precedes the new range spanned by the start, end
parameters of vmemmap_use_new_sub_pmd. This means it actually goes from
ALIGN_DOWN(start, PMD_SIZE) up to start. Use the correct address when
applying the mark using memset.

Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges")
Signed-off-by: Adrian-Ken Rueegsegger <ken@codelabs.ch>
---
 arch/x86/mm/init_64.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 96d34ebb20a9..e2942335d143 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -902,6 +902,8 @@ static void __meminit vmemmap_use_sub_pmd(unsigned long start, unsigned long end
 
 static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long end)
 {
+	const unsigned long page = ALIGN_DOWN(start, PMD_SIZE);
+
 	vmemmap_flush_unused_pmd();
 
 	/*
@@ -914,8 +916,7 @@ static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long
 	 * Mark with PAGE_UNUSED the unused parts of the new memmap range
 	 */
 	if (!IS_ALIGNED(start, PMD_SIZE))
-		memset((void *)start, PAGE_UNUSED,
-			start - ALIGN_DOWN(start, PMD_SIZE));
+		memset((void *)page, PAGE_UNUSED, start - page);
 
 	/*
 	 * We want to avoid memset(PAGE_UNUSED) when populating the vmemmap of
-- 
2.30.2

Re: [PATCH] x86/mm: Fix marking of unused sub-pmd ranges

[David Hildenbrand]

On 09.05.22 11:06, Adrian-Ken Rueegsegger wrote:
> The unused part precedes the new range spanned by the start, end
> parameters of vmemmap_use_new_sub_pmd. This means it actually goes from
> ALIGN_DOWN(start, PMD_SIZE) up to start. Use the correct address when
> applying the mark using memset.
> 
> Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges")
> Signed-off-by: Adrian-Ken Rueegsegger <ken@codelabs.ch>
> ---
>  arch/x86/mm/init_64.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 96d34ebb20a9..e2942335d143 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -902,6 +902,8 @@ static void __meminit vmemmap_use_sub_pmd(unsigned long start, unsigned long end
>  
>  static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long end)
>  {
> +	const unsigned long page = ALIGN_DOWN(start, PMD_SIZE);
> +
>  	vmemmap_flush_unused_pmd();
>  
>  	/*
> @@ -914,8 +916,7 @@ static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long
>  	 * Mark with PAGE_UNUSED the unused parts of the new memmap range
>  	 */
>  	if (!IS_ALIGNED(start, PMD_SIZE))
> -		memset((void *)start, PAGE_UNUSED,
> -			start - ALIGN_DOWN(start, PMD_SIZE));
> +		memset((void *)page, PAGE_UNUSED, start - page);
>  
>  	/*
>  	 * We want to avoid memset(PAGE_UNUSED) when populating the vmemmap of

As the x86 code was based on my s390x code, I assume that this was
accidentally introduced in the x86 variant.


We'd be marking the wrong range PAGE_UNUSED.


Your fix looks correct to me:

Reviewed-by: David Hildenbrand <david@redhat.com>


Do we want to cc stable?

-- 
Thanks,

David / dhildenb

Re: [PATCH] x86/mm: Fix marking of unused sub-pmd ranges

[Thomas Gleixner]

On Thu, May 12 2022 at 11:04, David Hildenbrand wrote:
> On 09.05.22 11:06, Adrian-Ken Rueegsegger wrote:
>>  static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long end)
>>  {
>> +	const unsigned long page = ALIGN_DOWN(start, PMD_SIZE);
>> +
>>  	vmemmap_flush_unused_pmd();
>>  
>>  	/*
>> @@ -914,8 +916,7 @@ static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long
>>  	 * Mark with PAGE_UNUSED the unused parts of the new memmap range
>>  	 */
>>  	if (!IS_ALIGNED(start, PMD_SIZE))
>> -		memset((void *)start, PAGE_UNUSED,
>> -			start - ALIGN_DOWN(start, PMD_SIZE));
>> +		memset((void *)page, PAGE_UNUSED, start - page);
>>  
>>  	/*
>>  	 * We want to avoid memset(PAGE_UNUSED) when populating the vmemmap of
>
> As the x86 code was based on my s390x code, I assume that this was
> accidentally introduced in the x86 variant.
>
> We'd be marking the wrong range PAGE_UNUSED.
>
> Your fix looks correct to me:
>
> Reviewed-by: David Hildenbrand <david@redhat.com>
>
> Do we want to cc stable?

Yes, we'll add it when picking it up.

I really have to ask why this duplicated code exists in the first
place. There is zero architecture specific code neither in the s390 nor
in the x86 version.

The x86 version is just copy & pasta & fatfinger, if I'm not missing
something here.

Thanks,

        tglx

Re: [PATCH] x86/mm: Fix marking of unused sub-pmd ranges

[Oscar Salvador]

On 2022-05-09 11:06, Adrian-Ken Rueegsegger wrote:
> The unused part precedes the new range spanned by the start, end
> parameters of vmemmap_use_new_sub_pmd. This means it actually goes from
> ALIGN_DOWN(start, PMD_SIZE) up to start. Use the correct address when
> applying the mark using memset.
> 
> Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges")
> Signed-off-by: Adrian-Ken Rueegsegger <ken@codelabs.ch>

Yes, this was clearly an oversight from my side.
Thanks for fixing it!

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  arch/x86/mm/init_64.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 96d34ebb20a9..e2942335d143 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -902,6 +902,8 @@ static void __meminit vmemmap_use_sub_pmd(unsigned
> long start, unsigned long end
> 
>  static void __meminit vmemmap_use_new_sub_pmd(unsigned long start,
> unsigned long end)
>  {
> +	const unsigned long page = ALIGN_DOWN(start, PMD_SIZE);
> +
>  	vmemmap_flush_unused_pmd();
> 
>  	/*
> @@ -914,8 +916,7 @@ static void __meminit
> vmemmap_use_new_sub_pmd(unsigned long start, unsigned long
>  	 * Mark with PAGE_UNUSED the unused parts of the new memmap range
>  	 */
>  	if (!IS_ALIGNED(start, PMD_SIZE))
> -		memset((void *)start, PAGE_UNUSED,
> -			start - ALIGN_DOWN(start, PMD_SIZE));
> +		memset((void *)page, PAGE_UNUSED, start - page);
> 
>  	/*
>  	 * We want to avoid memset(PAGE_UNUSED) when populating the vmemmap 
> of

-- 
Oscar Salvador
SUSE Labs

[tip: x86/urgent] x86/mm: Fix marking of unused sub-pmd ranges

[tip-bot2 for Adrian-Ken Rueegsegger]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     280abe14b6e0a38de9cc86fe6a019523aadd8f70
Gitweb:        https://git.kernel.org/tip/280abe14b6e0a38de9cc86fe6a019523aadd8f70
Author:        Adrian-Ken Rueegsegger <ken@codelabs.ch>
AuthorDate:    Mon, 09 May 2022 11:06:37 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 13 May 2022 12:41:21 +02:00

x86/mm: Fix marking of unused sub-pmd ranges

The unused part precedes the new range spanned by the start, end parameters
of vmemmap_use_new_sub_pmd(). This means it actually goes from
ALIGN_DOWN(start, PMD_SIZE) up to start.

Use the correct address when applying the mark using memset.

Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges")
Signed-off-by: Adrian-Ken Rueegsegger <ken@codelabs.ch>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20220509090637.24152-2-ken@codelabs.ch
---
 arch/x86/mm/init_64.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 96d34eb..e294233 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -902,6 +902,8 @@ static void __meminit vmemmap_use_sub_pmd(unsigned long start, unsigned long end
 
 static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long end)
 {
+	const unsigned long page = ALIGN_DOWN(start, PMD_SIZE);
+
 	vmemmap_flush_unused_pmd();
 
 	/*
@@ -914,8 +916,7 @@ static void __meminit vmemmap_use_new_sub_pmd(unsigned long start, unsigned long
 	 * Mark with PAGE_UNUSED the unused parts of the new memmap range
 	 */
 	if (!IS_ALIGNED(start, PMD_SIZE))
-		memset((void *)start, PAGE_UNUSED,
-			start - ALIGN_DOWN(start, PMD_SIZE));
+		memset((void *)page, PAGE_UNUSED, start - page);
 
 	/*
 	 * We want to avoid memset(PAGE_UNUSED) when populating the vmemmap of