2022-02-01 19:53:12

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 5.10 011/100] efi: runtime: avoid EFIv2 runtime services on Apple x86 machines

From: Ard Biesheuvel <[email protected]>

commit f5390cd0b43c2e54c7cf5506c7da4a37c5cef746 upstream.

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]>
Tested-by: Aditya Garg <[email protected]>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215277
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/firmware/efi/efi.c | 7 +++++++
1 file changed, 7 insertions(+)

--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -719,6 +719,13 @@ void __init efi_systab_report_header(con
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] = {



2022-02-04 11:50:49

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 5.10 011/100] efi: runtime: avoid EFIv2 runtime services on Apple x86 machines

On Thu, Feb 03, 2022 at 09:52:23PM +0100, Pavel Machek wrote:

> This problem is not 64-bit specific, right? Should it depend on
> CONFIG_X86, instead?

Only 64-bit systems are affected (the one 32-bit generation of Apple
hardware implementing EFI only claims 1.10 support), and 32-bit kernels
can't make 64-bit UEFI runtime calls, so I think it's correct to make it
64-bit only.

2022-02-05 18:34:12

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 5.10 011/100] efi: runtime: avoid EFIv2 runtime services on Apple x86 machines

Hi!

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

So Apple x86 machines claim they support EFI v2 but really don't?

> +++ b/drivers/firmware/efi/efi.c
> @@ -719,6 +719,13 @@ void __init efi_systab_report_header(con
> 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;
> + }
> }

This problem is not 64-bit specific, right? Should it depend on
CONFIG_X86, instead?

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Attachments:
(No filename) (2.13 kB)
signature.asc (201.00 B)
Download all attachments