From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 7521F78003C for ; Tue, 10 Sep 2024 15:47:34 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=alSloxRWLYyzB1REHdP0bGq1VPAv6y5vOiqaFlqZVys=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20240830; t=1725983254; v=1; x=1726242453; b=XrU1D5LptXnc0QsFCWpEkX0lAKeiSeyxw3b2J2kXrVcITRjvdOaims/n0X5V/BeOs2tkdNPG 82JxNcdqoA5tGXohxFS40cNJhrg7ae+UEiZN/WeNdpW2BOR6oBARwmONCQ0mdyShcabx8UtAZdg qsIPA+FHIfo03XQ51nGEtBuO0NBa0sd08CEKRME8mfzpwO8Q92ojPaofSOvHibxH794uX82/nPX iVD8uzbU2tAoL3+hVMvVhGUlOfNAZAlXyU4QqEzlX2dQhfqEOaVCViRlC5nToOZKxqKJfODxjV3 TwvC+PRRNntWWzV/ibHgAFCBVs6GQvlracW787Po/xGoQ== X-Received: by 127.0.0.2 with SMTP id GYhcYY7687511xaD8mbv5fqI; Tue, 10 Sep 2024 08:47:33 -0700 X-Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by mx.groups.io with SMTP id smtpd.web11.26832.1725983179077409194 for ; Tue, 10 Sep 2024 08:46:19 -0700 X-Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-5c3cdba33b0so6000246a12.1 for ; Tue, 10 Sep 2024 08:46:18 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCVWi9Dwm+iZYSkAw6v/kNjschHCuyKzamXhCfJFEZtKwaC/oHcbEVhqIHdpNFzADrFpnk2GPQ==@edk2.groups.io X-Gm-Message-State: pZ2MoXhp6pKWmzNImdlvwfj1x7686176AA= X-Google-Smtp-Source: AGHT+IFMQLfuXiiliWsCtziJxd30hsALblQ4GT8eOdoksyoOUVS3BwT/NFM4Vc0jDrzrMCc3oZ9Mpw== X-Received: by 2002:a05:6402:5203:b0:5c2:6e51:9d22 with SMTP id 4fb4d7f45d1cf-5c3dc7bd575mr10172025a12.25.1725983176535; Tue, 10 Sep 2024 08:46:16 -0700 (PDT) X-Received: from ?IPV6:2a03:83e0:1126:4:eb:d0d0:c7fd:c82c? ([2620:10d:c092:500::4:61d8]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5c3ebd41cd7sm4422605a12.3.2024.09.10.08.46.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 10 Sep 2024 08:46:16 -0700 (PDT) Message-ID: <9b024f7d-e326-46eb-bd88-71a16151fcf0@gmail.com> Date: Tue, 10 Sep 2024 16:46:15 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] EFI table being corrupted during Kexec To: "Eric W. Biederman" , Breno Leitao Cc: ardb@kernel.org, linux-efi@vger.kernel.org, kexec@lists.infradead.org, bhe@redhat.com, vgoyal@redhat.com, devel@edk2.groups.io, rppt@kernel.org, gourry@gourry.net, rmikey@meta.com, tglx@linutronix.de References: <20240910-juicy-festive-sambar-9ad23a@devvm32600> <87ed5rd1qf.fsf@email.froward.int.ebiederm.org> From: Usama Arif In-Reply-To: <87ed5rd1qf.fsf@email.froward.int.ebiederm.org> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Tue, 10 Sep 2024 08:47:32 -0700 Resent-From: usamaarif642@gmail.com Reply-To: devel@edk2.groups.io,usamaarif642@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b=XrU1D5Lp; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io On 10/09/2024 15:26, Eric W. Biederman wrote: > Breno Leitao 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 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 --- 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] -=-=-=-=-=-=-=-=-=-=-=-