Skip to contentSkip to conversation

efi: runtime: avoid EFIv2 runtime services on Apple x86 machines

PATCH In review Ard Biesheuvel 2 participants 3 messages Jan 12, 2022 – Jan 18, 2022

Subsystems (2)
Stable branch EFI
lore
Share & tools
lore

Ard Biesheuvel

author maintainer
#0 lore raw

PATCH 1/1 efi: runtime: avoid EFIv2 runtime services on Apple x86 machines

Aditya reports [0] that his recent MacbookPro crashes in the firmware when using the variable services at runtime. The culprit appears to be a call to QueryVariableInfo(), which we did not use to call on Apple x86 machines in the past as they only upgraded from EFI v1.10 to EFI v2.40 firmware fairly recently, and QueryVariableInfo() (along with UpdateCapsule() et al) was added in EFI v2.00.

The only runtime service introduced in EFI v2.00 that we actually use in Linux is QueryVariableInfo(), as the capsule based ones are optional, generally not used at runtime (all the LVFS/fwupd firmware update infrastructure uses helper EFI programs that invoke capsule update at boot time, not runtime), and not implemented by Apple machines in the first place. QueryVariableInfo() is used to 'safely' set variables, i.e., only when there is enough space. This prevents machines with buggy firmwares from corrupting their NVRAMs when they run out of space.

Given that Apple machines have been using EFI v1.10 services only for the longest time (the EFI v2.0 spec was released in 2006, and Linux support for the newly introduced runtime services was added in 2011, but the MacbookPro12,1 released in 2015 still claims to be EFI v1.10 only), let's avoid the EFI v2.0 ones on all Apple x86 machines.

[0] https://lore.kernel.org/all/[email protected]/

Cc <[email protected]>
Cc Jeremy Kerr <[email protected]>
Cc Matthew Garrett <[email protected]>
Reported-by Aditya Garg <[email protected]>
Tested-by Orlando Chamberlain <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 drivers/firmware/efi/efi.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/firmware/efi/efi.c +7 -0
index ae79c3300129..7de3f5b6e8d0 100644
@@ -722,6 +722,13 @@ void __init efi_systab_report_header(const efi_table_hdr_t *systab_hdr,
 		systab_hdr->revision >> 16,
 		systab_hdr->revision & 0xffff,
 		vendor);
+
+	if (IS_ENABLED(CONFIG_X86_64) &&
+	    systab_hdr->revision > EFI_1_10_SYSTEM_TABLE_REVISION &&
+	    !strcmp(vendor, "Apple")) {
+		pr_info("Apple Mac detected, using EFI v1.10 runtime services only\n");
+		efi.runtime_version = EFI_1_10_SYSTEM_TABLE_REVISION;
+	}
 }
 
 static __initdata char memory_type_name[][13] = {
Signature · 1 line
2.30.2
Tested-by on v1 1/1
#1 lore raw
Quoted 58 lines · Ard Biesheuvel

Aditya reports [0] that his recent MacbookPro crashes in the firmware when using the variable services at runtime. The culprit appears to be a call to QueryVariableInfo(), which we did not use to call on Apple x86 machines in the past as they only upgraded from EFI v1.10 to EFI v2.40 firmware fairly recently, and QueryVariableInfo() (along with UpdateCapsule() et al) was added in EFI v2.00.

The only runtime service introduced in EFI v2.00 that we actually use in Linux is QueryVariableInfo(), as the capsule based ones are optional, generally not used at runtime (all the LVFS/fwupd firmware update infrastructure uses helper EFI programs that invoke capsule update at boot time, not runtime), and not implemented by Apple machines in the first place. QueryVariableInfo() is used to 'safely' set variables, i.e., only when there is enough space. This prevents machines with buggy firmwares from corrupting their NVRAMs when they run out of space.

Given that Apple machines have been using EFI v1.10 services only for the longest time (the EFI v2.0 spec was released in 2006, and Linux support for the newly introduced runtime services was added in 2011, but the MacbookPro12,1 released in 2015 still claims to be EFI v1.10 only), let's avoid the EFI v2.0 ones on all Apple x86 machines.

[0] https://lore.kernel.org/all/[email protected]/

Cc <[email protected]>
Cc Jeremy Kerr <[email protected]>
Cc Matthew Garrett <[email protected]>
Reported-by Aditya Garg <[email protected]>
Tested-by Orlando Chamberlain <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>

---
drivers/firmware/efi/efi.c | 7 +++++++

1 file changed, 7 insertions(+)
drivers/firmware/efi/efi.c +7 -0
index ae79c3300129..7de3f5b6e8d0 100644
@@ -722,6 +722,13 @@ void __init efi_systab_report_header(const efi_table_hdr_t *systab_hdr,
 		systab_hdr->revision >> 16,
 		systab_hdr->revision & 0xffff,
 		vendor);
+
+	if (IS_ENABLED(CONFIG_X86_64) &&
+	    systab_hdr->revision > EFI_1_10_SYSTEM_TABLE_REVISION &&
+	    !strcmp(vendor, "Apple")) {
+		pr_info("Apple Mac detected, using EFI v1.10 runtime services only\n");
+		efi.runtime_version = EFI_1_10_SYSTEM_TABLE_REVISION;
+	}
 }
 
 static __initdata char memory_type_name[][13] = {
Signature · 1 line
2.30.2

Hi Ard

Patch works for me. Thanks for investing time to fix the issue :)

Tested-by Aditya Garg <[email protected]>

You may also add the link of the Bug report

Link https://bugzilla.kernel.org/show_bug.cgi?id=215277

Regards
Aditya

#2 lore raw
In reply to Ard Biesheuvel

Hi Ard

Quoted 24 lines

drivers/firmware/efi/efi.c | 7 +++++++

1 file changed, 7 insertions(+)
drivers/firmware/efi/efi.c +7 -0
index ae79c3300129..7de3f5b6e8d0 100644
@@ -722,6 +722,13 @@ void __init efi_systab_report_header(const efi_table_hdr_t *systab_hdr,
 		systab_hdr->revision >> 16,
 		systab_hdr->revision & 0xffff,
 		vendor);
+
+	if (IS_ENABLED(CONFIG_X86_64) &&
+	    systab_hdr->revision > EFI_1_10_SYSTEM_TABLE_REVISION &&
+	    !strcmp(vendor, "Apple")) {
+		pr_info("Apple Mac detected, using EFI v1.10 runtime services only\n");
+		efi.runtime_version = EFI_1_10_SYSTEM_TABLE_REVISION;
+	}
 }
 
 static __initdata char memory_type_name[][13] = {
Signature · 1 line
2.30.2

Just a friendly reminder to upstream this patch.