[edk2-devel] EFI table being corrupted during Kexec

[edk2-devel] EFI table being corrupted during Kexec

[Breno Leitao]

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?

Thanks
--breno


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120541): https://edk2.groups.io/g/devel/message/120541
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI table being corrupted during Kexec

[Breno Leitao]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI table being corrupted during Kexec

[Andrew Fish via groups.io]

> On Sep 10, 2024, at 6:58 AM, Breno Leitao <leitao@debian.org> wrote:
> 
> 
> 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.
> 

The E820 table is not part of the UEFI standard and it is produced by a library in the OvmfPkg for “special cases” so I guess that lib could have a bug?

> 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.
> 

INT 15h is Legacy BIOS not UEFI. For UEFI there is just the UEFI memory map and ACPI. 

Thanks,

Andrew Fish


> 
> 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?
> 
> Thanks
> --breno
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120543): https://edk2.groups.io/g/devel/message/120543
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI table being corrupted during Kexec

[Usama Arif]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI table being corrupted during Kexec

[Breno Leitao]

hello Usama,

On Tue, Sep 10, 2024 at 04:46:15PM +0100, Usama Arif wrote:
> --- 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);
> +	}

Shouldn't you reserve the region into 8250 independently of
memblock_reserve() return value?

Thanks for the patch,
--breno


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120545): https://edk2.groups.io/g/devel/message/120545
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI table being corrupted during Kexec

[Gregory Price]

On Tue, Sep 10, 2024 at 09:09:21AM -0700, Breno Leitao wrote:
> hello Usama,
> 
> On Tue, Sep 10, 2024 at 04:46:15PM +0100, Usama Arif wrote:
> > --- 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);
> > +	}
> 
> Shouldn't you reserve the region into 8250 independently of
> memblock_reserve() return value?
> 
> Thanks for the patch,
> --breno

Probably also want some sanity check here that we're not over
writing already reserved areas before we just update the map.

If we're dealing with the scenario where we can't trust the
hardware/efi generated map, we probably want to put a small
amount of effort to ensure we're not wrecking the state when
the system is working correctly.

Only so much we can do in this scenario.

~Gregory


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120546): https://edk2.groups.io/g/devel/message/120546
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI table being corrupted during Kexec

[Gerd Hoffmann]

On Tue, Sep 10, 2024 at 08:44:40AM GMT, Andrew Fish via groups.io wrote:
> 
> 
> > On Sep 10, 2024, at 6:58 AM, Breno Leitao <leitao@debian.org> wrote:
> > 
> > 
> > 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.
> 
> The E820 table is not part of the UEFI standard and it is produced by
> a library in the OvmfPkg for “special cases” so I guess that lib could
> have a bug?

This "special case" is direct kernel boot ('qemu -kernel vmlinux') and
loading the linux kernel as EFI binary failed.  That should only happen
with linux kernels so old that they do not have the efi stub.

Also note "problem happen on real machine" above, so it clearly is not
something OVMF-specific.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120549): https://edk2.groups.io/g/devel/message/120549
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI table being corrupted during Kexec

[Usama Arif]

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]
-=-=-=-=-=-=-=-=-=-=-=-