| Message ID | 20103919-A276-4CA6-B1AD-6E45DB58500B@live.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] efi: Do not import certificates from UEFI Secure Boot for T2 Macs | expand |
On Thu, 10 Feb 2022 at 11:45, Aditya Garg <gargaditya08@live.com> wrote: > > From: Aditya Garg <gargaditya08@live.com> > > On T2 Macs, the secure boot is handled by the T2 Chip. If enabled, only > macOS and Windows are allowed to boot on these machines. Thus we need to > disable secure boot for Linux. If we boot into Linux after disabling > secure boot, if CONFIG_LOAD_UEFI_KEYS is enabled, EFI Runtime services > fail to start, with the following logs in dmesg > > Call Trace: > <TASK> > page_fault_oops+0x4f/0x2c0 > ? search_bpf_extables+0x6b/0x80 > ? search_module_extables+0x50/0x80 > ? search_exception_tables+0x5b/0x60 > kernelmode_fixup_or_oops+0x9e/0x110 > __bad_area_nosemaphore+0x155/0x190 > bad_area_nosemaphore+0x16/0x20 > do_kern_addr_fault+0x8c/0xa0 > exc_page_fault+0xd8/0x180 > asm_exc_page_fault+0x1e/0x30 > (Removed some logs from here) > ? __efi_call+0x28/0x30 > ? switch_mm+0x20/0x30 > ? efi_call_rts+0x19a/0x8e0 > ? process_one_work+0x222/0x3f0 > ? worker_thread+0x4a/0x3d0 > ? kthread+0x17a/0x1a0 > ? process_one_work+0x3f0/0x3f0 > ? set_kthread_struct+0x40/0x40 > ? ret_from_fork+0x22/0x30 > </TASK> > ---[ end trace 1f82023595a5927f ]--- > efi: Froze efi_rts_wq and disabled EFI Runtime Services > integrity: Couldn't get size: 0x8000000000000015 > integrity: MODSIGN: Couldn't get UEFI db list > efi: EFI Runtime Services are disabled! > integrity: Couldn't get size: 0x8000000000000015 > integrity: Couldn't get UEFI dbx list > integrity: Couldn't get size: 0x8000000000000015 > integrity: Couldn't get mokx list > integrity: Couldn't get size: 0x80000000 > > This patch prevents querying of these UEFI variables, since these Macs > seem to use a non-standard EFI hardware > > Cc: stable@vger.kernel.org > Signed-off-by: Aditya Garg <gargaditya08@live.com> > --- > v2 :- Reduce code size of the table. NAK. As Matthew pointed out, other reads of the same variables may still trigger the same issue. > .../platform_certs/keyring_handler.h | 8 ++++ > security/integrity/platform_certs/load_uefi.c | 48 +++++++++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h > index 2462bfa08..cd06bd607 100644 > --- a/security/integrity/platform_certs/keyring_handler.h > +++ b/security/integrity/platform_certs/keyring_handler.h > @@ -30,3 +30,11 @@ efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type); > efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type); > > #endif > + > +#ifndef UEFI_QUIRK_SKIP_CERT > +#define UEFI_QUIRK_SKIP_CERT(vendor, product) \ > + .matches = { \ > + DMI_MATCH(DMI_BOARD_VENDOR, vendor), \ > + DMI_MATCH(DMI_PRODUCT_NAME, product), \ > + }, > +#endif > diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c > index 08b6d12f9..f246c8732 100644 > --- a/security/integrity/platform_certs/load_uefi.c > +++ b/security/integrity/platform_certs/load_uefi.c > @@ -3,6 +3,7 @@ > #include <linux/kernel.h> > #include <linux/sched.h> > #include <linux/cred.h> > +#include <linux/dmi.h> > #include <linux/err.h> > #include <linux/efi.h> > #include <linux/slab.h> > @@ -12,6 +13,32 @@ > #include "../integrity.h" > #include "keyring_handler.h" > > +/* Apple Macs with T2 Security chip don't support these UEFI variables. > + * The T2 chip manages the Secure Boot and does not allow Linux to boot > + * if it is turned on. If turned off, an attempt to get certificates > + * causes a crash, so we simply return 0 for them in each function. > + */ > + > +static const struct dmi_system_id uefi_skip_cert[] = { > + > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,1" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,2" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,3" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,4" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,1" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,2" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,3" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,4" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir8,1" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir8,2" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir9,1" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacMini8,1" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacPro7,1" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "iMac20,1" }, > + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "iMac20,2" }, > + { } > +}; > + > /* > * Look to see if a UEFI variable called MokIgnoreDB exists and return true if > * it does. > @@ -21,12 +48,18 @@ > * is set, we should ignore the db variable also and the true return indicates > * this. > */ > + > static __init bool uefi_check_ignore_db(void) > { > efi_status_t status; > unsigned int db = 0; > unsigned long size = sizeof(db); > efi_guid_t guid = EFI_SHIM_LOCK_GUID; > + const struct dmi_system_id *dmi_id; > + > + dmi_id = dmi_first_match(uefi_skip_cert); > + if (dmi_id) > + return 0; > > status = efi.get_variable(L"MokIgnoreDB", &guid, NULL, &size, &db); > return status == EFI_SUCCESS; > @@ -41,6 +74,11 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid, > unsigned long lsize = 4; > unsigned long tmpdb[4]; > void *db; > + const struct dmi_system_id *dmi_id; > + > + dmi_id = dmi_first_match(uefi_skip_cert); > + if (dmi_id) > + return 0; > > *status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb); > if (*status == EFI_NOT_FOUND) > @@ -85,6 +123,11 @@ static int __init load_moklist_certs(void) > unsigned long moksize; > efi_status_t status; > int rc; > + const struct dmi_system_id *dmi_id; > + > + dmi_id = dmi_first_match(uefi_skip_cert); > + if (dmi_id) > + return 0; > > /* First try to load certs from the EFI MOKvar config table. > * It's not an error if the MOKvar config table doesn't exist > @@ -138,6 +181,11 @@ static int __init load_uefi_certs(void) > unsigned long dbsize = 0, dbxsize = 0, mokxsize = 0; > efi_status_t status; > int rc = 0; > + const struct dmi_system_id *dmi_id; > + > + dmi_id = dmi_first_match(uefi_skip_cert); > + if (dmi_id) > + return 0; > > if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) > return false; > -- > 2.25.1 > >
> > NAK. As Matthew pointed out, other reads of the same variables may > still trigger the same issue. > Ohk. I just sent a v2 in order to fix the issue point out by David. We can go ahead with Matthew’s point of view, which makes sense, in further versions.
diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h index 2462bfa08..cd06bd607 100644 --- a/security/integrity/platform_certs/keyring_handler.h +++ b/security/integrity/platform_certs/keyring_handler.h @@ -30,3 +30,11 @@ efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type); efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type); #endif + +#ifndef UEFI_QUIRK_SKIP_CERT +#define UEFI_QUIRK_SKIP_CERT(vendor, product) \ + .matches = { \ + DMI_MATCH(DMI_BOARD_VENDOR, vendor), \ + DMI_MATCH(DMI_PRODUCT_NAME, product), \ + }, +#endif diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c index 08b6d12f9..f246c8732 100644 --- a/security/integrity/platform_certs/load_uefi.c +++ b/security/integrity/platform_certs/load_uefi.c @@ -3,6 +3,7 @@ #include <linux/kernel.h> #include <linux/sched.h> #include <linux/cred.h> +#include <linux/dmi.h> #include <linux/err.h> #include <linux/efi.h> #include <linux/slab.h> @@ -12,6 +13,32 @@ #include "../integrity.h" #include "keyring_handler.h" +/* Apple Macs with T2 Security chip don't support these UEFI variables. + * The T2 chip manages the Secure Boot and does not allow Linux to boot + * if it is turned on. If turned off, an attempt to get certificates + * causes a crash, so we simply return 0 for them in each function. + */ + +static const struct dmi_system_id uefi_skip_cert[] = { + + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,1" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,2" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,3" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,4" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,1" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,2" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,3" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,4" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir8,1" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir8,2" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir9,1" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacMini8,1" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacPro7,1" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "iMac20,1" }, + { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "iMac20,2" }, + { } +}; + /* * Look to see if a UEFI variable called MokIgnoreDB exists and return true if * it does. @@ -21,12 +48,18 @@ * is set, we should ignore the db variable also and the true return indicates * this. */ + static __init bool uefi_check_ignore_db(void) { efi_status_t status; unsigned int db = 0; unsigned long size = sizeof(db); efi_guid_t guid = EFI_SHIM_LOCK_GUID; + const struct dmi_system_id *dmi_id; + + dmi_id = dmi_first_match(uefi_skip_cert); + if (dmi_id) + return 0; status = efi.get_variable(L"MokIgnoreDB", &guid, NULL, &size, &db); return status == EFI_SUCCESS; @@ -41,6 +74,11 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid, unsigned long lsize = 4; unsigned long tmpdb[4]; void *db; + const struct dmi_system_id *dmi_id; + + dmi_id = dmi_first_match(uefi_skip_cert); + if (dmi_id) + return 0; *status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb); if (*status == EFI_NOT_FOUND) @@ -85,6 +123,11 @@ static int __init load_moklist_certs(void) unsigned long moksize; efi_status_t status; int rc; + const struct dmi_system_id *dmi_id; + + dmi_id = dmi_first_match(uefi_skip_cert); + if (dmi_id) + return 0; /* First try to load certs from the EFI MOKvar config table. * It's not an error if the MOKvar config table doesn't exist @@ -138,6 +181,11 @@ static int __init load_uefi_certs(void) unsigned long dbsize = 0, dbxsize = 0, mokxsize = 0; efi_status_t status; int rc = 0; + const struct dmi_system_id *dmi_id; + + dmi_id = dmi_first_match(uefi_skip_cert); + if (dmi_id) + return 0; if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) return false;