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
Add a new EFI flag to indicate whether secure boot is enabled by UEFI
firmware or not.
Signed-off-by: Chester Lin <[email protected]>
---
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
Add arm64 IMA arch support. The arch policy is inherited from x86.
Signed-off-by: Chester Lin <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
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;
> +}
On Fri, 4 Sep 2020 at 10:29, Chester Lin <[email protected]> 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 <[email protected]>
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
>
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 <[email protected]> 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 <[email protected]>
>
> 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
> >
>
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 <[email protected]> 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 <[email protected]>
> >
> > 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
> > >
> >
On Mon, 5 Oct 2020 at 04:20, Chester Lin <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > >
> > > 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.