* [PATCH] ARM: mm: use fully constructed struct pages for EFI pgd allocations
@ 2016-07-22 18:42 Ard Biesheuvel
2016-07-23 11:02 ` Ard Biesheuvel
0 siblings, 1 reply; 2+ messages in thread
From: Ard Biesheuvel @ 2016-07-22 18:42 UTC (permalink / raw)
To: linux-arm-kernel
The late_alloc() PTE allocation function used by create_mapping_late()
does not call pgtable_page_ctor() on PTE pages it allocates, leaving
the per-page spinlock uninitialized.
Since generic page table manipulation code may assume that translation
table pages that are not owned by init_mm are covered by fully
constructed struct pages, the following crash may occur with the new
UEFI memory attributes table code.
efi: memattr: Processing EFI Memory Attributes table:
efi: memattr: 0x0000ffa16000-0x0000ffa82fff [Runtime Code |RUN| | |XP| | | | | | | | ]
Unable to handle kernel NULL pointer dereference at virtual address 00000010
pgd = c0204000
[00000010] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc4-00063-g3882aa7b340b #361
Hardware name: Generic DT based system
task: ed858000 ti: ed842000 task.ti: ed842000
PC is at __lock_acquire+0xa0/0x19a8
...
[<c038c830>] (__lock_acquire) from [<c038e4f8>] (lock_acquire+0x6c/0x88)
[<c038e4f8>] (lock_acquire) from [<c0c06134>] (_raw_spin_lock+0x2c/0x3c)
[<c0c06134>] (_raw_spin_lock) from [<c0410384>] (apply_to_page_range+0xe8/0x238)
[<c0410384>] (apply_to_page_range) from [<c1205f34>] (efi_set_mapping_permissions+0x54/0x5c)
[<c1205f34>] (efi_set_mapping_permissions) from [<c1247474>] (efi_memattr_apply_permissions+0x2b8/0x378)
[<c1247474>] (efi_memattr_apply_permissions) from [<c1248258>] (arm_enable_runtime_services+0x1f0/0x22c)
[<c1248258>] (arm_enable_runtime_services) from [<c0301f0c>] (do_one_initcall+0x44/0x174)
[<c0301f0c>] (do_one_initcall) from [<c1200d10>] (kernel_init_freeable+0x90/0x1e8)
[<c1200d10>] (kernel_init_freeable) from [<c0bff690>] (kernel_init+0x8/0x114)
[<c0bff690>] (kernel_init) from [<c0307ed0>] (ret_from_fork+0x14/0x24)
The crash is due to the fact that the UEFI page tables are not owned by
init_mm, but are not covered by fully constructed struct pages.
Given that the UEFI subsystem is currently the only user of
create_mapping_late(), add an unconditional call to pgtable_page_ctor() to
late_alloc().
Fixes: 9fc68b717c24 ("ARM/efi: Apply strict permissions for UEFI Runtime Services regions")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
The commit in question was introduced in v4.7, so ideally this should go in
as a late fix. However, EFI on ARM is not widely used [yet], and the memory
attributes table even less so, so I am perfectly happy for this to go in
later.
arch/arm/mm/mmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 62f4d01941f7..d0ac45451805 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -728,7 +728,7 @@ static void *__init late_alloc(unsigned long sz)
{
void *ptr = (void *)__get_free_pages(PGALLOC_GFP, get_order(sz));
- BUG_ON(!ptr);
+ BUG_ON(!ptr || !pgtable_page_ctor(virt_to_page(ptr)));
return ptr;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH] ARM: mm: use fully constructed struct pages for EFI pgd allocations
2016-07-22 18:42 [PATCH] ARM: mm: use fully constructed struct pages for EFI pgd allocations Ard Biesheuvel
@ 2016-07-23 11:02 ` Ard Biesheuvel
0 siblings, 0 replies; 2+ messages in thread
From: Ard Biesheuvel @ 2016-07-23 11:02 UTC (permalink / raw)
To: linux-arm-kernel
On 22 July 2016 at 20:42, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> The late_alloc() PTE allocation function used by create_mapping_late()
> does not call pgtable_page_ctor() on PTE pages it allocates, leaving
> the per-page spinlock uninitialized.
>
> Since generic page table manipulation code may assume that translation
> table pages that are not owned by init_mm are covered by fully
> constructed struct pages, the following crash may occur with the new
> UEFI memory attributes table code.
>
> efi: memattr: Processing EFI Memory Attributes table:
> efi: memattr: 0x0000ffa16000-0x0000ffa82fff [Runtime Code |RUN| | |XP| | | | | | | | ]
> Unable to handle kernel NULL pointer dereference at virtual address 00000010
> pgd = c0204000
> [00000010] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc4-00063-g3882aa7b340b #361
> Hardware name: Generic DT based system
> task: ed858000 ti: ed842000 task.ti: ed842000
> PC is at __lock_acquire+0xa0/0x19a8
> ...
> [<c038c830>] (__lock_acquire) from [<c038e4f8>] (lock_acquire+0x6c/0x88)
> [<c038e4f8>] (lock_acquire) from [<c0c06134>] (_raw_spin_lock+0x2c/0x3c)
> [<c0c06134>] (_raw_spin_lock) from [<c0410384>] (apply_to_page_range+0xe8/0x238)
> [<c0410384>] (apply_to_page_range) from [<c1205f34>] (efi_set_mapping_permissions+0x54/0x5c)
> [<c1205f34>] (efi_set_mapping_permissions) from [<c1247474>] (efi_memattr_apply_permissions+0x2b8/0x378)
> [<c1247474>] (efi_memattr_apply_permissions) from [<c1248258>] (arm_enable_runtime_services+0x1f0/0x22c)
> [<c1248258>] (arm_enable_runtime_services) from [<c0301f0c>] (do_one_initcall+0x44/0x174)
> [<c0301f0c>] (do_one_initcall) from [<c1200d10>] (kernel_init_freeable+0x90/0x1e8)
> [<c1200d10>] (kernel_init_freeable) from [<c0bff690>] (kernel_init+0x8/0x114)
> [<c0bff690>] (kernel_init) from [<c0307ed0>] (ret_from_fork+0x14/0x24)
>
> The crash is due to the fact that the UEFI page tables are not owned by
> init_mm, but are not covered by fully constructed struct pages.
>
> Given that the UEFI subsystem is currently the only user of
> create_mapping_late(), add an unconditional call to pgtable_page_ctor() to
> late_alloc().
>
> Fixes: 9fc68b717c24 ("ARM/efi: Apply strict permissions for UEFI Runtime Services regions")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>
> The commit in question was introduced in v4.7, so ideally this should go in
> as a late fix. However, EFI on ARM is not widely used [yet], and the memory
> attributes table even less so, so I am perfectly happy for this to go in
> later.
>
> arch/arm/mm/mmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 62f4d01941f7..d0ac45451805 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -728,7 +728,7 @@ static void *__init late_alloc(unsigned long sz)
> {
> void *ptr = (void *)__get_free_pages(PGALLOC_GFP, get_order(sz));
>
> - BUG_ON(!ptr);
> + BUG_ON(!ptr || !pgtable_page_ctor(virt_to_page(ptr)));
Actually, putting code with side effects inside BUG_ON() is not a good
idea, and I deliberately avoided doing so in the arm64 counterpart of
this patch.
If there are no other comments on this patch, I will fix that up
before dropping this in the patch system
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-07-23 11:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-22 18:42 [PATCH] ARM: mm: use fully constructed struct pages for EFI pgd allocations Ard Biesheuvel
2016-07-23 11:02 ` Ard Biesheuvel
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 NNTP newsgroup(s).