* [PATCH efi-next] efi/x86: ignore memory attributes table on i386
@ 2020-03-04 16:59 Ard Biesheuvel
2020-03-04 17:15 ` Arvind Sankar
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 16:59 UTC (permalink / raw)
To: linux-efi; +Cc: nivedita, Ard Biesheuvel
Commit 3a6b6c6fb23667fa ("efi: Make EFI_MEMORY_ATTRIBUTES_TABLE
initialization common across all architectures") moved the call to
efi_memattr_init() from ARM specific to generic EFI init code, in
order to be able to apply the restricted permissions described in
that table on x86 as well.
We never enabled this feature fully on i386, and so mapping and
reserving this table is pointless. However, due to the early call to
memblock_reserve(), the memory bookkeeping gets confused to the point
where it produces the splat below when we try to map the memory later
on:
------------[ cut here ]------------
ioremap on RAM at 0x3f251000 - 0x3fa1afff
WARNING: CPU: 0 PID: 0 at arch/x86/mm/ioremap.c:166 __ioremap_caller ...
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.20.0 #48
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
EIP: __ioremap_caller.constprop.0+0x249/0x260
Code: 90 0f b7 05 4e 38 40 de 09 45 e0 e9 09 ff ff ff 90 8d 45 ec c6 05 ...
EAX: 00000029 EBX: 00000000 ECX: de59c228 EDX: 00000001
ESI: 3f250fff EDI: 00000000 EBP: de3edf20 ESP: de3edee0
DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00200296
CR0: 80050033 CR2: ffd17000 CR3: 1e58c000 CR4: 00040690
Call Trace:
ioremap_cache+0xd/0x10
? old_map_region+0x72/0x9d
old_map_region+0x72/0x9d
efi_map_region+0x8/0xa
efi_enter_virtual_mode+0x260/0x43b
start_kernel+0x329/0x3aa
i386_start_kernel+0xa7/0xab
startup_32_smp+0x164/0x168
---[ end trace e15ccf6b9f356833 ]---
Let's work around this by disregarding the memory attributes table
altogether on i386, which does not result in a loss of functionality
or protection, given that we never consumed the contents.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
drivers/firmware/efi/efi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index d1746a579c99..34bca039fca0 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -543,7 +543,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
}
}
- if (efi_enabled(EFI_MEMMAP))
+ if (!IS_ENABLED(CONFIG_X86_32) && efi_enabled(EFI_MEMMAP))
efi_memattr_init();
efi_tpm_eventlog_init();
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH efi-next] efi/x86: ignore memory attributes table on i386
2020-03-04 16:59 [PATCH efi-next] efi/x86: ignore memory attributes table on i386 Ard Biesheuvel
@ 2020-03-04 17:15 ` Arvind Sankar
2020-03-04 17:17 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Arvind Sankar @ 2020-03-04 17:15 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, nivedita
On Wed, Mar 04, 2020 at 05:59:17PM +0100, Ard Biesheuvel wrote:
> Commit 3a6b6c6fb23667fa ("efi: Make EFI_MEMORY_ATTRIBUTES_TABLE
> initialization common across all architectures") moved the call to
> efi_memattr_init() from ARM specific to generic EFI init code, in
> order to be able to apply the restricted permissions described in
> that table on x86 as well.
>
> We never enabled this feature fully on i386, and so mapping and
> reserving this table is pointless. However, due to the early call to
> memblock_reserve(), the memory bookkeeping gets confused to the point
> where it produces the splat below when we try to map the memory later
> on:
>
> ------------[ cut here ]------------
> ioremap on RAM at 0x3f251000 - 0x3fa1afff
> WARNING: CPU: 0 PID: 0 at arch/x86/mm/ioremap.c:166 __ioremap_caller ...
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.20.0 #48
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
> EIP: __ioremap_caller.constprop.0+0x249/0x260
> Code: 90 0f b7 05 4e 38 40 de 09 45 e0 e9 09 ff ff ff 90 8d 45 ec c6 05 ...
> EAX: 00000029 EBX: 00000000 ECX: de59c228 EDX: 00000001
> ESI: 3f250fff EDI: 00000000 EBP: de3edf20 ESP: de3edee0
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00200296
> CR0: 80050033 CR2: ffd17000 CR3: 1e58c000 CR4: 00040690
> Call Trace:
> ioremap_cache+0xd/0x10
> ? old_map_region+0x72/0x9d
> old_map_region+0x72/0x9d
> efi_map_region+0x8/0xa
> efi_enter_virtual_mode+0x260/0x43b
> start_kernel+0x329/0x3aa
> i386_start_kernel+0xa7/0xab
> startup_32_smp+0x164/0x168
> ---[ end trace e15ccf6b9f356833 ]---
>
> Let's work around this by disregarding the memory attributes table
> altogether on i386, which does not result in a loss of functionality
> or protection, given that we never consumed the contents.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Thanks!
Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
> drivers/firmware/efi/efi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index d1746a579c99..34bca039fca0 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -543,7 +543,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
> }
> }
>
> - if (efi_enabled(EFI_MEMMAP))
> + if (!IS_ENABLED(CONFIG_X86_32) && efi_enabled(EFI_MEMMAP))
> efi_memattr_init();
>
> efi_tpm_eventlog_init();
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH efi-next] efi/x86: ignore memory attributes table on i386
2020-03-04 17:15 ` Arvind Sankar
@ 2020-03-04 17:17 ` Ard Biesheuvel
0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 17:17 UTC (permalink / raw)
To: Arvind Sankar; +Cc: linux-efi
On Wed, 4 Mar 2020 at 18:15, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Wed, Mar 04, 2020 at 05:59:17PM +0100, Ard Biesheuvel wrote:
> > Commit 3a6b6c6fb23667fa ("efi: Make EFI_MEMORY_ATTRIBUTES_TABLE
> > initialization common across all architectures") moved the call to
> > efi_memattr_init() from ARM specific to generic EFI init code, in
> > order to be able to apply the restricted permissions described in
> > that table on x86 as well.
> >
> > We never enabled this feature fully on i386, and so mapping and
> > reserving this table is pointless. However, due to the early call to
> > memblock_reserve(), the memory bookkeeping gets confused to the point
> > where it produces the splat below when we try to map the memory later
> > on:
> >
> > ------------[ cut here ]------------
> > ioremap on RAM at 0x3f251000 - 0x3fa1afff
> > WARNING: CPU: 0 PID: 0 at arch/x86/mm/ioremap.c:166 __ioremap_caller ...
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.20.0 #48
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
> > EIP: __ioremap_caller.constprop.0+0x249/0x260
> > Code: 90 0f b7 05 4e 38 40 de 09 45 e0 e9 09 ff ff ff 90 8d 45 ec c6 05 ...
> > EAX: 00000029 EBX: 00000000 ECX: de59c228 EDX: 00000001
> > ESI: 3f250fff EDI: 00000000 EBP: de3edf20 ESP: de3edee0
> > DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00200296
> > CR0: 80050033 CR2: ffd17000 CR3: 1e58c000 CR4: 00040690
> > Call Trace:
> > ioremap_cache+0xd/0x10
> > ? old_map_region+0x72/0x9d
> > old_map_region+0x72/0x9d
> > efi_map_region+0x8/0xa
> > efi_enter_virtual_mode+0x260/0x43b
> > start_kernel+0x329/0x3aa
> > i386_start_kernel+0xa7/0xab
> > startup_32_smp+0x164/0x168
> > ---[ end trace e15ccf6b9f356833 ]---
> >
> > Let's work around this by disregarding the memory attributes table
> > altogether on i386, which does not result in a loss of functionality
> > or protection, given that we never consumed the contents.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Thanks!
>
> Tested-by: Arvind Sankar <nivedita@alum.mit.edu>
>
Thanks. I guess nobody noticed because nobody enabled this feature on
production i386 hardware.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-04 17:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04 16:59 [PATCH efi-next] efi/x86: ignore memory attributes table on i386 Ard Biesheuvel
2020-03-04 17:15 ` Arvind Sankar
2020-03-04 17:17 ` 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).