All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
@ 2022-04-10 10:49 Aditya Garg
  2022-04-12 12:32 ` Mimi Zohar
  2022-04-12 16:40 ` [PATCH v4] " Aditya Garg
  0 siblings, 2 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-10 10:49 UTC (permalink / raw)
  To: jarkko, zohar, dmitry.kasatkin, jmorris, serge, ast, daniel,
	andrii, kafai, songliubraving, yhs, john.fastabend, kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin

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.
V3 :- Close the brackets which were left open by mistake.
 .../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



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-10 10:49 [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs Aditya Garg
@ 2022-04-12 12:32 ` Mimi Zohar
  2022-04-12 14:13   ` Aditya Garg
  2022-04-12 16:40 ` [PATCH v4] " Aditya Garg
  1 sibling, 1 reply; 21+ messages in thread
From: Mimi Zohar @ 2022-04-12 12:32 UTC (permalink / raw)
  To: Aditya Garg, jarkko, dmitry.kasatkin, jmorris, serge, ast,
	daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin

On Sun, 2022-04-10 at 10:49 +0000, Aditya Garg 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.

The end result might be "disable secure boot for Linux", but that isn't
what the code is actually doing.  As a result of not being able to read
or load certificates, secure boot cannot be enabled.  Please be more
precise.

> 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.
> V3 :- Close the brackets which were left open by mistake.
>  .../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.

Please refer to Documentation/process/coding-style.rst for the format
of multi-line comments.

> + * 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.
> + */
> +

No need for a blank line here.

> +static const struct dmi_system_id uefi_skip_cert[] = {
> +
No need for a blank here either.

> +	{ 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.
>   */
> +
Or here

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

The function returns a bool.  Return either "true" or "false".

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

The return value here should be NULL.

>  
>  	*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;

uefi_check_ignore_db(), get_cert_list(), uefi_check_ignore_db(), and
/load_moklist_certs() are all defined all static and are gated here by
this dmi_first_match().  There's probably no need for any of the other
calls to dmi_first_match().

Like in all the other cases, there should be some sort of message.  At
minimum, there should be a pr_info().

>  
>  	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>  		return false;

thanks,

Mimi


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 12:32 ` Mimi Zohar
@ 2022-04-12 14:13   ` Aditya Garg
  2022-04-12 15:13     ` Mimi Zohar
  0 siblings, 1 reply; 21+ messages in thread
From: Aditya Garg @ 2022-04-12 14:13 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin



> On 12-Apr-2022, at 6:02 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Sun, 2022-04-10 at 10:49 +0000, Aditya Garg 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.
> 
> The end result might be "disable secure boot for Linux", but that isn't
> what the code is actually doing. As a result of not being able to read
> or load certificates, secure boot cannot be enabled. Please be more
> precise.
I’ll fix this
> 
>> 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.
>> V3 :- Close the brackets which were left open by mistake.
>> .../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.
> 
> Please refer to Documentation/process/coding-style.rst for the format
> of multi-line comments.
Done
> 
>> + * 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.
>> + */
>> +
> 
> No need for a blank line here.
All blanks removed
> 
>> +static const struct dmi_system_id uefi_skip_cert[] = {
>> +
> No need for a blank here either.
> 
>> +	{ 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.
>> */
>> +
> Or here
> 
>> 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;
> 
> The function returns a bool. Return either "true" or "false".
> 
>> 
>> 	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;
> 
> The return value here should be NULL.
> 
>> 
>> 	*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;
> 
> uefi_check_ignore_db(), get_cert_list(), uefi_check_ignore_db(), and
> /load_moklist_certs() are all defined all static and are gated here by
> this dmi_first_match(). There's probably no need for any of the other
> calls to dmi_first_match().
I couldn’t get you here. Could you elaborate?
> 
> Like in all the other cases, there should be some sort of message. At
> minimum, there should be a pr_info().
> 
>> 
>> 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>> 		return false;
> 
> thanks,
> 
> Mimi


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 14:13   ` Aditya Garg
@ 2022-04-12 15:13     ` Mimi Zohar
  2022-04-12 15:40       ` Aditya Garg
  2022-04-12 16:38       ` Aditya Garg
  0 siblings, 2 replies; 21+ messages in thread
From: Mimi Zohar @ 2022-04-12 15:13 UTC (permalink / raw)
  To: Aditya Garg
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin

On Tue, 2022-04-12 at 14:13 +0000, Aditya Garg wrote:
> >> @@ -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;
> > 
> > uefi_check_ignore_db(), get_cert_list(), uefi_check_ignore_db(), and
> > /load_moklist_certs() are all defined all static and are gated here by
> > this dmi_first_match(). There's probably no need for any of the other
> > calls to dmi_first_match().
> I couldn’t get you here. Could you elaborate?

dmi_first_match() is called here at the beginning of load_uefi_certs().
Only if it succeeds would uefi_check_ignore_db(), get_cert_list(),
uefi_check_ignore_db(), or
load_moklist_certs() be called.  Is there a need for adding a call to
dmi_first_match() in any of these other functions?

thanks,

Mimi

> > 
> > Like in all the other cases, there should be some sort of message. At
> > minimum, there should be a pr_info().
> > 
> >> 
> >>      if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
> >>              return false;
> > 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 15:13     ` Mimi Zohar
@ 2022-04-12 15:40       ` Aditya Garg
  2022-04-12 16:38       ` Aditya Garg
  1 sibling, 0 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-12 15:40 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin


> dmi_first_match() is called here at the beginning of load_uefi_certs().
> Only if it succeeds would uefi_check_ignore_db(), get_cert_list(),
> uefi_check_ignore_db(), or
> load_moklist_certs() be called.  Is there a need for adding a call to
> dmi_first_match() in any of these other functions?
I’ll test this out.
> 
> thanks,
> 
> Mimi
> 
>>> 
>>> Like in all the other cases, there should be some sort of message. At
>>> minimum, there should be a pr_info().
>>> 
>>>> 
>>>>     if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>>>>             return false;
>>> 
> 
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 15:13     ` Mimi Zohar
  2022-04-12 15:40       ` Aditya Garg
@ 2022-04-12 16:38       ` Aditya Garg
  1 sibling, 0 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-12 16:38 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin


> 
> dmi_first_match() is called here at the beginning of load_uefi_certs().
> Only if it succeeds would uefi_check_ignore_db(), get_cert_list(),
> uefi_check_ignore_db(), or
> load_moklist_certs() be called.  Is there a need for adding a call to
> dmi_first_match() in any of these other functions?

Well, there actually isn’t a need to call dmi_first_match() in other functions.

Sending a v4 with the changes

Thanks
Aditya

> 
> thanks,
> 
> Mimi
> 
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-10 10:49 [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs Aditya Garg
  2022-04-12 12:32 ` Mimi Zohar
@ 2022-04-12 16:40 ` Aditya Garg
  2022-04-12 16:44   ` [PATCH v4 RESEND] " Aditya Garg
  1 sibling, 1 reply; 21+ messages in thread
From: Aditya Garg @ 2022-04-12 16:40 UTC (permalink / raw)
  To: jarkko, zohar, dmitry.kasatkin, jmorris, serge, ast, daniel,
	andrii, kafai, songliubraving, yhs, john.fastabend, kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin

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. Moreover, loading
UEFI Secure Boot certificates is not supported on these machines on Linux.
An attempt to do so causes a crash with the following logs :-

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

As a result of not being able to read or load certificates, secure boot
cannot be enabled. 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.
v3 :- Close the brackets which were left open by mistake.
v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
 .../platform_certs/keyring_handler.h          |  8 +++++
 security/integrity/platform_certs/load_uefi.c | 35 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 284558f30..212d894a8 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..c3393b2b1 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,33 @@
 #include "../integrity.h"
 #include "keyring_handler.h"
 
+/*
+ * Apple Macs with T2 Security chip seem to be using a non standard
+ * implementation of Secure Boot. For Linux to run on these machines
+ * Secure Boot needs to be turned off, since the T2 Chip manages
+ * Secure Boot and doesn't allow OS other than macOS or Windows to
+ * boot. If turned off, an attempt to get certificates causes a crash,
+ * so we simply prevent doing the same.
+ */
+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.
@@ -138,6 +166,13 @@ 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) {
+		pr_err("Getting UEFI Secure Boot Certs is not supported on T2 Macs.\n");
+		return false;
+	}
 
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 16:40 ` [PATCH v4] " Aditya Garg
@ 2022-04-12 16:44   ` Aditya Garg
  2022-04-12 17:16     ` Mimi Zohar
  2022-04-13 14:04     ` [PATCH v5] " Aditya Garg
  0 siblings, 2 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-12 16:44 UTC (permalink / raw)
  To: jarkko, zohar, dmitry.kasatkin, jmorris, serge, ast, daniel,
	andrii, kafai, songliubraving, yhs, john.fastabend, kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

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. Moreover, loading
UEFI Secure Boot certificates is not supported on these machines on Linux.
An attempt to do so causes a crash with the following logs :-

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

As a result of not being able to read or load certificates, secure boot
cannot be enabled. 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.
v3 :- Close the brackets which were left open by mistake.
v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
v4 RESEND :- Add stable to cc
 .../platform_certs/keyring_handler.h          |  8 +++++
 security/integrity/platform_certs/load_uefi.c | 35 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 284558f30..212d894a8 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..c3393b2b1 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,33 @@
 #include "../integrity.h"
 #include "keyring_handler.h"
 
+/*
+ * Apple Macs with T2 Security chip seem to be using a non standard
+ * implementation of Secure Boot. For Linux to run on these machines
+ * Secure Boot needs to be turned off, since the T2 Chip manages
+ * Secure Boot and doesn't allow OS other than macOS or Windows to
+ * boot. If turned off, an attempt to get certificates causes a crash,
+ * so we simply prevent doing the same.
+ */
+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.
@@ -138,6 +166,13 @@ 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) {
+		pr_err("Getting UEFI Secure Boot Certs is not supported on T2 Macs.\n");
+		return false;
+	}
 
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 16:44   ` [PATCH v4 RESEND] " Aditya Garg
@ 2022-04-12 17:16     ` Mimi Zohar
  2022-04-13 14:03       ` Aditya Garg
  2022-04-13 14:04     ` [PATCH v5] " Aditya Garg
  1 sibling, 1 reply; 21+ messages in thread
From: Mimi Zohar @ 2022-04-12 17:16 UTC (permalink / raw)
  To: Aditya Garg, jarkko, dmitry.kasatkin, jmorris, serge, ast,
	daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

On Tue, 2022-04-12 at 16:44 +0000, Aditya Garg 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. Moreover, loading
> UEFI Secure Boot certificates is not supported on these machines on Linux.
> An attempt to do so causes a crash with the following logs :-
> 
> 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
> 
> As a result of not being able to read or load certificates, secure boot
> cannot be enabled. 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.
> v3 :- Close the brackets which were left open by mistake.
> v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
> v4 RESEND :- Add stable to cc
>  .../platform_certs/keyring_handler.h          |  8 +++++
>  security/integrity/platform_certs/load_uefi.c | 35 +++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
> index 284558f30..212d894a8 100644
> --- a/security/integrity/platform_certs/keyring_handler.h
> +++ b/security/integrity/platform_certs/keyring_handler.h
> @@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..c3393b2b1 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,33 @@
>  #include "../integrity.h"
>  #include "keyring_handler.h"
>  
> +/*
> + * Apple Macs with T2 Security chip seem to be using a non standard
> + * implementation of Secure Boot. For Linux to run on these machines
> + * Secure Boot needs to be turned off, since the T2 Chip manages
> + * Secure Boot and doesn't allow OS other than macOS or Windows to
> + * boot. If turned off, an attempt to get certificates causes a crash,
> + * so we simply prevent doing the same.
> + */

Both the comment here and the patch description above still needs to be
improved.  Perhaps something along these lines.

Secure boot on Apple Macs with a T2 Security chip cannot read either
the EFI variables or the certificates stored in different db's (e.g.
db, dbx, MokListXRT).  Attempting to read them causes ...   

Avoid reading the EFI variables or the certificates stored in different
dbs.  As a result, without certificates secure boot signature
verification fails.

thanks,

Mimi


> +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.
> @@ -138,6 +166,13 @@ 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) {
> +		pr_err("Getting UEFI Secure Boot Certs is not supported on T2 Macs.\n");
> +		return false;
> +	}
>  
>  	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>  		return false;



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 17:16     ` Mimi Zohar
@ 2022-04-13 14:03       ` Aditya Garg
  0 siblings, 0 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-13 14:03 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

> 
> Both the comment here and the patch description above still needs to be
> improved. Perhaps something along these lines.
Checkout v5
> 
> Secure boot on Apple Macs with a T2 Security chip cannot read either
> the EFI variables or the certificates stored in different db's (e.g.
> db, dbx, MokListXRT). Attempting to read them causes ... 
> 
> Avoid reading the EFI variables or the certificates stored in different
> dbs. As a result, without certificates secure boot signature
> verification fails.
> 
> thanks,
> 
> Mimi
> 
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-12 16:44   ` [PATCH v4 RESEND] " Aditya Garg
  2022-04-12 17:16     ` Mimi Zohar
@ 2022-04-13 14:04     ` Aditya Garg
  2022-04-14 13:30       ` Mimi Zohar
  2022-04-15  6:19       ` [PATCH v6] " Aditya Garg
  1 sibling, 2 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-13 14:04 UTC (permalink / raw)
  To: jarkko, zohar, dmitry.kasatkin, jmorris, serge, ast, daniel,
	andrii, kafai, songliubraving, yhs, john.fastabend, kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

From: Aditya Garg <gargaditya08@live.com>

On Apple T2 Macs, when Linux reads the db and dbx efi variables to load
UEFI Secure Boot certificates, a page fault occurs in Apple firmware
code and EFI services are disabled with the following logs:

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 also occurs when some other variables are read (see examples below,
there are many more), but only when these variables are read at an early
stage like db and dbx are to load UEFI certs.

BridgeOSBootSessionUUID-4d1ede05-38c7-4a6a-9cc6-4bcca8b38c14
KEK-8be4df61-93ca-11d2-aa0d-00e098032b8c

On these Macs, we skip reading the EFI variables for the UEFI certificates.
As a result without certificates, secure boot signature verification fails.
As these Macs have a non-standard implementation of secure boot that only
uses Apple's and Microsoft's keys (users can't add their own), securely
booting Linux was never supported on this hardware, so skipping it
shouldn't cause a regression.

Cc: stable@vger.kernel.org
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
v2 :- Reduce code size of the table.
v3 :- Close the brackets which were left open by mistake.
v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
v4 RESEND :- Add stable to cc
v5 :- Rewrite the description
 .../platform_certs/keyring_handler.h          |  8 ++++
 security/integrity/platform_certs/load_uefi.c | 37 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 284558f30..212d894a8 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..bbddc7c7c 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,35 @@
 #include "../integrity.h"
 #include "keyring_handler.h"
 
+/*
+ * On T2 Macs reading the reading the db and dbx efi variables to load UEFI
+ * Secure Boot certificates causes occurrence of a page fault in Apple's
+ * firmware and a crash disabling EFI runtime services. The following quirk
+ * skips loading of these certificates.
+ *
+ * As these Macs have a non-standard implementation of secure boot that only uses
+ * Apple's and Microsoft's keys, booting Linux securely was never supported on
+ * this hardware, so these quirks shouldn't cause a regression.
+ */
+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.
@@ -138,6 +168,13 @@ 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) {
+		pr_err("Getting UEFI Secure Boot Certs is not supported on T2 Macs.\n");
+		return false;
+	}
 
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v5] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-13 14:04     ` [PATCH v5] " Aditya Garg
@ 2022-04-14 13:30       ` Mimi Zohar
  2022-04-15  6:17         ` Aditya Garg
  2022-04-15  6:19       ` [PATCH v6] " Aditya Garg
  1 sibling, 1 reply; 21+ messages in thread
From: Mimi Zohar @ 2022-04-14 13:30 UTC (permalink / raw)
  To: Aditya Garg, jarkko, dmitry.kasatkin, jmorris, serge, ast,
	daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

On Wed, 2022-04-13 at 14:04 +0000, Aditya Garg wrote:
> From: Aditya Garg <gargaditya08@live.com>
> 
> On Apple T2 Macs, when Linux reads the db and dbx efi variables to load
> UEFI Secure Boot certificates, a page fault occurs in Apple firmware
> code and EFI services are disabled with the following logs:
> 
> 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 also occurs when some other variables are read (see examples below,
> there are many more), but only when these variables are read at an early
> stage like db and dbx are to load UEFI certs.
> 
> BridgeOSBootSessionUUID-4d1ede05-38c7-4a6a-9cc6-4bcca8b38c14
> KEK-8be4df61-93ca-11d2-aa0d-00e098032b8c
> 
> On these Macs, we skip reading the EFI variables for the UEFI certificates.
> As a result without certificates, secure boot signature verification fails.
> As these Macs have a non-standard implementation of secure boot that only
> uses Apple's and Microsoft's keys (users can't add their own), securely
> booting Linux was never supported on this hardware, so skipping it
> shouldn't cause a regression.

Based on your explanation, there seems to be two issues - inability to
read EFI variables, "users can't add their own" keys.  Neither of which
mean "a non-standard implementation of secure boot".  Please fix the
"cause" and "affect" in the patch description and comments.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-14 13:30       ` Mimi Zohar
@ 2022-04-15  6:17         ` Aditya Garg
  0 siblings, 0 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-15  6:17 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable


> Based on your explanation, there seems to be two issues - inability to
> read EFI variables, "users can't add their own" keys. Neither of which
> mean "a non-standard implementation of secure boot". Please fix the
> "cause" and "affect" in the patch description and comments.

Sending a v6

Also, I guess I should just remove the secure boot bit, cause secure boot, though kinda related, doesn’t have much role here.

The “cause” is reading of specific UEFI variables, like db, dbx etc, and the “affect” being crashing of EFI Runtime Services.

The “fix”, simply prevent reading of these variables

The role of secure boot (Which I have removed in the description of v6, cause its not of much significance in regard to this patch) :-

Loading of these certificates is required to “boot securely”. By disabling loading of these certificates, we are technically preventing booting Linux “securely” on these machines. But, this shouldn’t be a matter to worry about. The reason being, Apple doesn’t allow anything other that macOS or Windows to boot if Secure Boot in turned on, on these Macs, making it impossible to boot Linux with secure boot on, unless Apple itself updates the firmware on the T2 Chip, to support Linux as well, which is highly unlikely.
> 
> thanks,
> 
> Mimi


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-13 14:04     ` [PATCH v5] " Aditya Garg
  2022-04-14 13:30       ` Mimi Zohar
@ 2022-04-15  6:19       ` Aditya Garg
  2022-04-15 16:26         ` Mimi Zohar
  2022-04-15 17:02         ` [PATCH v7] " Aditya Garg
  1 sibling, 2 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-15  6:19 UTC (permalink / raw)
  To: jarkko, zohar, dmitry.kasatkin, jmorris, serge, ast, daniel,
	andrii, kafai, songliubraving, yhs, john.fastabend, kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

From: Aditya Garg <gargaditya08@live.com>

On Apple T2 Macs, when Linux attempts to read the db and dbx efi variables
at early boot to load UEFI Secure Boot certificates, a page fault occurs
in Apple firmware code and EFI runtime services are disabled with the
following logs:

[Firmware Bug]: Page fault caused by firmware at PA: 0xffffb1edc0068000
WARNING: CPU: 3 PID: 104 at arch/x86/platform/efi/quirks.c:735 efi_crash_gracefully_on_page_fault+0x50/0xf0
(Removed some logs from here)
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 skips reading these UEFI variables and thus prevents the crash.

Cc: stable@vger.kernel.org
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
v2 :- Reduce code size of the table.
v3 :- Close the brackets which were left open by mistake.
v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
v4 RESEND :- Add stable to cc
v5 :- Rewrite the description
v6 :- Make description more clear
 .../platform_certs/keyring_handler.h          |  8 +++++
 security/integrity/platform_certs/load_uefi.c | 33 +++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 284558f30..212d894a8 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..1a7e7d597 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,31 @@
 #include "../integrity.h"
 #include "keyring_handler.h"
 
+/*
+ * On T2 Macs reading the reading the db and dbx efi variables to load UEFI
+ * Secure Boot certificates causes occurrence of a page fault in Apple's
+ * firmware and a crash disabling EFI runtime services. The following quirk
+ * skips reading these variables.
+ */
+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.
@@ -138,6 +164,13 @@ 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) {
+		pr_err("Getting UEFI Secure Boot Certs is not supported on T2 Macs.\n");
+		return false;
+	}
 
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v6] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-15  6:19       ` [PATCH v6] " Aditya Garg
@ 2022-04-15 16:26         ` Mimi Zohar
  2022-04-15 17:02           ` Aditya Garg
  2022-04-15 17:02         ` [PATCH v7] " Aditya Garg
  1 sibling, 1 reply; 21+ messages in thread
From: Mimi Zohar @ 2022-04-15 16:26 UTC (permalink / raw)
  To: Aditya Garg, jarkko, dmitry.kasatkin, jmorris, serge, ast,
	daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

On Fri, 2022-04-15 at 06:19 +0000, Aditya Garg wrote:
> From: Aditya Garg <gargaditya08@live.com>
> 
> On Apple T2 Macs, when Linux attempts to read the db and dbx efi variables
> at early boot to load UEFI Secure Boot certificates, a page fault occurs
> in Apple firmware code and EFI runtime services are disabled with the
> following logs:
> 
> [Firmware Bug]: Page fault caused by firmware at PA: 0xffffb1edc0068000
> WARNING: CPU: 3 PID: 104 at arch/x86/platform/efi/quirks.c:735 efi_crash_gracefully_on_page_fault+0x50/0xf0
> (Removed some logs from here)
> 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 skips reading these UEFI variables and thus prevents the crash.

Instead of "This patch skips reading" say "Avoid reading".

> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Aditya Garg <gargaditya08@live.com>

After making these minor changes, both above and below, 
	Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
> v2 :- Reduce code size of the table.
> v3 :- Close the brackets which were left open by mistake.
> v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
> v4 RESEND :- Add stable to cc
> v5 :- Rewrite the description
> v6 :- Make description more clear
>  .../platform_certs/keyring_handler.h          |  8 +++++
>  security/integrity/platform_certs/load_uefi.c | 33 +++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
> index 284558f30..212d894a8 100644
> --- a/security/integrity/platform_certs/keyring_handler.h
> +++ b/security/integrity/platform_certs/keyring_handler.h
> @@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..1a7e7d597 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,31 @@
>  #include "../integrity.h"
>  #include "keyring_handler.h"
>  
> +/*
> + * On T2 Macs reading the reading the db and dbx efi variables to load UEFI
> + * Secure Boot certificates causes occurrence of a page fault in Apple's
> + * firmware and a crash disabling EFI runtime services. The following quirk
> + * skips reading these variables.
> + */
> +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.
> @@ -138,6 +164,13 @@ 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) {
> +		pr_err("Getting UEFI Secure Boot Certs is not supported on T2 Macs.\n");

Replace "Getting" with "Reading".

thanks,

Mimi

> +		return false;
> +	}
>  
>  	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>  		return false;



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-15 16:26         ` Mimi Zohar
@ 2022-04-15 17:02           ` Aditya Garg
  0 siblings, 0 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-15 17:02 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable


> 
> After making these minor changes, both above and below, 
> 	Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> 

Sending a v7


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v7] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-15  6:19       ` [PATCH v6] " Aditya Garg
  2022-04-15 16:26         ` Mimi Zohar
@ 2022-04-15 17:02         ` Aditya Garg
  2022-04-22 17:39           ` [PATCH v7 RESEND] " Aditya Garg
  2022-05-13 15:24           ` [PATCH v7] " Mimi Zohar
  1 sibling, 2 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-15 17:02 UTC (permalink / raw)
  To: jarkko, zohar, dmitry.kasatkin, jmorris, serge, ast, daniel,
	andrii, kafai, songliubraving, yhs, john.fastabend, kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

From: Aditya Garg <gargaditya08@live.com>

On Apple T2 Macs, when Linux attempts to read the db and dbx efi variables
at early boot to load UEFI Secure Boot certificates, a page fault occurs
in Apple firmware code and EFI runtime services are disabled with the
following logs:

[Firmware Bug]: Page fault caused by firmware at PA: 0xffffb1edc0068000
WARNING: CPU: 3 PID: 104 at arch/x86/platform/efi/quirks.c:735 efi_crash_gracefully_on_page_fault+0x50/0xf0
(Removed some logs from here)
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

So we avoid reading these UEFI variables and thus prevent the crash.

Cc: stable@vger.kernel.org
Signed-off-by: Aditya Garg <gargaditya08@live.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
v2 :- Reduce code size of the table.
v3 :- Close the brackets which were left open by mistake.
v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
v4 RESEND :- Add stable to cc
v5 :- Rewrite the description
v6 :- Make description more clear
v7 :- Minor changes and add reviewed by
 .../platform_certs/keyring_handler.h          |  8 +++++
 security/integrity/platform_certs/load_uefi.c | 33 +++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 284558f30..212d894a8 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..1a7e7d597 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,31 @@
 #include "../integrity.h"
 #include "keyring_handler.h"
 
+/*
+ * On T2 Macs reading the db and dbx efi variables to load UEFI Secure Boot
+ * certificates causes occurrence of a page fault in Apple's firmware and
+ * a crash disabling EFI runtime services. The following quirk skips reading
+ * these variables.
+ */
+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.
@@ -138,6 +164,13 @@ 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) {
+		pr_err("Reading UEFI Secure Boot Certs is not supported on T2 Macs.\n");
+		return false;
+	}
 
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v7 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-15 17:02         ` [PATCH v7] " Aditya Garg
@ 2022-04-22 17:39           ` Aditya Garg
  2022-05-13 15:24           ` [PATCH v7] " Mimi Zohar
  1 sibling, 0 replies; 21+ messages in thread
From: Aditya Garg @ 2022-04-22 17:39 UTC (permalink / raw)
  To: jarkko, zohar, dmitry.kasatkin, jmorris, serge, ast, daniel,
	andrii, kafai, songliubraving, yhs, john.fastabend, kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

From: Aditya Garg <gargaditya08@live.com>

On Apple T2 Macs, when Linux attempts to read the db and dbx efi variables
at early boot to load UEFI Secure Boot certificates, a page fault occurs
in Apple firmware code and EFI runtime services are disabled with the
following logs:

[Firmware Bug]: Page fault caused by firmware at PA: 0xffffb1edc0068000
WARNING: CPU: 3 PID: 104 at arch/x86/platform/efi/quirks.c:735 efi_crash_gracefully_on_page_fault+0x50/0xf0
(Removed some logs from here)
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

So we avoid reading these UEFI variables and thus prevent the crash.

Cc: stable@vger.kernel.org
Signed-off-by: Aditya Garg <gargaditya08@live.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
v2 :- Reduce code size of the table.
v3 :- Close the brackets which were left open by mistake.
v4 :- Fix comment style issues, remove blank spaces and limit use of dmi_first_match()
v4 RESEND :- Add stable to cc
v5 :- Rewrite the description
v6 :- Make description more clear
v7 :- Minor changes and add reviewed by
 .../platform_certs/keyring_handler.h          |  8 +++++
 security/integrity/platform_certs/load_uefi.c | 33 +++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 284558f30..212d894a8 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -35,3 +35,11 @@ efi_element_handler_t get_handler_for_mok(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 5f45c3c07..1a7e7d597 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,31 @@
 #include "../integrity.h"
 #include "keyring_handler.h"
 
+/*
+ * On T2 Macs reading the db and dbx efi variables to load UEFI Secure Boot
+ * certificates causes occurrence of a page fault in Apple's firmware and
+ * a crash disabling EFI runtime services. The following quirk skips reading
+ * these variables.
+ */
+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.
@@ -138,6 +164,13 @@ 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) {
+		pr_err("Reading UEFI Secure Boot Certs is not supported on T2 Macs.\n");
+		return false;
+	}
 
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v7] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-04-15 17:02         ` [PATCH v7] " Aditya Garg
  2022-04-22 17:39           ` [PATCH v7 RESEND] " Aditya Garg
@ 2022-05-13 15:24           ` Mimi Zohar
  2022-05-13 18:31             ` Aditya Garg
  1 sibling, 1 reply; 21+ messages in thread
From: Mimi Zohar @ 2022-05-13 15:24 UTC (permalink / raw)
  To: Aditya Garg, jarkko, dmitry.kasatkin, jmorris, serge, ast,
	daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

Hi Aditya,

On Fri, 2022-04-15 at 17:02 +0000, Aditya Garg wrote:
> From: Aditya Garg <gargaditya08@live.com>
> 
> On Apple T2 Macs, when Linux attempts to read the db and dbx efi variables
> at early boot to load UEFI Secure Boot certificates, a page fault occurs
> in Apple firmware code and EFI runtime services are disabled with the
> following logs:

Are there directions for installing Linux on a Mac with Apple firmware
code?  Are you dual booting Linux and Mac, or just Linux?  While in
secure boot mode, without being able to read the keys to verify the
kernel image signature, the signature verification should fail.

Has anyone else tested this patch?

thanks,

Mimi



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v7] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-05-13 15:24           ` [PATCH v7] " Mimi Zohar
@ 2022-05-13 18:31             ` Aditya Garg
  2022-05-15 12:41               ` Mimi Zohar
  0 siblings, 1 reply; 21+ messages in thread
From: Aditya Garg @ 2022-05-13 18:31 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable


> Are there directions for installing Linux on a Mac with Apple firmware
> code?  

Well, directions of installing Linux on an Intel based Mac, which includes the T2 Macs is the same as on a normal PC.

Though, in case of T2 Macs, we for now need to use customised ISOs, since some drivers and patches to support T2 Macs are yet to be upstreamed.

An example of installing Ubuntu can be read here on https://wiki.t2linux.org/distributions/ubuntu/installation/

Talking about the official ISOs, for many distros, since CONFIG_LOAD_UEFI_KEYS is not enabled in their kernel config, we can install Linux using them, but they still lack many drivers required, since they are yet to be upstreamed. So the installation doesn’t work efficiently and we have to manually install custom kernels having those patches.

In some distros like Ubuntu, they have CONFIG_LOAD_UEFI_KEYS enabled in their kernel config. In this case the crash as mentioned in the patch description occurs and EFI Runtime Services get disabled. Since installing GRUB requires access to NVRAM, the installation fails with official ISOs in this case. Thus, a custom ISO, with this patch incorporated in being used for now for users interested in Ubuntu on T2 Macs.

> Are you dual booting Linux and Mac, or just Linux?

I don’t think it actually matters, though in most of the cases, we dual boot macOS and Linux, but I do have seen cases who wipe out their macOS completely. But this doesn't affect the Secure Boot policy of these machines.

>  While in
> secure boot mode, without being able to read the keys to verify the
> kernel image signature, the signature verification should fail.

If I enable secure boot in the BIOS settings (macOS Recovery), Apple’s firmware won't allow even the boot loader like GRUB, rEFInd to boot. It shall only allow Windows and macOS to Boot. You could see https://support.apple.com/en-in/HT208198 for more details.

> 
> Has anyone else tested this patch?

I work as a maintainer for Ubuntu for T2 Linux community and I have this patch incorporated in the kernels used for Ubuntu ISOs customised for T2 Macs, and thus have many users who have used the ISO and have a successful installation. Thus, there are many users who have tested this patch and are actually using it right now.
We also need the have the NVRAM writes enabled so as to unlock the iGPU in Macs with both Intel and AMD GPU, and with this patch, we have been successfully able to unlock it,

I hope I could answer your questions

Regards
Aditya

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v7] efi: Do not import certificates from UEFI Secure Boot for T2 Macs
  2022-05-13 18:31             ` Aditya Garg
@ 2022-05-15 12:41               ` Mimi Zohar
  0 siblings, 0 replies; 21+ messages in thread
From: Mimi Zohar @ 2022-05-15 12:41 UTC (permalink / raw)
  To: Aditya Garg
  Cc: jarkko, dmitry.kasatkin, jmorris, serge, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	linux-integrity, keyrings, linux-security-module, linux-kernel,
	netdev, bpf, Orlando Chamberlain, admin, stable

On Fri, 2022-05-13 at 18:31 +0000, Aditya Garg wrote:
> > Are there directions for installing Linux on a Mac with Apple firmware
> > code?  
> 
> Well, directions of installing Linux on an Intel based Mac, which
> includes the T2 Macs is the same as on a normal PC.
> 
> Though, in case of T2 Macs, we for now need to use customised ISOs,
> since some drivers and patches to support T2 Macs are yet to be
> upstreamed.
> 
> An example of installing Ubuntu can be read here on 
> https://wiki.t2linux.org/distributions/ubuntu/installation/
> 
> Talking about the official ISOs, for many distros, since
> CONFIG_LOAD_UEFI_KEYS is not enabled in their kernel config, we can
> install Linux using them, but they still lack many drivers required,
> since they are yet to be upstreamed. So the installation doesn’t work
> efficiently and we have to manually install custom kernels having
> those patches.
> 
> In some distros like Ubuntu, they have CONFIG_LOAD_UEFI_KEYS enabled
> in their kernel config. In this case the crash as mentioned in the
> patch description occurs and EFI Runtime Services get disabled. Since
> installing GRUB requires access to NVRAM, the installation fails with
> official ISOs in this case. Thus, a custom ISO, with this patch
> incorporated in being used for now for users interested in Ubuntu on
> T2 Macs.
> 
> > Are you dual booting Linux and Mac, or just Linux?
> 
> I don’t think it actually matters, though in most of the cases, we
> dual boot macOS and Linux, but I do have seen cases who wipe out
> their macOS completely. But this doesn't affect the Secure Boot
> policy of these machines.
> 
> >  While in
> > secure boot mode, without being able to read the keys to verify the
> > kernel image signature, the signature verification should fail.
> 
> If I enable secure boot in the BIOS settings (macOS Recovery),
> Apple’s firmware won't allow even the boot loader like GRUB, rEFInd
> to boot. It shall only allow Windows and macOS to Boot. You could see
> https://support.apple.com/en-in/HT208198 for more details.
> 
> > 
> > Has anyone else tested this patch?
> 
> I work as a maintainer for Ubuntu for T2 Linux community and I have
> this patch incorporated in the kernels used for Ubuntu ISOs
> customised for T2 Macs, and thus have many users who have used the
> ISO and have a successful installation. Thus, there are many users
> who have tested this patch and are actually using it right now.
> We also need the have the NVRAM writes enabled so as to unlock the
> iGPU in Macs with both Intel and AMD GPU, and with this patch, we
> have been successfully able to unlock it,
> 
> I hope I could answer your questions

Yes, thank you.   Based on the link above and 
https://wiki.t2linux.org/guides/kernel/, I was able boot a kernel
with/without this patch.

The patch is now queued in the next-integrity-testing branch.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-05-15 12:43 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-10 10:49 [PATCH v3 RESEND] efi: Do not import certificates from UEFI Secure Boot for T2 Macs Aditya Garg
2022-04-12 12:32 ` Mimi Zohar
2022-04-12 14:13   ` Aditya Garg
2022-04-12 15:13     ` Mimi Zohar
2022-04-12 15:40       ` Aditya Garg
2022-04-12 16:38       ` Aditya Garg
2022-04-12 16:40 ` [PATCH v4] " Aditya Garg
2022-04-12 16:44   ` [PATCH v4 RESEND] " Aditya Garg
2022-04-12 17:16     ` Mimi Zohar
2022-04-13 14:03       ` Aditya Garg
2022-04-13 14:04     ` [PATCH v5] " Aditya Garg
2022-04-14 13:30       ` Mimi Zohar
2022-04-15  6:17         ` Aditya Garg
2022-04-15  6:19       ` [PATCH v6] " Aditya Garg
2022-04-15 16:26         ` Mimi Zohar
2022-04-15 17:02           ` Aditya Garg
2022-04-15 17:02         ` [PATCH v7] " Aditya Garg
2022-04-22 17:39           ` [PATCH v7 RESEND] " Aditya Garg
2022-05-13 15:24           ` [PATCH v7] " Mimi Zohar
2022-05-13 18:31             ` Aditya Garg
2022-05-15 12:41               ` Mimi Zohar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.