LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64/hugetlb: Fix page table walk in huge_pte_alloc()
@ 2024-04-15  6:31 Anshuman Khandual
  2024-04-15  6:38 ` Ard Biesheuvel
  0 siblings, 1 reply; 4+ messages in thread
From: Anshuman Khandual @ 2024-04-15  6:31 UTC (permalink / raw
  To: linux-arm-kernel
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Ard Biesheuvel,
	Ryan Roberts, Mark Rutland, linux-kernel, Dev Jain

Currently normal HugeTLB fault ends up crashing the kernel, as p4dp derived
from p4d_offset() is an invalid address when PGTABLE_LEVEL = 5. A p4d level
entry needs to be allocated when not available while walking the page table
during HugeTLB faults. Let's call p4d_alloc() to allocate such entries when
required instead of current p4d_offset().

 Unable to handle kernel paging request at virtual address ffffffff80000000
 Mem abort info:
   ESR = 0x0000000096000005
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
   FSC = 0x05: level 1 translation fault
 Data abort info:
   ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
 swapper pgtable: 4k pages, 52-bit VAs, pgdp=0000000081da9000
 [ffffffff80000000] pgd=1000000082cec003, p4d=0000000082c32003, pud=0000000000000000
 Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
 Modules linked in:
 CPU: 1 PID: 108 Comm: high_addr_hugep Not tainted 6.9.0-rc4 #48
 Hardware name: Foundation-v8A (DT)
 pstate: 01402005 (nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
 pc : huge_pte_alloc+0xd4/0x334
 lr : hugetlb_fault+0x1b8/0xc68
 sp : ffff8000833bbc20
 x29: ffff8000833bbc20 x28: fff000080080cb58 x27: ffff800082a7cc58
 x26: 0000000000000000 x25: fff0000800378e40 x24: fff00008008d6c60
 x23: 00000000de9dbf07 x22: fff0000800378e40 x21: 0004000000000000
 x20: 0004000000000000 x19: ffffffff80000000 x18: 1ffe00010011d7a1
 x17: 0000000000000001 x16: ffffffffffffffff x15: 0000000000000001
 x14: 0000000000000000 x13: ffff8000816120d0 x12: ffffffffffffffff
 x11: 0000000000000000 x10: fff00008008ebd0c x9 : 0004000000000000
 x8 : 0000000000001255 x7 : fff00008003e2000 x6 : 00000000061d54b0
 x5 : 0000000000001000 x4 : ffffffff80000000 x3 : 0000000000200000
 x2 : 0000000000000004 x1 : 0000000080000000 x0 : 0000000000000000
 Call trace:
 huge_pte_alloc+0xd4/0x334
 hugetlb_fault+0x1b8/0xc68
 handle_mm_fault+0x260/0x29c
 do_page_fault+0xfc/0x47c
 do_translation_fault+0x68/0x74
 do_mem_abort+0x44/0x94
 el0_da+0x2c/0x9c
 el0t_64_sync_handler+0x70/0xc4
 el0t_64_sync+0x190/0x194
 Code: aa000084 cb010084 b24c2c84 8b130c93 (f9400260)
 ---[ end trace 0000000000000000 ]---

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Fixes: a6bbf5d4d9d1 ("arm64: mm: Add definitions to support 5 levels of paging")
Reported-by: Dev Jain <dev.jain@arm.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This patch applies on v6.9-rc4

 arch/arm64/mm/hugetlbpage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 0f0e10bb0a95..5c819459555a 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -276,7 +276,7 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma,
 	pte_t *ptep = NULL;
 
 	pgdp = pgd_offset(mm, addr);
-	p4dp = p4d_offset(pgdp, addr);
+	p4dp = p4d_alloc(mm, pgdp, addr);
 	pudp = pud_alloc(mm, p4dp, addr);
 	if (!pudp)
 		return NULL;
-- 
2.25.1


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

* Re: [PATCH] arm64/hugetlb: Fix page table walk in huge_pte_alloc()
  2024-04-15  6:31 [PATCH] arm64/hugetlb: Fix page table walk in huge_pte_alloc() Anshuman Khandual
@ 2024-04-15  6:38 ` Ard Biesheuvel
  2024-04-15  6:52   ` Anshuman Khandual
  0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2024-04-15  6:38 UTC (permalink / raw
  To: Anshuman Khandual
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Ryan Roberts,
	Mark Rutland, linux-kernel, Dev Jain

Hello Anshuman,

On Mon, 15 Apr 2024 at 08:31, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> Currently normal HugeTLB fault ends up crashing the kernel, as p4dp derived
> from p4d_offset() is an invalid address when PGTABLE_LEVEL = 5. A p4d level
> entry needs to be allocated when not available while walking the page table
> during HugeTLB faults. Let's call p4d_alloc() to allocate such entries when
> required instead of current p4d_offset().
>
>  Unable to handle kernel paging request at virtual address ffffffff80000000
>  Mem abort info:
>    ESR = 0x0000000096000005
>    EC = 0x25: DABT (current EL), IL = 32 bits
>    SET = 0, FnV = 0
>    EA = 0, S1PTW = 0
>    FSC = 0x05: level 1 translation fault
>  Data abort info:
>    ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
>    CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>    GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>  swapper pgtable: 4k pages, 52-bit VAs, pgdp=0000000081da9000
>  [ffffffff80000000] pgd=1000000082cec003, p4d=0000000082c32003, pud=0000000000000000
>  Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
>  Modules linked in:
>  CPU: 1 PID: 108 Comm: high_addr_hugep Not tainted 6.9.0-rc4 #48
>  Hardware name: Foundation-v8A (DT)
>  pstate: 01402005 (nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
>  pc : huge_pte_alloc+0xd4/0x334
>  lr : hugetlb_fault+0x1b8/0xc68
>  sp : ffff8000833bbc20
>  x29: ffff8000833bbc20 x28: fff000080080cb58 x27: ffff800082a7cc58
>  x26: 0000000000000000 x25: fff0000800378e40 x24: fff00008008d6c60
>  x23: 00000000de9dbf07 x22: fff0000800378e40 x21: 0004000000000000
>  x20: 0004000000000000 x19: ffffffff80000000 x18: 1ffe00010011d7a1
>  x17: 0000000000000001 x16: ffffffffffffffff x15: 0000000000000001
>  x14: 0000000000000000 x13: ffff8000816120d0 x12: ffffffffffffffff
>  x11: 0000000000000000 x10: fff00008008ebd0c x9 : 0004000000000000
>  x8 : 0000000000001255 x7 : fff00008003e2000 x6 : 00000000061d54b0
>  x5 : 0000000000001000 x4 : ffffffff80000000 x3 : 0000000000200000
>  x2 : 0000000000000004 x1 : 0000000080000000 x0 : 0000000000000000
>  Call trace:
>  huge_pte_alloc+0xd4/0x334
>  hugetlb_fault+0x1b8/0xc68
>  handle_mm_fault+0x260/0x29c
>  do_page_fault+0xfc/0x47c
>  do_translation_fault+0x68/0x74
>  do_mem_abort+0x44/0x94
>  el0_da+0x2c/0x9c
>  el0t_64_sync_handler+0x70/0xc4
>  el0t_64_sync+0x190/0x194
>  Code: aa000084 cb010084 b24c2c84 8b130c93 (f9400260)
>  ---[ end trace 0000000000000000 ]---
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Fixes: a6bbf5d4d9d1 ("arm64: mm: Add definitions to support 5 levels of paging")
> Reported-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> This patch applies on v6.9-rc4
>
>  arch/arm64/mm/hugetlbpage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

Thanks for fixing this. One question below.


> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 0f0e10bb0a95..5c819459555a 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -276,7 +276,7 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma,
>         pte_t *ptep = NULL;
>
>         pgdp = pgd_offset(mm, addr);
> -       p4dp = p4d_offset(pgdp, addr);
> +       p4dp = p4d_alloc(mm, pgdp, addr);

Shouldn't we check p4dp for NULL here?

>         pudp = pud_alloc(mm, p4dp, addr);
>         if (!pudp)
>                 return NULL;
> --
> 2.25.1
>

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

* Re: [PATCH] arm64/hugetlb: Fix page table walk in huge_pte_alloc()
  2024-04-15  6:38 ` Ard Biesheuvel
@ 2024-04-15  6:52   ` Anshuman Khandual
  2024-04-15  8:37     ` Mark Rutland
  0 siblings, 1 reply; 4+ messages in thread
From: Anshuman Khandual @ 2024-04-15  6:52 UTC (permalink / raw
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Ryan Roberts,
	Mark Rutland, linux-kernel, Dev Jain



On 4/15/24 12:08, Ard Biesheuvel wrote:
> Hello Anshuman,
> 
> On Mon, 15 Apr 2024 at 08:31, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> Currently normal HugeTLB fault ends up crashing the kernel, as p4dp derived
>> from p4d_offset() is an invalid address when PGTABLE_LEVEL = 5. A p4d level
>> entry needs to be allocated when not available while walking the page table
>> during HugeTLB faults. Let's call p4d_alloc() to allocate such entries when
>> required instead of current p4d_offset().
>>
>>  Unable to handle kernel paging request at virtual address ffffffff80000000
>>  Mem abort info:
>>    ESR = 0x0000000096000005
>>    EC = 0x25: DABT (current EL), IL = 32 bits
>>    SET = 0, FnV = 0
>>    EA = 0, S1PTW = 0
>>    FSC = 0x05: level 1 translation fault
>>  Data abort info:
>>    ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
>>    CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>>    GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>>  swapper pgtable: 4k pages, 52-bit VAs, pgdp=0000000081da9000
>>  [ffffffff80000000] pgd=1000000082cec003, p4d=0000000082c32003, pud=0000000000000000
>>  Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
>>  Modules linked in:
>>  CPU: 1 PID: 108 Comm: high_addr_hugep Not tainted 6.9.0-rc4 #48
>>  Hardware name: Foundation-v8A (DT)
>>  pstate: 01402005 (nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
>>  pc : huge_pte_alloc+0xd4/0x334
>>  lr : hugetlb_fault+0x1b8/0xc68
>>  sp : ffff8000833bbc20
>>  x29: ffff8000833bbc20 x28: fff000080080cb58 x27: ffff800082a7cc58
>>  x26: 0000000000000000 x25: fff0000800378e40 x24: fff00008008d6c60
>>  x23: 00000000de9dbf07 x22: fff0000800378e40 x21: 0004000000000000
>>  x20: 0004000000000000 x19: ffffffff80000000 x18: 1ffe00010011d7a1
>>  x17: 0000000000000001 x16: ffffffffffffffff x15: 0000000000000001
>>  x14: 0000000000000000 x13: ffff8000816120d0 x12: ffffffffffffffff
>>  x11: 0000000000000000 x10: fff00008008ebd0c x9 : 0004000000000000
>>  x8 : 0000000000001255 x7 : fff00008003e2000 x6 : 00000000061d54b0
>>  x5 : 0000000000001000 x4 : ffffffff80000000 x3 : 0000000000200000
>>  x2 : 0000000000000004 x1 : 0000000080000000 x0 : 0000000000000000
>>  Call trace:
>>  huge_pte_alloc+0xd4/0x334
>>  hugetlb_fault+0x1b8/0xc68
>>  handle_mm_fault+0x260/0x29c
>>  do_page_fault+0xfc/0x47c
>>  do_translation_fault+0x68/0x74
>>  do_mem_abort+0x44/0x94
>>  el0_da+0x2c/0x9c
>>  el0t_64_sync_handler+0x70/0xc4
>>  el0t_64_sync+0x190/0x194
>>  Code: aa000084 cb010084 b24c2c84 8b130c93 (f9400260)
>>  ---[ end trace 0000000000000000 ]---
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Ard Biesheuvel <ardb@kernel.org>
>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Fixes: a6bbf5d4d9d1 ("arm64: mm: Add definitions to support 5 levels of paging")
>> Reported-by: Dev Jain <dev.jain@arm.com>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> This patch applies on v6.9-rc4
>>
>>  arch/arm64/mm/hugetlbpage.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> 
> Thanks for fixing this. One question below.
> 
> 
>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>> index 0f0e10bb0a95..5c819459555a 100644
>> --- a/arch/arm64/mm/hugetlbpage.c
>> +++ b/arch/arm64/mm/hugetlbpage.c
>> @@ -276,7 +276,7 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma,
>>         pte_t *ptep = NULL;
>>
>>         pgdp = pgd_offset(mm, addr);
>> -       p4dp = p4d_offset(pgdp, addr);
>> +       p4dp = p4d_alloc(mm, pgdp, addr);
> 
> Shouldn't we check p4dp for NULL here?

Sure, makes sense. I will add a check for p4dp, just like pudp below.

> 
>>         pudp = pud_alloc(mm, p4dp, addr);
>>         if (!pudp)
>>                 return NULL;
>> --
>> 2.25.1
>>

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

* Re: [PATCH] arm64/hugetlb: Fix page table walk in huge_pte_alloc()
  2024-04-15  6:52   ` Anshuman Khandual
@ 2024-04-15  8:37     ` Mark Rutland
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Rutland @ 2024-04-15  8:37 UTC (permalink / raw
  To: Anshuman Khandual
  Cc: Ard Biesheuvel, linux-arm-kernel, Catalin Marinas, Will Deacon,
	Ryan Roberts, linux-kernel, Dev Jain

On Mon, Apr 15, 2024 at 12:22:40PM +0530, Anshuman Khandual wrote:
> 
> 
> On 4/15/24 12:08, Ard Biesheuvel wrote:
> > Hello Anshuman,
> > 
> > On Mon, 15 Apr 2024 at 08:31, Anshuman Khandual
> > <anshuman.khandual@arm.com> wrote:
> >>
> >> Currently normal HugeTLB fault ends up crashing the kernel, as p4dp derived
> >> from p4d_offset() is an invalid address when PGTABLE_LEVEL = 5. A p4d level
> >> entry needs to be allocated when not available while walking the page table
> >> during HugeTLB faults. Let's call p4d_alloc() to allocate such entries when
> >> required instead of current p4d_offset().
> >>
> >>  Unable to handle kernel paging request at virtual address ffffffff80000000
> >>  Mem abort info:
> >>    ESR = 0x0000000096000005
> >>    EC = 0x25: DABT (current EL), IL = 32 bits
> >>    SET = 0, FnV = 0
> >>    EA = 0, S1PTW = 0
> >>    FSC = 0x05: level 1 translation fault
> >>  Data abort info:
> >>    ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
> >>    CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> >>    GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> >>  swapper pgtable: 4k pages, 52-bit VAs, pgdp=0000000081da9000
> >>  [ffffffff80000000] pgd=1000000082cec003, p4d=0000000082c32003, pud=0000000000000000
> >>  Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
> >>  Modules linked in:
> >>  CPU: 1 PID: 108 Comm: high_addr_hugep Not tainted 6.9.0-rc4 #48
> >>  Hardware name: Foundation-v8A (DT)
> >>  pstate: 01402005 (nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> >>  pc : huge_pte_alloc+0xd4/0x334
> >>  lr : hugetlb_fault+0x1b8/0xc68
> >>  sp : ffff8000833bbc20
> >>  x29: ffff8000833bbc20 x28: fff000080080cb58 x27: ffff800082a7cc58
> >>  x26: 0000000000000000 x25: fff0000800378e40 x24: fff00008008d6c60
> >>  x23: 00000000de9dbf07 x22: fff0000800378e40 x21: 0004000000000000
> >>  x20: 0004000000000000 x19: ffffffff80000000 x18: 1ffe00010011d7a1
> >>  x17: 0000000000000001 x16: ffffffffffffffff x15: 0000000000000001
> >>  x14: 0000000000000000 x13: ffff8000816120d0 x12: ffffffffffffffff
> >>  x11: 0000000000000000 x10: fff00008008ebd0c x9 : 0004000000000000
> >>  x8 : 0000000000001255 x7 : fff00008003e2000 x6 : 00000000061d54b0
> >>  x5 : 0000000000001000 x4 : ffffffff80000000 x3 : 0000000000200000
> >>  x2 : 0000000000000004 x1 : 0000000080000000 x0 : 0000000000000000
> >>  Call trace:
> >>  huge_pte_alloc+0xd4/0x334
> >>  hugetlb_fault+0x1b8/0xc68
> >>  handle_mm_fault+0x260/0x29c
> >>  do_page_fault+0xfc/0x47c
> >>  do_translation_fault+0x68/0x74
> >>  do_mem_abort+0x44/0x94
> >>  el0_da+0x2c/0x9c
> >>  el0t_64_sync_handler+0x70/0xc4
> >>  el0t_64_sync+0x190/0x194
> >>  Code: aa000084 cb010084 b24c2c84 8b130c93 (f9400260)
> >>  ---[ end trace 0000000000000000 ]---
> >>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Will Deacon <will@kernel.org>
> >> Cc: Ard Biesheuvel <ardb@kernel.org>
> >> Cc: Ryan Roberts <ryan.roberts@arm.com>
> >> Cc: Mark Rutland <mark.rutland@arm.com>
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Fixes: a6bbf5d4d9d1 ("arm64: mm: Add definitions to support 5 levels of paging")
> >> Reported-by: Dev Jain <dev.jain@arm.com>
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >> This patch applies on v6.9-rc4
> >>
> >>  arch/arm64/mm/hugetlbpage.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> > 
> > Acked-by: Ard Biesheuvel <ardb@kernel.org>
> > 
> > Thanks for fixing this. One question below.
> > 
> > 
> >> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> >> index 0f0e10bb0a95..5c819459555a 100644
> >> --- a/arch/arm64/mm/hugetlbpage.c
> >> +++ b/arch/arm64/mm/hugetlbpage.c
> >> @@ -276,7 +276,7 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma,
> >>         pte_t *ptep = NULL;
> >>
> >>         pgdp = pgd_offset(mm, addr);
> >> -       p4dp = p4d_offset(pgdp, addr);
> >> +       p4dp = p4d_alloc(mm, pgdp, addr);
> > 
> > Shouldn't we check p4dp for NULL here?
> 
> Sure, makes sense. I will add a check for p4dp, just like pudp below.

With that:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> 
> > 
> >>         pudp = pud_alloc(mm, p4dp, addr);
> >>         if (!pudp)
> >>                 return NULL;
> >> --
> >> 2.25.1
> >>

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

end of thread, other threads:[~2024-04-15  8:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-15  6:31 [PATCH] arm64/hugetlb: Fix page table walk in huge_pte_alloc() Anshuman Khandual
2024-04-15  6:38 ` Ard Biesheuvel
2024-04-15  6:52   ` Anshuman Khandual
2024-04-15  8:37     ` Mark Rutland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).