Linux-EFI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/6] add ima_arch support for ARM64
@ 2020-09-04  7:28 Chester Lin
  2020-09-04  7:29 ` [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params Chester Lin
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Chester Lin @ 2020-09-04  7:28 UTC (permalink / raw)
  To: ardb, catalin.marinas, will, zohar, dmitry.kasatkin, corbet,
	mark.rutland, vincenzo.frascino, samitolvanen, masahiroy, mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee, clin

Add IMA arch dependent support for ARM64. Some IMA functions can check
arch-specific status before running. For example, the ima_load_data
function or the boot param "ima_appraise=" should not be executed when
UEFI secure boot is enabled. We want to fill the gap in order to complete
the IMA support on ARM64.

Chester Lin (6):
  efistub: pass uefi secureboot flag via fdt params
  efi/arm: a helper to parse secure boot param in fdt params
  efi: add secure boot flag
  efi/arm: check secure boot status in efi init
  arm64/ima: add ima arch support
  docs/arm: add the description of uefi-secure-boot param

 Documentation/arm/uefi.rst         |  2 ++
 arch/arm64/Kconfig                 |  1 +
 arch/arm64/kernel/Makefile         |  2 ++
 arch/arm64/kernel/ima_arch.c       | 37 ++++++++++++++++++++++++++++
 drivers/firmware/efi/arm-init.c    |  3 +++
 drivers/firmware/efi/fdtparams.c   | 23 ++++++++++++++++++
 drivers/firmware/efi/libstub/fdt.c | 39 +++++++++++++++++++++++++++++-
 include/linux/efi.h                |  2 ++
 8 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/ima_arch.c

-- 
2.26.1


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

* [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params
  2020-09-04  7:28 [PATCH 0/6] add ima_arch support for ARM64 Chester Lin
@ 2020-09-04  7:29 ` Chester Lin
  2020-09-11 15:01   ` Ard Biesheuvel
  2020-09-04  7:29 ` [PATCH 2/6] efi/arm: a helper to parse secure boot param in " Chester Lin
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Chester Lin @ 2020-09-04  7:29 UTC (permalink / raw)
  To: ardb, catalin.marinas, will, zohar, dmitry.kasatkin, corbet,
	mark.rutland, vincenzo.frascino, samitolvanen, masahiroy, mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee, clin

Add a new UEFI parameter: "linux,uefi-secure-boot" in fdt boot params
as other architectures have done in their own boot data. For example,
the boot_params->secure_boot in x86.

Signed-off-by: Chester Lin <clin@suse.com>
---
 drivers/firmware/efi/libstub/fdt.c | 39 +++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 11ecf3c4640e..c9a341e4715f 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -136,6 +136,10 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
 	if (status)
 		goto fdt_set_fail;
 
+	status = fdt_setprop_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
+	if (status)
+		goto fdt_set_fail;
+
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
 		efi_status_t efi_status;
 
@@ -199,6 +203,24 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
 	return EFI_SUCCESS;
 }
 
+static efi_status_t update_fdt_secboot(void *fdt, u32 secboot)
+{
+	int node = fdt_path_offset(fdt, "/chosen");
+	u32 fdt_val32;
+	int err;
+
+	if (node < 0)
+		return EFI_LOAD_ERROR;
+
+	fdt_val32 = cpu_to_fdt32(secboot);
+
+	err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
+	if (err)
+		return EFI_LOAD_ERROR;
+
+	return EFI_SUCCESS;
+}
+
 struct exit_boot_struct {
 	efi_memory_desc_t	*runtime_map;
 	int			*runtime_entry_count;
@@ -208,6 +230,9 @@ struct exit_boot_struct {
 static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
 				   void *priv)
 {
+	efi_status_t status;
+	enum efi_secureboot_mode secboot_status;
+	u32 secboot_var = 0;
 	struct exit_boot_struct *p = priv;
 	/*
 	 * Update the memory map with virtual addresses. The function will also
@@ -217,7 +242,19 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
 	efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
 			p->runtime_map, p->runtime_entry_count);
 
-	return update_fdt_memmap(p->new_fdt_addr, map);
+	status = update_fdt_memmap(p->new_fdt_addr, map);
+
+	if (status != EFI_SUCCESS)
+		return status;
+
+	secboot_status = efi_get_secureboot();
+
+	if (secboot_status == efi_secureboot_mode_enabled)
+		secboot_var = 1;
+
+	status = update_fdt_secboot(p->new_fdt_addr, secboot_var);
+
+	return status;
 }
 
 #ifndef MAX_FDT_SIZE
-- 
2.26.1


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

* [PATCH 2/6] efi/arm: a helper to parse secure boot param in fdt params
  2020-09-04  7:28 [PATCH 0/6] add ima_arch support for ARM64 Chester Lin
  2020-09-04  7:29 ` [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params Chester Lin
@ 2020-09-04  7:29 ` Chester Lin
  2020-09-04  7:29 ` [PATCH 3/6] efi: add secure boot flag Chester Lin
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Chester Lin @ 2020-09-04  7:29 UTC (permalink / raw)
  To: ardb, catalin.marinas, will, zohar, dmitry.kasatkin, corbet,
	mark.rutland, vincenzo.frascino, samitolvanen, masahiroy, mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee, clin

Add a helper to query the UEFI secureboot param from the chosen node in
FDT.

Signed-off-by: Chester Lin <clin@suse.com>
---
 drivers/firmware/efi/fdtparams.c | 23 +++++++++++++++++++++++
 include/linux/efi.h              |  1 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
index bb042ab7c2be..d58ec4119bcf 100644
--- a/drivers/firmware/efi/fdtparams.c
+++ b/drivers/firmware/efi/fdtparams.c
@@ -124,3 +124,26 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
 	pr_info("UEFI not found.\n");
 	return 0;
 }
+
+bool __init efi_secureboot_enabled_in_fdt(void)
+{
+	const void *fdt = initial_boot_params;
+	int node;
+	u32 secboot;
+
+
+	node = fdt_path_offset(fdt, "/chosen");
+
+	if (node < 0) {
+		pr_err("chosen node not found.\n");
+		return false;
+	}
+
+	if (!efi_get_fdt_prop(fdt, node, "linux,uefi-secure-boot",
+			      "SECURE BOOT", &secboot, sizeof(secboot))) {
+		if (secboot)
+			return true;
+	}
+
+	return false;
+}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 73db1ae04cef..315126b2f5e9 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -663,6 +663,7 @@ extern int efi_mem_reserve_persistent(phys_addr_t addr, u64 size);
 extern void efi_initialize_iomem_resources(struct resource *code_resource,
 		struct resource *data_resource, struct resource *bss_resource);
 extern u64 efi_get_fdt_params(struct efi_memory_map_data *data);
+extern bool __init efi_secureboot_enabled_in_fdt(void);
 extern struct kobject *efi_kobj;
 
 extern int efi_reboot_quirk_mode;
-- 
2.26.1


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

* [PATCH 3/6] efi: add secure boot flag
  2020-09-04  7:28 [PATCH 0/6] add ima_arch support for ARM64 Chester Lin
  2020-09-04  7:29 ` [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params Chester Lin
  2020-09-04  7:29 ` [PATCH 2/6] efi/arm: a helper to parse secure boot param in " Chester Lin
@ 2020-09-04  7:29 ` Chester Lin
  2020-09-04  7:29 ` [PATCH 4/6] efi/arm: check secure boot status in efi init Chester Lin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Chester Lin @ 2020-09-04  7:29 UTC (permalink / raw)
  To: ardb, catalin.marinas, will, zohar, dmitry.kasatkin, corbet,
	mark.rutland, vincenzo.frascino, samitolvanen, masahiroy, mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee, clin

Add a new EFI flag to indicate whether secure boot is enabled by UEFI
firmware or not.

Signed-off-by: Chester Lin <clin@suse.com>
---
 include/linux/efi.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 315126b2f5e9..82a19bb0237a 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -784,6 +784,7 @@ extern int __init efi_setup_pcdp_console(char *);
 #define EFI_MEM_ATTR		10	/* Did firmware publish an EFI_MEMORY_ATTRIBUTES table? */
 #define EFI_MEM_NO_SOFT_RESERVE	11	/* Is the kernel configured to ignore soft reservations? */
 #define EFI_PRESERVE_BS_REGIONS	12	/* Are EFI boot-services memory segments available? */
+#define EFI_SECURE_BOOT		13	/* Is EFI secure-boot enabled? */
 
 #ifdef CONFIG_EFI
 /*
-- 
2.26.1


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

* [PATCH 4/6] efi/arm: check secure boot status in efi init
  2020-09-04  7:28 [PATCH 0/6] add ima_arch support for ARM64 Chester Lin
                   ` (2 preceding siblings ...)
  2020-09-04  7:29 ` [PATCH 3/6] efi: add secure boot flag Chester Lin
@ 2020-09-04  7:29 ` Chester Lin
  2020-09-04  7:29 ` [PATCH 5/6] arm64/ima: add ima arch support Chester Lin
  2020-09-04  7:29 ` [PATCH 6/6] docs/arm: add the description of uefi-secure-boot param Chester Lin
  5 siblings, 0 replies; 12+ messages in thread
From: Chester Lin @ 2020-09-04  7:29 UTC (permalink / raw)
  To: ardb, catalin.marinas, will, zohar, dmitry.kasatkin, corbet,
	mark.rutland, vincenzo.frascino, samitolvanen, masahiroy, mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee, clin

set EFI_SECURE_BOOT flag when UEFI secure boot is eanbled on ARM.

Signed-off-by: Chester Lin <clin@suse.com>
---
 drivers/firmware/efi/arm-init.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 71c445d20258..70f2eaf5fb1a 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -234,6 +234,9 @@ void __init efi_init(void)
 		return;
 	}
 
+	if (efi_secureboot_enabled_in_fdt())
+		set_bit(EFI_SECURE_BOOT, &efi.flags);
+
 	reserve_regions();
 	efi_esrt_init();
 
-- 
2.26.1


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

* [PATCH 5/6] arm64/ima: add ima arch support
  2020-09-04  7:28 [PATCH 0/6] add ima_arch support for ARM64 Chester Lin
                   ` (3 preceding siblings ...)
  2020-09-04  7:29 ` [PATCH 4/6] efi/arm: check secure boot status in efi init Chester Lin
@ 2020-09-04  7:29 ` Chester Lin
  2020-09-04 11:47   ` Mimi Zohar
  2020-09-04  7:29 ` [PATCH 6/6] docs/arm: add the description of uefi-secure-boot param Chester Lin
  5 siblings, 1 reply; 12+ messages in thread
From: Chester Lin @ 2020-09-04  7:29 UTC (permalink / raw)
  To: ardb, catalin.marinas, will, zohar, dmitry.kasatkin, corbet,
	mark.rutland, vincenzo.frascino, samitolvanen, masahiroy, mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee, clin

Add arm64 IMA arch support. The arch policy is inherited from x86.

Signed-off-by: Chester Lin <clin@suse.com>
---
 arch/arm64/Kconfig           |  1 +
 arch/arm64/kernel/Makefile   |  2 ++
 arch/arm64/kernel/ima_arch.c | 37 ++++++++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)
 create mode 100644 arch/arm64/kernel/ima_arch.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6d232837cbee..b5518e7b604d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -196,6 +196,7 @@ config ARM64
 	select SWIOTLB
 	select SYSCTL_EXCEPTION_TRACE
 	select THREAD_INFO_IN_TASK
+	imply IMA_SECURE_AND_OR_TRUSTED_BOOT    if EFI
 	help
 	  ARM 64-bit (AArch64) Linux support.
 
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index a561cbb91d4d..0300ab60785d 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -71,3 +71,5 @@ extra-y					+= $(head-y) vmlinux.lds
 ifeq ($(CONFIG_DEBUG_EFI),y)
 AFLAGS_head.o += -DVMLINUX_PATH="\"$(realpath $(objtree)/vmlinux)\""
 endif
+
+obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT)	+= ima_arch.o
diff --git a/arch/arm64/kernel/ima_arch.c b/arch/arm64/kernel/ima_arch.c
new file mode 100644
index 000000000000..46f5641c3da5
--- /dev/null
+++ b/arch/arm64/kernel/ima_arch.c
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2018 IBM Corporation
+ */
+#include <linux/efi.h>
+#include <linux/module.h>
+
+bool arch_ima_get_secureboot(void)
+{
+	if (efi_enabled(EFI_SECURE_BOOT))
+		return true;
+
+	return false;
+}
+
+/* secureboot arch rules */
+static const char * const sb_arch_rules[] = {
+#if !IS_ENABLED(CONFIG_KEXEC_SIG)
+	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
+#endif /* CONFIG_KEXEC_SIG */
+	"measure func=KEXEC_KERNEL_CHECK",
+#if !IS_ENABLED(CONFIG_MODULE_SIG)
+	"appraise func=MODULE_CHECK appraise_type=imasig",
+#endif
+	"measure func=MODULE_CHECK",
+	NULL
+};
+
+const char * const *arch_get_ima_policy(void)
+{
+	if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
+		if (IS_ENABLED(CONFIG_MODULE_SIG))
+			set_module_sig_enforced();
+		return sb_arch_rules;
+	}
+	return NULL;
+}
-- 
2.26.1


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

* [PATCH 6/6] docs/arm: add the description of uefi-secure-boot param
  2020-09-04  7:28 [PATCH 0/6] add ima_arch support for ARM64 Chester Lin
                   ` (4 preceding siblings ...)
  2020-09-04  7:29 ` [PATCH 5/6] arm64/ima: add ima arch support Chester Lin
@ 2020-09-04  7:29 ` Chester Lin
  5 siblings, 0 replies; 12+ messages in thread
From: Chester Lin @ 2020-09-04  7:29 UTC (permalink / raw)
  To: ardb, catalin.marinas, will, zohar, dmitry.kasatkin, corbet,
	mark.rutland, vincenzo.frascino, samitolvanen, masahiroy, mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee, clin

Add the description of "linux,uefi-secure-boot" param.

Signed-off-by: Chester Lin <clin@suse.com>
---
 Documentation/arm/uefi.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/arm/uefi.rst b/Documentation/arm/uefi.rst
index f868330df6be..7d9c6a1697af 100644
--- a/Documentation/arm/uefi.rst
+++ b/Documentation/arm/uefi.rst
@@ -64,4 +64,6 @@ linux,uefi-mmap-desc-size   32-bit   Size in bytes of each entry in the UEFI
                                      memory map.
 
 linux,uefi-mmap-desc-ver    32-bit   Version of the mmap descriptor format.
+
+linux,uefi-secure-boot      32-bit   UEFI Secure Boot [0: Disabled 1: Enabled]
 ==========================  ======   ===========================================
-- 
2.26.1


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

* Re: [PATCH 5/6] arm64/ima: add ima arch support
  2020-09-04  7:29 ` [PATCH 5/6] arm64/ima: add ima arch support Chester Lin
@ 2020-09-04 11:47   ` Mimi Zohar
  0 siblings, 0 replies; 12+ messages in thread
From: Mimi Zohar @ 2020-09-04 11:47 UTC (permalink / raw)
  To: Chester Lin, ardb, catalin.marinas, will, dmitry.kasatkin,
	corbet, mark.rutland, vincenzo.frascino, samitolvanen, masahiroy,
	mingo
  Cc: linux-kernel, linux-efi, linux-arm-kernel, linux-integrity,
	linux-doc, jlee

On Fri, 2020-09-04 at 15:29 +0800, Chester Lin wrote:
> Add arm64 IMA arch support. The arch policy is inherited from x86.
> 
> Signed-off-by: Chester Lin <clin@suse.com>

The "secureboot arch rules" comment should be updated to reflect that
the policy is both "secure and trusted boot arch rules", both here and
in x86.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

thanks,

Mimi

> ---
>  arch/arm64/Kconfig           |  1 +
>  arch/arm64/kernel/Makefile   |  2 ++
>  arch/arm64/kernel/ima_arch.c | 37 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+)
>  create mode 100644 arch/arm64/kernel/ima_arch.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 6d232837cbee..b5518e7b604d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -196,6 +196,7 @@ config ARM64
>  	select SWIOTLB
>  	select SYSCTL_EXCEPTION_TRACE
>  	select THREAD_INFO_IN_TASK
> +	imply IMA_SECURE_AND_OR_TRUSTED_BOOT    if EFI
>  	help
>  	  ARM 64-bit (AArch64) Linux support.
>  
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index a561cbb91d4d..0300ab60785d 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -71,3 +71,5 @@ extra-y					+= $(head-y) vmlinux.lds
>  ifeq ($(CONFIG_DEBUG_EFI),y)
>  AFLAGS_head.o += -DVMLINUX_PATH="\"$(realpath $(objtree)/vmlinux)\""
>  endif
> +
> +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT)	+= ima_arch.o
> diff --git a/arch/arm64/kernel/ima_arch.c b/arch/arm64/kernel/ima_arch.c
> new file mode 100644
> index 000000000000..46f5641c3da5
> --- /dev/null
> +++ b/arch/arm64/kernel/ima_arch.c
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2018 IBM Corporation
> + */
> +#include <linux/efi.h>
> +#include <linux/module.h>
> +
> +bool arch_ima_get_secureboot(void)
> +{
> +	if (efi_enabled(EFI_SECURE_BOOT))
> +		return true;
> +
> +	return false;
> +}
> +
> +/* secureboot arch rules */
> +static const char * const sb_arch_rules[] = {
> +#if !IS_ENABLED(CONFIG_KEXEC_SIG)
> +	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
> +#endif /* CONFIG_KEXEC_SIG */
> +	"measure func=KEXEC_KERNEL_CHECK",
> +#if !IS_ENABLED(CONFIG_MODULE_SIG)
> +	"appraise func=MODULE_CHECK appraise_type=imasig",
> +#endif
> +	"measure func=MODULE_CHECK",
> +	NULL
> +};
> +
> +const char * const *arch_get_ima_policy(void)
> +{
> +	if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
> +		if (IS_ENABLED(CONFIG_MODULE_SIG))
> +			set_module_sig_enforced();
> +		return sb_arch_rules;
> +	}
> +	return NULL;
> +}



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

* Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params
  2020-09-04  7:29 ` [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params Chester Lin
@ 2020-09-11 15:01   ` Ard Biesheuvel
  2020-09-14  8:05     ` Chester Lin
  0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2020-09-11 15:01 UTC (permalink / raw)
  To: Chester Lin
  Cc: Catalin Marinas, Will Deacon, zohar, dmitry.kasatkin,
	Jonathan Corbet, Mark Rutland, Vincenzo Frascino, Sami Tolvanen,
	Masahiro Yamada, Ingo Molnar, Linux Kernel Mailing List,
	linux-efi, Linux ARM, linux-integrity, Linux Doc Mailing List,
	Lee, Chun-Yi

On Fri, 4 Sep 2020 at 10:29, Chester Lin <clin@suse.com> wrote:
>
> Add a new UEFI parameter: "linux,uefi-secure-boot" in fdt boot params
> as other architectures have done in their own boot data. For example,
> the boot_params->secure_boot in x86.
>
> Signed-off-by: Chester Lin <clin@suse.com>

Why do we need this flag? Can't the OS simply check the variable directly?

> ---
>  drivers/firmware/efi/libstub/fdt.c | 39 +++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 11ecf3c4640e..c9a341e4715f 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -136,6 +136,10 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
>         if (status)
>                 goto fdt_set_fail;
>
> +       status = fdt_setprop_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
> +       if (status)
> +               goto fdt_set_fail;
> +
>         if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
>                 efi_status_t efi_status;
>
> @@ -199,6 +203,24 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
>         return EFI_SUCCESS;
>  }
>
> +static efi_status_t update_fdt_secboot(void *fdt, u32 secboot)
> +{
> +       int node = fdt_path_offset(fdt, "/chosen");
> +       u32 fdt_val32;
> +       int err;
> +
> +       if (node < 0)
> +               return EFI_LOAD_ERROR;
> +
> +       fdt_val32 = cpu_to_fdt32(secboot);
> +
> +       err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
> +       if (err)
> +               return EFI_LOAD_ERROR;
> +
> +       return EFI_SUCCESS;
> +}
> +
>  struct exit_boot_struct {
>         efi_memory_desc_t       *runtime_map;
>         int                     *runtime_entry_count;
> @@ -208,6 +230,9 @@ struct exit_boot_struct {
>  static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
>                                    void *priv)
>  {
> +       efi_status_t status;
> +       enum efi_secureboot_mode secboot_status;
> +       u32 secboot_var = 0;
>         struct exit_boot_struct *p = priv;
>         /*
>          * Update the memory map with virtual addresses. The function will also
> @@ -217,7 +242,19 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
>         efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
>                         p->runtime_map, p->runtime_entry_count);
>
> -       return update_fdt_memmap(p->new_fdt_addr, map);
> +       status = update_fdt_memmap(p->new_fdt_addr, map);
> +
> +       if (status != EFI_SUCCESS)
> +               return status;
> +
> +       secboot_status = efi_get_secureboot();
> +
> +       if (secboot_status == efi_secureboot_mode_enabled)
> +               secboot_var = 1;
> +
> +       status = update_fdt_secboot(p->new_fdt_addr, secboot_var);
> +
> +       return status;
>  }
>
>  #ifndef MAX_FDT_SIZE
> --
> 2.26.1
>

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

* Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params
  2020-09-11 15:01   ` Ard Biesheuvel
@ 2020-09-14  8:05     ` Chester Lin
  2020-10-05  2:20       ` Chester Lin
  0 siblings, 1 reply; 12+ messages in thread
From: Chester Lin @ 2020-09-14  8:05 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Catalin Marinas, Will Deacon, zohar, dmitry.kasatkin,
	Jonathan Corbet, Mark Rutland, Vincenzo Frascino, Sami Tolvanen,
	Masahiro Yamada, Ingo Molnar, Linux Kernel Mailing List,
	linux-efi, Linux ARM, linux-integrity, Linux Doc Mailing List,
	Lee, Chun-Yi, clin

Hi Ard,

On Fri, Sep 11, 2020 at 06:01:09PM +0300, Ard Biesheuvel wrote:
> On Fri, 4 Sep 2020 at 10:29, Chester Lin <clin@suse.com> wrote:
> >
> > Add a new UEFI parameter: "linux,uefi-secure-boot" in fdt boot params
> > as other architectures have done in their own boot data. For example,
> > the boot_params->secure_boot in x86.
> >
> > Signed-off-by: Chester Lin <clin@suse.com>
> 
> Why do we need this flag? Can't the OS simply check the variable directly?
> 

In fact, there's a difficulty to achieve this.

When linux kernel is booting on ARM, the runtime services are enabled later on.
It's done by arm_enable_runtime_services(), which is registered as an early_initcall.
Before it calls efi_native_runtime_setup(), all EFI runtime callbacks are still
NULL so calling efi.get_variable() will cause NULL pointer dereference.

There's a case that arch_ima_get_secureboot() can be called in early boot stage.
For example, when you try to set "ima_appraise=off" in kernel command line, it's
actually handled early:

[    0.000000] Kernel command line: BOOT_IMAGE=/boot/Image-5.9.0-rc3-9.gdd61cda-
vanilla root=UUID=a88bfb80-8abb-425c-a0f3-ad317465c28b splash=silent mitigations
=auto ignore_loglevel earlycon=pl011,mmio,0x9000000 console=ttyAMA0 ima_appraise=off
[    0.000000] ima: Secure boot enabled: ignoring ima_appraise=off boot parameter option
[    0.000000] Dentry cache hash table entries: 1048576 (order: 11, 8388608 bytes, linear)

However EFI services are remapped and enabled afterwards.

[    0.082286] rcu: Hierarchical SRCU implementation.
[    0.089592] Remapping and enabling EFI services.
[    0.097509] smp: Bringing up secondary CPUs ...

Another problem is that efi_rts_wq is created in subsys_initcall so we have to
wait for both EFI services mapping and the workqueue get initiated before calling
efi.get_variable() on ARM.

The only way I can think of is to put a flag via fdt params. May I have your
suggestions? I will appreciate if there's any better approach.

Thanks,
Chester

> > ---
> >  drivers/firmware/efi/libstub/fdt.c | 39 +++++++++++++++++++++++++++++-
> >  1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> > index 11ecf3c4640e..c9a341e4715f 100644
> > --- a/drivers/firmware/efi/libstub/fdt.c
> > +++ b/drivers/firmware/efi/libstub/fdt.c
> > @@ -136,6 +136,10 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
> >         if (status)
> >                 goto fdt_set_fail;
> >
> > +       status = fdt_setprop_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
> > +       if (status)
> > +               goto fdt_set_fail;
> > +
> >         if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> >                 efi_status_t efi_status;
> >
> > @@ -199,6 +203,24 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
> >         return EFI_SUCCESS;
> >  }
> >
> > +static efi_status_t update_fdt_secboot(void *fdt, u32 secboot)
> > +{
> > +       int node = fdt_path_offset(fdt, "/chosen");
> > +       u32 fdt_val32;
> > +       int err;
> > +
> > +       if (node < 0)
> > +               return EFI_LOAD_ERROR;
> > +
> > +       fdt_val32 = cpu_to_fdt32(secboot);
> > +
> > +       err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
> > +       if (err)
> > +               return EFI_LOAD_ERROR;
> > +
> > +       return EFI_SUCCESS;
> > +}
> > +
> >  struct exit_boot_struct {
> >         efi_memory_desc_t       *runtime_map;
> >         int                     *runtime_entry_count;
> > @@ -208,6 +230,9 @@ struct exit_boot_struct {
> >  static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
> >                                    void *priv)
> >  {
> > +       efi_status_t status;
> > +       enum efi_secureboot_mode secboot_status;
> > +       u32 secboot_var = 0;
> >         struct exit_boot_struct *p = priv;
> >         /*
> >          * Update the memory map with virtual addresses. The function will also
> > @@ -217,7 +242,19 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
> >         efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
> >                         p->runtime_map, p->runtime_entry_count);
> >
> > -       return update_fdt_memmap(p->new_fdt_addr, map);
> > +       status = update_fdt_memmap(p->new_fdt_addr, map);
> > +
> > +       if (status != EFI_SUCCESS)
> > +               return status;
> > +
> > +       secboot_status = efi_get_secureboot();
> > +
> > +       if (secboot_status == efi_secureboot_mode_enabled)
> > +               secboot_var = 1;
> > +
> > +       status = update_fdt_secboot(p->new_fdt_addr, secboot_var);
> > +
> > +       return status;
> >  }
> >
> >  #ifndef MAX_FDT_SIZE
> > --
> > 2.26.1
> >
> 


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

* Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params
  2020-09-14  8:05     ` Chester Lin
@ 2020-10-05  2:20       ` Chester Lin
  2020-10-12  8:20         ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Chester Lin @ 2020-10-05  2:20 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Catalin Marinas, Will Deacon, zohar, dmitry.kasatkin,
	Jonathan Corbet, Mark Rutland, Vincenzo Frascino, Sami Tolvanen,
	Masahiro Yamada, Ingo Molnar, Linux Kernel Mailing List,
	linux-efi, Linux ARM, linux-integrity, Linux Doc Mailing List,
	Lee, Chun-Yi

On Mon, Sep 14, 2020 at 04:05:22PM +0800, Chester Lin wrote:
> Hi Ard,
> 
> On Fri, Sep 11, 2020 at 06:01:09PM +0300, Ard Biesheuvel wrote:
> > On Fri, 4 Sep 2020 at 10:29, Chester Lin <clin@suse.com> wrote:
> > >
> > > Add a new UEFI parameter: "linux,uefi-secure-boot" in fdt boot params
> > > as other architectures have done in their own boot data. For example,
> > > the boot_params->secure_boot in x86.
> > >
> > > Signed-off-by: Chester Lin <clin@suse.com>
> > 
> > Why do we need this flag? Can't the OS simply check the variable directly?
> > 
> 
> In fact, there's a difficulty to achieve this.
> 
> When linux kernel is booting on ARM, the runtime services are enabled later on.
> It's done by arm_enable_runtime_services(), which is registered as an early_initcall.
> Before it calls efi_native_runtime_setup(), all EFI runtime callbacks are still
> NULL so calling efi.get_variable() will cause NULL pointer dereference.
> 
> There's a case that arch_ima_get_secureboot() can be called in early boot stage.
> For example, when you try to set "ima_appraise=off" in kernel command line, it's
> actually handled early:
> 
> [    0.000000] Kernel command line: BOOT_IMAGE=/boot/Image-5.9.0-rc3-9.gdd61cda-
> vanilla root=UUID=a88bfb80-8abb-425c-a0f3-ad317465c28b splash=silent mitigations
> =auto ignore_loglevel earlycon=pl011,mmio,0x9000000 console=ttyAMA0 ima_appraise=off
> [    0.000000] ima: Secure boot enabled: ignoring ima_appraise=off boot parameter option
> [    0.000000] Dentry cache hash table entries: 1048576 (order: 11, 8388608 bytes, linear)
> 
> However EFI services are remapped and enabled afterwards.
> 
> [    0.082286] rcu: Hierarchical SRCU implementation.
> [    0.089592] Remapping and enabling EFI services.
> [    0.097509] smp: Bringing up secondary CPUs ...
> 
> Another problem is that efi_rts_wq is created in subsys_initcall so we have to
> wait for both EFI services mapping and the workqueue get initiated before calling
> efi.get_variable() on ARM.
> 
> The only way I can think of is to put a flag via fdt params. May I have your
> suggestions? I will appreciate if there's any better approach.
> 
> Thanks,
> Chester

Ping. May I have some suggestions here?

Thanks,
Chester

> 
> > > ---
> > >  drivers/firmware/efi/libstub/fdt.c | 39 +++++++++++++++++++++++++++++-
> > >  1 file changed, 38 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> > > index 11ecf3c4640e..c9a341e4715f 100644
> > > --- a/drivers/firmware/efi/libstub/fdt.c
> > > +++ b/drivers/firmware/efi/libstub/fdt.c
> > > @@ -136,6 +136,10 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
> > >         if (status)
> > >                 goto fdt_set_fail;
> > >
> > > +       status = fdt_setprop_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
> > > +       if (status)
> > > +               goto fdt_set_fail;
> > > +
> > >         if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > >                 efi_status_t efi_status;
> > >
> > > @@ -199,6 +203,24 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
> > >         return EFI_SUCCESS;
> > >  }
> > >
> > > +static efi_status_t update_fdt_secboot(void *fdt, u32 secboot)
> > > +{
> > > +       int node = fdt_path_offset(fdt, "/chosen");
> > > +       u32 fdt_val32;
> > > +       int err;
> > > +
> > > +       if (node < 0)
> > > +               return EFI_LOAD_ERROR;
> > > +
> > > +       fdt_val32 = cpu_to_fdt32(secboot);
> > > +
> > > +       err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-secure-boot", fdt_val32);
> > > +       if (err)
> > > +               return EFI_LOAD_ERROR;
> > > +
> > > +       return EFI_SUCCESS;
> > > +}
> > > +
> > >  struct exit_boot_struct {
> > >         efi_memory_desc_t       *runtime_map;
> > >         int                     *runtime_entry_count;
> > > @@ -208,6 +230,9 @@ struct exit_boot_struct {
> > >  static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
> > >                                    void *priv)
> > >  {
> > > +       efi_status_t status;
> > > +       enum efi_secureboot_mode secboot_status;
> > > +       u32 secboot_var = 0;
> > >         struct exit_boot_struct *p = priv;
> > >         /*
> > >          * Update the memory map with virtual addresses. The function will also
> > > @@ -217,7 +242,19 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
> > >         efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
> > >                         p->runtime_map, p->runtime_entry_count);
> > >
> > > -       return update_fdt_memmap(p->new_fdt_addr, map);
> > > +       status = update_fdt_memmap(p->new_fdt_addr, map);
> > > +
> > > +       if (status != EFI_SUCCESS)
> > > +               return status;
> > > +
> > > +       secboot_status = efi_get_secureboot();
> > > +
> > > +       if (secboot_status == efi_secureboot_mode_enabled)
> > > +               secboot_var = 1;
> > > +
> > > +       status = update_fdt_secboot(p->new_fdt_addr, secboot_var);
> > > +
> > > +       return status;
> > >  }
> > >
> > >  #ifndef MAX_FDT_SIZE
> > > --
> > > 2.26.1
> > >
> > 


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

* Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params
  2020-10-05  2:20       ` Chester Lin
@ 2020-10-12  8:20         ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2020-10-12  8:20 UTC (permalink / raw)
  To: Chester Lin
  Cc: Catalin Marinas, Will Deacon, Mimi Zohar, dmitry.kasatkin,
	Jonathan Corbet, Mark Rutland, Vincenzo Frascino, Sami Tolvanen,
	Masahiro Yamada, Ingo Molnar, Linux Kernel Mailing List,
	linux-efi, Linux ARM, linux-integrity, Linux Doc Mailing List,
	Lee, Chun-Yi

On Mon, 5 Oct 2020 at 04:20, Chester Lin <clin@suse.com> wrote:
>
> On Mon, Sep 14, 2020 at 04:05:22PM +0800, Chester Lin wrote:
> > Hi Ard,
> >
> > On Fri, Sep 11, 2020 at 06:01:09PM +0300, Ard Biesheuvel wrote:
> > > On Fri, 4 Sep 2020 at 10:29, Chester Lin <clin@suse.com> wrote:
> > > >
> > > > Add a new UEFI parameter: "linux,uefi-secure-boot" in fdt boot params
> > > > as other architectures have done in their own boot data. For example,
> > > > the boot_params->secure_boot in x86.
> > > >
> > > > Signed-off-by: Chester Lin <clin@suse.com>
> > >
> > > Why do we need this flag? Can't the OS simply check the variable directly?
> > >
> >
> > In fact, there's a difficulty to achieve this.
> >
> > When linux kernel is booting on ARM, the runtime services are enabled later on.
> > It's done by arm_enable_runtime_services(), which is registered as an early_initcall.
> > Before it calls efi_native_runtime_setup(), all EFI runtime callbacks are still
> > NULL so calling efi.get_variable() will cause NULL pointer dereference.
> >
> > There's a case that arch_ima_get_secureboot() can be called in early boot stage.
> > For example, when you try to set "ima_appraise=off" in kernel command line, it's
> > actually handled early:
> >
> > [    0.000000] Kernel command line: BOOT_IMAGE=/boot/Image-5.9.0-rc3-9.gdd61cda-
> > vanilla root=UUID=a88bfb80-8abb-425c-a0f3-ad317465c28b splash=silent mitigations
> > =auto ignore_loglevel earlycon=pl011,mmio,0x9000000 console=ttyAMA0 ima_appraise=off
> > [    0.000000] ima: Secure boot enabled: ignoring ima_appraise=off boot parameter option
> > [    0.000000] Dentry cache hash table entries: 1048576 (order: 11, 8388608 bytes, linear)
> >
> > However EFI services are remapped and enabled afterwards.
> >
> > [    0.082286] rcu: Hierarchical SRCU implementation.
> > [    0.089592] Remapping and enabling EFI services.
> > [    0.097509] smp: Bringing up secondary CPUs ...
> >
> > Another problem is that efi_rts_wq is created in subsys_initcall so we have to
> > wait for both EFI services mapping and the workqueue get initiated before calling
> > efi.get_variable() on ARM.
> >
> > The only way I can think of is to put a flag via fdt params. May I have your
> > suggestions? I will appreciate if there's any better approach.
> >
> > Thanks,
> > Chester
>
> Ping. May I have some suggestions here?
>

IMA itself is initialized as a late initcall. The only reason you see
this message early is because this is where the parsing of the command
line parameter happens.

I'll send out a patch with a proposed solution for this issue.

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

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04  7:28 [PATCH 0/6] add ima_arch support for ARM64 Chester Lin
2020-09-04  7:29 ` [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params Chester Lin
2020-09-11 15:01   ` Ard Biesheuvel
2020-09-14  8:05     ` Chester Lin
2020-10-05  2:20       ` Chester Lin
2020-10-12  8:20         ` Ard Biesheuvel
2020-09-04  7:29 ` [PATCH 2/6] efi/arm: a helper to parse secure boot param in " Chester Lin
2020-09-04  7:29 ` [PATCH 3/6] efi: add secure boot flag Chester Lin
2020-09-04  7:29 ` [PATCH 4/6] efi/arm: check secure boot status in efi init Chester Lin
2020-09-04  7:29 ` [PATCH 5/6] arm64/ima: add ima arch support Chester Lin
2020-09-04 11:47   ` Mimi Zohar
2020-09-04  7:29 ` [PATCH 6/6] docs/arm: add the description of uefi-secure-boot param Chester Lin

Linux-EFI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-efi/0 linux-efi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-efi linux-efi/ https://lore.kernel.org/linux-efi \
		linux-efi@vger.kernel.org
	public-inbox-index linux-efi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-efi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git