* Re: [edk2-devel] EFI table being corrupted during Kexec
[not found] ` <87ed5rd1qf.fsf@email.froward.int.ebiederm.org>
@ 2024-09-10 15:13 ` Breno Leitao
2024-09-10 15:46 ` Usama Arif
2024-09-11 10:58 ` Usama Arif
2 siblings, 0 replies; 8+ messages in thread
From: Breno Leitao @ 2024-09-10 15:13 UTC (permalink / raw)
To: Eric W. Biederman
Cc: ardb, linux-efi, kexec, bhe, vgoyal, devel, rppt, usamaarif642,
gourry, rmikey
Hello Eric,
On Tue, Sep 10, 2024 at 09:26:00AM -0500, Eric W. Biederman wrote:
> > I am wondering if that memory region/range should be part of e820 table that is
> > passed by EFI firmware to kernel, and if it is not passed (as it is not being
> > passed today), then the kernel doesn't need to respect it, and it is free to
> > overwrite (as it does today). In other words, this is a firmware bug and not a
> > kernel bug.
> >
> > Am I missing something?
>
> I agree that this appears to be a firmware bug. This memory is reserved
> in one location and not in another location.
That was is our current understanding also, but, having the same issue
in EDK2 and on a real machine firmware was surprising.
Anyway, I've CCed the EDK2 mailing list in this thread as well, let's
see if someone has any comment.
> As I recall the memblock allocator is the bootstrap memory allocator
> used when bringing up the kernel. So I don't see reserving something
> in the memblock allocator as being authoritative as to how the firmware
> has setup memory.
>
> I would suggest writing a patch to update whatever is calling
> memblock_reserve to also, or perhaps in preference to update the e820
> map. If the code is not x86 specific I would suggest using ACPI's
> arch_reserve_mem_area call.
Should all memblock_reserve() memory ranges be mapped to e820 table, or,
just specific cases where we see problems?
Thanks
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120542): https://edk2.groups.io/g/devel/message/120542
Mute This Topic: https://groups.io/mt/108376671/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [edk2-devel] EFI table being corrupted during Kexec
[not found] ` <87ed5rd1qf.fsf@email.froward.int.ebiederm.org>
2024-09-10 15:13 ` Breno Leitao
@ 2024-09-10 15:46 ` Usama Arif
2024-09-10 16:09 ` Breno Leitao
2024-09-11 10:58 ` Usama Arif
2 siblings, 1 reply; 8+ messages in thread
From: Usama Arif @ 2024-09-10 15:46 UTC (permalink / raw)
To: Eric W. Biederman, Breno Leitao
Cc: ardb, linux-efi, kexec, bhe, vgoyal, devel, rppt, gourry, rmikey,
tglx
On 10/09/2024 15:26, Eric W. Biederman wrote:
> Breno Leitao <leitao@debian.org> writes:
>
>> We've seen a problem in upstream kernel kexec, where a EFI TPM log event table
>> is being overwritten. This problem happen on real machine, as well as in a
>> recent EDK2 qemu VM.
>>
>> Digging deep, the table is being overwritten during kexec, more precisely when
>> relocating kernel (relocate_kernel() function).
>>
>> I've also found that the table is being properly reserved using
>> memblock_reserve() early in the boot, and that range gets overwritten later in
>> by relocate_kernel(). In other words, kexec is overwriting a memory that was
>> previously reserved (as memblock_reserve()).
>>
>> Usama found that kexec only honours memory reservations from /sys/firmware/memmap
>> which comes from e820_table_firmware table.
>>
>> Looking at the TPM spec, I found the following part:
>>
>> If the ACPI TPM2 table contains the address and size of the Platform Firmware TCG log,
>> firmware “pins” the memory associated with the Platform Firmware TCG log, and reports
>> this memory as “Reserved” memory via the INT 15h/E820 interface.
>>
>>
>> From: https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientPlatform_Profile_for_TPM_2p0_Systems_v49_161114_public-review.pdf
>>
>> I am wondering if that memory region/range should be part of e820 table that is
>> passed by EFI firmware to kernel, and if it is not passed (as it is not being
>> passed today), then the kernel doesn't need to respect it, and it is free to
>> overwrite (as it does today). In other words, this is a firmware bug and not a
>> kernel bug.
>>
>> Am I missing something?
>
> I agree that this appears to be a firmware bug. This memory is reserved
> in one location and not in another location.
>
> That said that doesn't mean we can't deal with it in the kernel.
> acpi_table_upgrade seems to have hit a similar issue issue and calls
> arch_reserve_mem_area to reserve the area in the e820tables.
>
>
> The last time I looked the e820 tables (in the kernel) are used to store
> the efi memory map when available and only use the true e820 data on
> older systems.
>
> Which is a long way of say that the e820 table in the kernel last I
> looked was the master table, of how the firmware views the memory.
>
>
> As I recall the memblock allocator is the bootstrap memory allocator
> used when bringing up the kernel. So I don't see reserving something
> in the memblock allocator as being authoritative as to how the firmware
> has setup memory.
>
>
m>
> I would suggest writing a patch to update whatever is calling
> memblock_reserve to also, or perhaps in preference to update the e820
> map. If the code is not x86 specific I would suggest using ACPI's
> arch_reserve_mem_area call.
>
So I believe arch_reserve_mem_area is unfortunately not enough. It updates
e820_table, but kexec seems to use e820_table_firmware. I believe the proper
fix should be in efi firmware, which might be a bit difficult to get through.
But with the below secondary fix in kernel, the corruption is gone, it would be
good to have efi, tpm and kexec experts to look at this and tell if it makes sense?
Thanks!
Usama
From cd8a9a8b5903fa86f4f76a250fd48bc2abfb13d6 Mon Sep 17 00:00:00 2001
From: Usama Arif <usamaarif642@gmail.com>
Date: Tue, 10 Sep 2024 16:14:38 +0100
Subject: [PATCH] efi/tpm: add efi.tpm_log as a reserved region in
820_table_firmware
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Looking at the TPM spec [1]
If the ACPI TPM2 table contains the address and size of the Platform
Firmware TCG log, firmware “pins” the memory associated with the
Platform FirmwareTCG log, and reports this memory as “Reserved” memory
via the INT 15h/E820 interface.
It looks like the firmware should pass this as reserved in e820 memory
map. However, it doesn't seem to. The firmware being tested on is:
dmidecode -s bios-version
edk2-20240214-2.el9
When this area is not reserved, it comes up as usable in
/sys/firmware/memmap. This means that kexec, which uses that memmap
to find usable memory regions, can select the region where efi.tpm_log
is and overwrite it and relocate_kernel.
Having a fix in firmware can be difficult to get through. As a secondary fix,
this patch marks that region as reserved in e820_table_firmware so that
kexec doesn't use it for kernel segments.
[1] https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientPlatform_Profile_for_TPM_2p0_Systems_v49_161114_public-review.pdf
Signed-off-by: Usama Arif <usamaarif642@gmail.com>
---
arch/x86/include/asm/e820/api.h | 2 ++
arch/x86/kernel/e820.c | 6 ++++++
arch/x86/platform/efi/efi.c | 6 ++++++
drivers/firmware/efi/tpm.c | 5 +++--
include/linux/efi.h | 7 +++++++
5 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
index 2e74a7f0e935..4e9aa24f03bd 100644
--- a/arch/x86/include/asm/e820/api.h
+++ b/arch/x86/include/asm/e820/api.h
@@ -16,6 +16,8 @@ extern bool e820__mapped_all(u64 start, u64 end, enum e820_type type);
extern void e820__range_add (u64 start, u64 size, enum e820_type type);
extern u64 e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
+extern u64 e820__range_update_firmware(u64 start, u64 size, enum e820_type old_type,
+ enum e820_type new_type);
extern u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type);
extern u64 e820__range_update_table(struct e820_table *t, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 4893d30ce438..912400161623 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -538,6 +538,12 @@ u64 __init e820__range_update_table(struct e820_table *t, u64 start, u64 size,
return __e820__range_update(t, start, size, old_type, new_type);
}
+u64 __init e820__range_update_firmware(u64 start, u64 size, enum e820_type old_type,
+ enum e820_type new_type)
+{
+ return __e820__range_update(e820_table_firmware, start, size, old_type, new_type);
+}
+
/* Remove a range of memory from the E820 table: */
u64 __init e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type)
{
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 88a96816de9a..644c5da8fb39 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -171,6 +171,12 @@ static void __init do_add_efi_memmap(void)
e820__update_table(e820_table);
}
+void arch_update_firmware_area(u64 addr, u64 size)
+{
+ e820__range_update_firmware(efi.tpm_log, size, E820_TYPE_RAM, E820_TYPE_RESERVED);
+ e820__update_table(e820_table_firmware);
+}
+
/*
* Given add_efi_memmap defaults to 0 and there is no alternative
* e820 mechanism for soft-reserved memory, import the full EFI memory
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index e8d69bd548f3..787c5a206010 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -60,7 +60,9 @@ int __init efi_tpm_eventlog_init(void)
}
tbl_size = sizeof(*log_tbl) + log_tbl->size;
- memblock_reserve(efi.tpm_log, tbl_size);
+ if (!memblock_reserve(efi.tpm_log, tbl_size)) {
+ arch_update_firmware_area(efi.tpm_log, tbl_size);
+ }
if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR) {
pr_info("TPM Final Events table not present\n");
@@ -107,4 +109,3 @@ int __init efi_tpm_eventlog_init(void)
early_memunmap(log_tbl, sizeof(*log_tbl));
return ret;
}
-
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 6bf3c4fe8511..9c239cdff771 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1371,4 +1371,11 @@ extern struct blocking_notifier_head efivar_ops_nh;
void efivars_generic_ops_register(void);
void efivars_generic_ops_unregister(void);
+#ifdef CONFIG_X86_64
+void __init arch_update_firmware_area(u64 addr, u64 size);
+#else
+static inline void __init arch_update_firmware_area(u64 addr, u64 size)
+{
+}
+#endif
#endif /* _LINUX_EFI_H */
--
2.43.5
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120544): https://edk2.groups.io/g/devel/message/120544
Mute This Topic: https://groups.io/mt/108376671/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [edk2-devel] EFI table being corrupted during Kexec
[not found] ` <87ed5rd1qf.fsf@email.froward.int.ebiederm.org>
2024-09-10 15:13 ` Breno Leitao
2024-09-10 15:46 ` Usama Arif
@ 2024-09-11 10:58 ` Usama Arif
2 siblings, 0 replies; 8+ messages in thread
From: Usama Arif @ 2024-09-11 10:58 UTC (permalink / raw)
To: ardb, linux-efi
Cc: kexec, bhe, vgoyal, devel, rppt, gourry, rmikey, afish, kraxel,
Eric W. Biederman, Breno Leitao
On 10/09/2024 15:26, Eric W. Biederman wrote:
> Breno Leitao <leitao@debian.org> writes:
>
>> We've seen a problem in upstream kernel kexec, where a EFI TPM log event table
>> is being overwritten. This problem happen on real machine, as well as in a
>> recent EDK2 qemu VM.
>>
>> Digging deep, the table is being overwritten during kexec, more precisely when
>> relocating kernel (relocate_kernel() function).
>>
>> I've also found that the table is being properly reserved using
>> memblock_reserve() early in the boot, and that range gets overwritten later in
>> by relocate_kernel(). In other words, kexec is overwriting a memory that was
>> previously reserved (as memblock_reserve()).
>>
>> Usama found that kexec only honours memory reservations from /sys/firmware/memmap
>> which comes from e820_table_firmware table.
>>
>> Looking at the TPM spec, I found the following part:
>>
>> If the ACPI TPM2 table contains the address and size of the Platform Firmware TCG log,
>> firmware “pins” the memory associated with the Platform Firmware TCG log, and reports
>> this memory as “Reserved” memory via the INT 15h/E820 interface.
>>
>>
>> From: https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientPlatform_Profile_for_TPM_2p0_Systems_v49_161114_public-review.pdf
>>
>> I am wondering if that memory region/range should be part of e820 table that is
>> passed by EFI firmware to kernel, and if it is not passed (as it is not being
>> passed today), then the kernel doesn't need to respect it, and it is free to
>> overwrite (as it does today). In other words, this is a firmware bug and not a
>> kernel bug.
>>
>> Am I missing something?
>
> I agree that this appears to be a firmware bug. This memory is reserved
> in one location and not in another location.
>
> That said that doesn't mean we can't deal with it in the kernel.
> acpi_table_upgrade seems to have hit a similar issue issue and calls
> arch_reserve_mem_area to reserve the area in the e820tables.
>
>
> The last time I looked the e820 tables (in the kernel) are used to store
> the efi memory map when available and only use the true e820 data on
> older systems.
>
> Which is a long way of say that the e820 table in the kernel last I
> looked was the master table, of how the firmware views the memory.
>
>
> As I recall the memblock allocator is the bootstrap memory allocator
> used when bringing up the kernel. So I don't see reserving something
> in the memblock allocator as being authoritative as to how the firmware
> has setup memory.
>
>
>
> I would suggest writing a patch to update whatever is calling
> memblock_reserve to also, or perhaps in preference to update the e820
> map. If the code is not x86 specific I would suggest using ACPI's
> arch_reserve_mem_area call.
>
Thanks, I have sent a potential fix for this at
https://lore.kernel.org/all/20240911104109.1831501-1-usamaarif642@gmail.com/
We can see this issue in kernels going all the way back to 5.12. Up until now it only corrupted
the tpm_log version, so it wasn't really an issue. After upgrading production to 6.9, the tpm_log
size has started to get corrupted as well. When size was corrupted to a negative value, the
memblock_reserve in efi_tpm_eventlog_init is reserving the entire memory available, and the system
OOMs at boot time, which is causing a serious issue. It would be good to know if the above patch is
an acceptable fix.
Thanks!
Usama
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120552): https://edk2.groups.io/g/devel/message/120552
Mute This Topic: https://groups.io/mt/108376671/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread