[tip: efi/core] ima: generalize x86/EFI arch glue for other EFI architectures

2020-11-17 Thread tip-bot2 for Chester Lin
The following commit has been merged into the efi/core branch of tip:

Commit-ID: 25519d68344269f9dc58b5bc72f648248a1fafb9
Gitweb:
https://git.kernel.org/tip/25519d68344269f9dc58b5bc72f648248a1fafb9
Author:Chester Lin 
AuthorDate:Fri, 30 Oct 2020 14:08:39 +08:00
Committer: Ard Biesheuvel 
CommitterDate: Fri, 06 Nov 2020 07:40:42 +01:00

ima: generalize x86/EFI arch glue for other EFI architectures

Move the x86 IMA arch code into security/integrity/ima/ima_efi.c,
so that we will be able to wire it up for arm64 in a future patch.

Co-developed-by: Chester Lin 
Signed-off-by: Chester Lin 
Acked-by: Mimi Zohar 
Signed-off-by: Ard Biesheuvel 
---
 arch/x86/include/asm/efi.h   |  3 +-
 arch/x86/kernel/Makefile |  2 +-
 arch/x86/kernel/ima_arch.c   | 94 +---
 security/integrity/ima/Makefile  |  4 +-
 security/integrity/ima/ima_efi.c | 73 -
 5 files changed, 80 insertions(+), 96 deletions(-)
 delete mode 100644 arch/x86/kernel/ima_arch.c
 create mode 100644 security/integrity/ima/ima_efi.c

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 7673dc8..c98f783 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -380,4 +380,7 @@ static inline void efi_fake_memmap_early(void)
 }
 #endif
 
+#define arch_ima_efi_boot_mode \
+   ({ extern struct boot_params boot_params; boot_params.secure_boot; })
+
 #endif /* _ASM_X86_EFI_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 68608bd..5eeb808 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -161,5 +161,3 @@ ifeq ($(CONFIG_X86_64),y)
obj-$(CONFIG_MMCONF_FAM10H) += mmconf-fam10h_64.o
obj-y   += vsmp_64.o
 endif
-
-obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT)   += ima_arch.o
diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
deleted file mode 100644
index 7dfb1e8..000
--- a/arch/x86/kernel/ima_arch.c
+++ /dev/null
@@ -1,94 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * Copyright (C) 2018 IBM Corporation
- */
-#include 
-#include 
-#include 
-
-extern struct boot_params boot_params;
-
-static enum efi_secureboot_mode get_sb_mode(void)
-{
-   efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-   efi_status_t status;
-   unsigned long size;
-   u8 secboot, setupmode;
-
-   size = sizeof(secboot);
-
-   if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
-   pr_info("ima: secureboot mode unknown, no efi\n");
-   return efi_secureboot_mode_unknown;
-   }
-
-   /* Get variable contents into buffer */
-   status = efi.get_variable(L"SecureBoot", _variable_guid,
- NULL, , );
-   if (status == EFI_NOT_FOUND) {
-   pr_info("ima: secureboot mode disabled\n");
-   return efi_secureboot_mode_disabled;
-   }
-
-   if (status != EFI_SUCCESS) {
-   pr_info("ima: secureboot mode unknown\n");
-   return efi_secureboot_mode_unknown;
-   }
-
-   size = sizeof(setupmode);
-   status = efi.get_variable(L"SetupMode", _variable_guid,
- NULL, , );
-
-   if (status != EFI_SUCCESS)  /* ignore unknown SetupMode */
-   setupmode = 0;
-
-   if (secboot == 0 || setupmode == 1) {
-   pr_info("ima: secureboot mode disabled\n");
-   return efi_secureboot_mode_disabled;
-   }
-
-   pr_info("ima: secureboot mode enabled\n");
-   return efi_secureboot_mode_enabled;
-}
-
-bool arch_ima_get_secureboot(void)
-{
-   static enum efi_secureboot_mode sb_mode;
-   static bool initialized;
-
-   if (!initialized && efi_enabled(EFI_BOOT)) {
-   sb_mode = boot_params.secure_boot;
-
-   if (sb_mode == efi_secureboot_mode_unset)
-   sb_mode = get_sb_mode();
-   initialized = true;
-   }
-
-   if (sb_mode == efi_secureboot_mode_enabled)
-   return true;
-   else
-   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;
-   }

[tip: efi/core] efi: generalize efi_get_secureboot

2020-11-17 Thread tip-bot2 for Chester Lin
The following commit has been merged into the efi/core branch of tip:

Commit-ID: e1ac4b2406d94eddce8ac2c5ab4235f6075a9602
Gitweb:
https://git.kernel.org/tip/e1ac4b2406d94eddce8ac2c5ab4235f6075a9602
Author:Chester Lin 
AuthorDate:Fri, 30 Oct 2020 14:08:38 +08:00
Committer: Ard Biesheuvel 
CommitterDate: Wed, 04 Nov 2020 23:05:40 +01:00

efi: generalize efi_get_secureboot

Generalize the efi_get_secureboot() function so not only efistub but also
other subsystems can use it.

Note that the MokSbState handling is not factored out: the variable is
boot time only, and so it cannot be parameterized as easily. Also, the
IMA code will switch to this version in a future patch, and it does not
incorporate the MokSbState exception in the first place.

Note that the new efi_get_secureboot_mode() helper treats any failures
to read SetupMode as setup mode being disabled.

Co-developed-by: Chester Lin 
Signed-off-by: Chester Lin 
Acked-by: Mimi Zohar 
Signed-off-by: Ard Biesheuvel 
---
 arch/x86/boot/compressed/Makefile |  2 +-
 drivers/firmware/efi/libstub/efistub.h|  2 +-
 drivers/firmware/efi/libstub/secureboot.c | 41 --
 include/linux/efi.h   | 23 +++-
 4 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index ee24908..8d358a6 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -35,7 +35,7 @@ cflags-$(CONFIG_X86_32) := -march=i386
 cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
 KBUILD_CFLAGS += $(cflags-y)
 KBUILD_CFLAGS += -mno-mmx -mno-sse
-KBUILD_CFLAGS += -ffreestanding
+KBUILD_CFLAGS += -ffreestanding -fshort-wchar
 KBUILD_CFLAGS += -fno-stack-protector
 KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
 KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
diff --git a/drivers/firmware/efi/libstub/efistub.h 
b/drivers/firmware/efi/libstub/efistub.h
index 2d7abcd..b8ec29d 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -848,4 +848,6 @@ asmlinkage void __noreturn efi_enter_kernel(unsigned long 
entrypoint,
 
 void efi_handle_post_ebs_state(void);
 
+enum efi_secureboot_mode efi_get_secureboot(void);
+
 #endif
diff --git a/drivers/firmware/efi/libstub/secureboot.c 
b/drivers/firmware/efi/libstub/secureboot.c
index 5efc524..af18d86 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -12,15 +12,16 @@
 
 #include "efistub.h"
 
-/* BIOS variables */
-static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-static const efi_char16_t efi_SecureBoot_name[] = L"SecureBoot";
-static const efi_char16_t efi_SetupMode_name[] = L"SetupMode";
-
 /* SHIM variables */
 static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
 static const efi_char16_t shim_MokSBState_name[] = L"MokSBState";
 
+static efi_status_t get_var(efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
+   unsigned long *data_size, void *data)
+{
+   return get_efi_var(name, vendor, attr, data_size, data);
+}
+
 /*
  * Determine whether we're in secure boot mode.
  *
@@ -30,26 +31,18 @@ static const efi_char16_t shim_MokSBState_name[] = 
L"MokSBState";
 enum efi_secureboot_mode efi_get_secureboot(void)
 {
u32 attr;
-   u8 secboot, setupmode, moksbstate;
unsigned long size;
+   enum efi_secureboot_mode mode;
efi_status_t status;
+   u8 moksbstate;
 
-   size = sizeof(secboot);
-   status = get_efi_var(efi_SecureBoot_name, _variable_guid,
-NULL, , );
-   if (status == EFI_NOT_FOUND)
-   return efi_secureboot_mode_disabled;
-   if (status != EFI_SUCCESS)
-   goto out_efi_err;
-
-   size = sizeof(setupmode);
-   status = get_efi_var(efi_SetupMode_name, _variable_guid,
-NULL, , );
-   if (status != EFI_SUCCESS)
-   goto out_efi_err;
-
-   if (secboot == 0 || setupmode == 1)
-   return efi_secureboot_mode_disabled;
+   mode = efi_get_secureboot_mode(get_var);
+   if (mode == efi_secureboot_mode_unknown) {
+   efi_err("Could not determine UEFI Secure Boot status.\n");
+   return efi_secureboot_mode_unknown;
+   }
+   if (mode != efi_secureboot_mode_enabled)
+   return mode;
 
/*
 * See if a user has put the shim into insecure mode. If so, and if the
@@ -69,8 +62,4 @@ enum efi_secureboot_mode efi_get_secureboot(void)
 secure_boot_enabled:
efi_info("UEFI Secure Boot is enabled.\n");
return efi_secureboot_mode_enabled;
-
-out_efi_err:
-   efi_err("Could not determine UEFI Secure Boot status.\n");
-   return efi_secureboot_mode_unknown;
 }
diff --git a/inclu

[tip: efi/core] arm64/ima: add ima_arch support

2020-11-17 Thread tip-bot2 for Chester Lin
The following commit has been merged into the efi/core branch of tip:

Commit-ID: 8d39cee0592e0129280e5a3cc480d64649c5e63f
Gitweb:
https://git.kernel.org/tip/8d39cee0592e0129280e5a3cc480d64649c5e63f
Author:Chester Lin 
AuthorDate:Fri, 30 Oct 2020 14:08:40 +08:00
Committer: Ard Biesheuvel 
CommitterDate: Tue, 17 Nov 2020 15:09:32 +01:00

arm64/ima: add ima_arch support

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

Co-developed-by: Chester Lin 
Signed-off-by: Chester Lin 
Acked-by: Mimi Zohar 
Acked-by: Catalin Marinas 
Signed-off-by: Ard Biesheuvel 
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index f858c35..04e78a3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1849,6 +1849,7 @@ config EFI
select EFI_RUNTIME_WRAPPERS
select EFI_STUB
select EFI_GENERIC_STUB
+   imply IMA_SECURE_AND_OR_TRUSTED_BOOT
default y
help
  This option provides support for runtime services provided


Re: [PATCH v3 3/3] arm64/ima: add ima_arch support

2020-11-01 Thread Chester Lin
On Fri, Oct 30, 2020 at 12:53:25PM +0100, Ard Biesheuvel wrote:
> On Fri, 30 Oct 2020 at 07:09, Chester Lin  wrote:
> >
> > Add arm64 IMA arch support. The code and arch policy is mainly inherited
> > from x86.
> >
> > Signed-off-by: Chester Lin 
> > ---
> >  arch/arm64/Kconfig   |  1 +
> >  arch/arm64/kernel/Makefile   |  2 ++
> >  arch/arm64/kernel/ima_arch.c | 43 
> >  3 files changed, 46 insertions(+)
> >  create mode 100644 arch/arm64/kernel/ima_arch.c
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index a42e8d13cc88..496a4a26afc6 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -201,6 +201,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 bbaf0bc4ad60..0f6cbb50668c 100644
> > --- a/arch/arm64/kernel/Makefile
> > +++ b/arch/arm64/kernel/Makefile
> > @@ -69,3 +69,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 ..564236d77adc
> > --- /dev/null
> > +++ b/arch/arm64/kernel/ima_arch.c
> > @@ -0,0 +1,43 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2018 IBM Corporation
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +
> > +bool arch_ima_get_secureboot(void)
> > +{
> > +   static bool sb_enabled;
> > +   static bool initialized;
> > +
> > +   if (!initialized & efi_enabled(EFI_BOOT)) {
> > +   sb_enabled = ima_get_efi_secureboot();
> > +   initialized = true;
> > +   }
> > +
> > +   return sb_enabled;
> > +}
> > +
> > +/* secure and trusted boot 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.28.0
> >
> 
> Can we move all this stuff into security/integrity/ima/ima_efi.c instead?
>
Actually I hesitated to move all this stuff into ima_efi.c when coding v3
because I haven't figured out a clear picture to achieve it. Since each
architecture could still have different details to trigger secure boot detection
and define their arch-specific rules [e.g. Having boot_params in x86_64 creates
more conditions that need to be determined before calling get_sb_mode()], moving
all this stuff seems to decrease the flexibility. Besides, it might also affect
the consistency of ima_arch as well, for example, ppc and s390 still use these
function prototypes defined in ima.h. Since these functions are already referred
by non-EFI architectures, why don't we still reuse these prototypes? For 
example,
we could remain a smaller arch_ima_get_secureboot() and the arch-specific rules
but move the major part of arch_get_ima_policy() into ima_efi.c. For example,
we could implement an efi_ima_policy() for arch_get_ima_policy() to call so that
the arch_get_ima_policy() doesn't have to know some details such as checking
conditions or calling set_module_sig_enforced().

Please feel free to let me know if any suggestions.



Re: [PATCH v3 1/3] efi: generalize efi_get_secureboot

2020-11-01 Thread Chester Lin
Hi Ard,

Thanks for your time and reviewing.

On Fri, Oct 30, 2020 at 12:51:10PM +0100, Ard Biesheuvel wrote:
> Hello Chester,
> 
> Thanks again for looking into this.
> 
> On Fri, 30 Oct 2020 at 07:09, Chester Lin  wrote:
> >
> > Generalize the efi_get_secureboot() function so not only efistub but also
> > other subsystems can use it.
> >
> > Signed-off-by: Chester Lin 
> > ---
> >  drivers/firmware/efi/libstub/Makefile |  2 +-
> >  drivers/firmware/efi/libstub/efi-stub.c   |  2 +-
> >  drivers/firmware/efi/libstub/efistub.h| 22 ---
> >  drivers/firmware/efi/libstub/secureboot.c | 76 ---
> >  drivers/firmware/efi/libstub/x86-stub.c   |  2 +-
> >  include/linux/efi.h   | 41 +++-
> >  6 files changed, 57 insertions(+), 88 deletions(-)
> >  delete mode 100644 drivers/firmware/efi/libstub/secureboot.c
> >
> > diff --git a/drivers/firmware/efi/libstub/Makefile 
> > b/drivers/firmware/efi/libstub/Makefile
> > index 8a94388e38b3..88e47b0ca09d 100644
> > --- a/drivers/firmware/efi/libstub/Makefile
> > +++ b/drivers/firmware/efi/libstub/Makefile
> > @@ -49,7 +49,7 @@ OBJECT_FILES_NON_STANDARD := y
> >  # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
> >  KCOV_INSTRUMENT:= n
> >
> > -lib-y  := efi-stub-helper.o gop.o secureboot.o 
> > tpm.o \
> > +lib-y  := efi-stub-helper.o gop.o tpm.o \
> >file.o mem.o random.o randomalloc.o 
> > pci.o \
> >skip_spaces.o lib-cmdline.o lib-ctype.o \
> >alignedmem.o relocate.o vsprintf.o
> > diff --git a/drivers/firmware/efi/libstub/efi-stub.c 
> > b/drivers/firmware/efi/libstub/efi-stub.c
> > index 914a343c7785..ad96f1d786a9 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub.c
> > @@ -196,7 +196,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
> > /* Ask the firmware to clear memory on unclean shutdown */
> > efi_enable_reset_attack_mitigation();
> >
> > -   secure_boot = efi_get_secureboot();
> > +   secure_boot = efi_get_secureboot(get_efi_var);
> >
> > /*
> >  * Unauthenticated device tree data is a security hazard, so ignore
> > diff --git a/drivers/firmware/efi/libstub/efistub.h 
> > b/drivers/firmware/efi/libstub/efistub.h
> > index 2d7abcd99de9..b1833b51e6d6 100644
> > --- a/drivers/firmware/efi/libstub/efistub.h
> > +++ b/drivers/firmware/efi/libstub/efistub.h
> > @@ -91,14 +91,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
> > fdt_setprop((fdt), (node_offset), (name), &(var), sizeof(var))
> >  #endif
> >
> > -#define get_efi_var(name, vendor, ...) \
> > -   efi_rt_call(get_variable, (efi_char16_t *)(name),   \
> > -   (efi_guid_t *)(vendor), __VA_ARGS__)
> > -
> > -#define set_efi_var(name, vendor, ...) \
> > -   efi_rt_call(set_variable, (efi_char16_t *)(name),   \
> > -   (efi_guid_t *)(vendor), __VA_ARGS__)
> > -
> >  #define efi_get_handle_at(array, idx)  \
> > (efi_is_native() ? (array)[idx] \
> > : (efi_handle_t)(unsigned long)((u32 *)(array))[idx])
> > @@ -112,6 +104,20 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
> > ((handle = efi_get_handle_at((array), i)) || true); \
> >  i++)
> >
> > +static inline
> > +efi_status_t get_efi_var(efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
> > +unsigned long *size, void *data)
> > +{
> > +   return efi_rt_call(get_variable, name, vendor, attr, size, data);
> > +}
> > +
> > +static inline
> > +efi_status_t set_efi_var(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
> > +unsigned long size, void *data)
> > +{
> > +   return efi_rt_call(set_variable, name, vendor, attr, size, data);
> > +}
> > +
> >  static inline
> >  void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
> >  {
> > diff --git a/drivers/firmware/efi/libstub/secureboot.c 
> > b/drivers/firmware/efi/libstub/secureboot.c
> > deleted file mode 100644
> > index 5efc524b14be..
> > --- a/drivers/firmware/efi/libstub/secureboot.c
&g

[PATCH v3 2/3] ima: replace arch-specific get_sb_mode() with a common helper ima_get_efi_secureboot()

2020-10-30 Thread Chester Lin
remove the get_sb_mode() from x86/kernel/ima_arch.c and create a common
helper ima_get_efi_secureboot() in IMA so that all EFI-based architectures
can refer to the same procedure.

Signed-off-by: Chester Lin 
---
 arch/x86/kernel/ima_arch.c   | 69 +++-
 include/linux/ima.h  | 10 +
 security/integrity/ima/Makefile  |  1 +
 security/integrity/ima/ima_efi.c | 26 
 4 files changed, 51 insertions(+), 55 deletions(-)
 create mode 100644 security/integrity/ima/ima_efi.c

diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
index 7dfb1e808928..2c773532ff0a 100644
--- a/arch/x86/kernel/ima_arch.c
+++ b/arch/x86/kernel/ima_arch.c
@@ -8,69 +8,28 @@
 
 extern struct boot_params boot_params;
 
-static enum efi_secureboot_mode get_sb_mode(void)
-{
-   efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-   efi_status_t status;
-   unsigned long size;
-   u8 secboot, setupmode;
-
-   size = sizeof(secboot);
-
-   if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
-   pr_info("ima: secureboot mode unknown, no efi\n");
-   return efi_secureboot_mode_unknown;
-   }
-
-   /* Get variable contents into buffer */
-   status = efi.get_variable(L"SecureBoot", _variable_guid,
- NULL, , );
-   if (status == EFI_NOT_FOUND) {
-   pr_info("ima: secureboot mode disabled\n");
-   return efi_secureboot_mode_disabled;
-   }
-
-   if (status != EFI_SUCCESS) {
-   pr_info("ima: secureboot mode unknown\n");
-   return efi_secureboot_mode_unknown;
-   }
-
-   size = sizeof(setupmode);
-   status = efi.get_variable(L"SetupMode", _variable_guid,
- NULL, , );
-
-   if (status != EFI_SUCCESS)  /* ignore unknown SetupMode */
-   setupmode = 0;
-
-   if (secboot == 0 || setupmode == 1) {
-   pr_info("ima: secureboot mode disabled\n");
-   return efi_secureboot_mode_disabled;
-   }
-
-   pr_info("ima: secureboot mode enabled\n");
-   return efi_secureboot_mode_enabled;
-}
-
 bool arch_ima_get_secureboot(void)
 {
-   static enum efi_secureboot_mode sb_mode;
-   static bool initialized;
-
-   if (!initialized && efi_enabled(EFI_BOOT)) {
-   sb_mode = boot_params.secure_boot;
+   static bool sb_enabled, initialized;
 
-   if (sb_mode == efi_secureboot_mode_unset)
-   sb_mode = get_sb_mode();
+   if (initialized) {
+   return sb_enabled;
+   } else if (efi_enabled(EFI_BOOT)) {
initialized = true;
+
+   if (boot_params.secure_boot == efi_secureboot_mode_unset) {
+   sb_enabled = ima_get_efi_secureboot();
+   return sb_enabled;
+   }
}
 
-   if (sb_mode == efi_secureboot_mode_enabled)
-   return true;
-   else
-   return false;
+   if (boot_params.secure_boot == efi_secureboot_mode_enabled)
+   sb_enabled = true;
+
+   return sb_enabled;
 }
 
-/* secureboot arch rules */
+/* secure and trusted boot arch rules */
 static const char * const sb_arch_rules[] = {
 #if !IS_ENABLED(CONFIG_KEXEC_SIG)
"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 8fa7bcfb2da2..9f9699f017be 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -50,6 +50,16 @@ static inline const char * const *arch_get_ima_policy(void)
 }
 #endif
 
+#if defined(CONFIG_EFI) && defined(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT)
+extern bool ima_get_efi_secureboot(void);
+#else
+static inline bool ima_get_efi_secureboot(void)
+{
+   return false;
+}
+#endif
+
+
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
 {
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 67dabca670e2..32076b3fd292 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -14,3 +14,4 @@ ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
 ima-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
 ima-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o
 ima-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o
+ima-$(CONFIG_EFI) += ima_efi.o
diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c
new file mode 100644
index ..a78f66e19689
--- /dev/null
+++ b/security/integrity/ima/ima_efi.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 SUSE LLC
+ *
+ * Author:
+ * Chester Lin 
+ */
+
+#include 
+#include 
+
+#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
+bool ima_get_efi_secureboot(void)
+{
+   enum efi_secureboot_mode sb_mode;
+

[PATCH v3 3/3] arm64/ima: add ima_arch support

2020-10-30 Thread Chester Lin
Add arm64 IMA arch support. The code and arch policy is mainly inherited
from x86.

Signed-off-by: Chester Lin 
---
 arch/arm64/Kconfig   |  1 +
 arch/arm64/kernel/Makefile   |  2 ++
 arch/arm64/kernel/ima_arch.c | 43 
 3 files changed, 46 insertions(+)
 create mode 100644 arch/arm64/kernel/ima_arch.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a42e8d13cc88..496a4a26afc6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -201,6 +201,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 bbaf0bc4ad60..0f6cbb50668c 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -69,3 +69,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 ..564236d77adc
--- /dev/null
+++ b/arch/arm64/kernel/ima_arch.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 IBM Corporation
+ */
+#include 
+#include 
+#include 
+
+bool arch_ima_get_secureboot(void)
+{
+   static bool sb_enabled;
+   static bool initialized;
+
+   if (!initialized & efi_enabled(EFI_BOOT)) {
+   sb_enabled = ima_get_efi_secureboot();
+   initialized = true;
+   }
+
+   return sb_enabled;
+}
+
+/* secure and trusted boot 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.28.0



[PATCH v3 0/3] add ima_arch support for ARM64

2020-10-30 Thread Chester Lin
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.

Changes in v3:
- Generalize efi_get_secureboot() so both ima_arch and efistub can reuse
  it.
- Implement ima_get_efi_secureboot() as the replacement of get_sb_mode()
  so x86 and arm64 can share the same logic.

Changes in v2:
- Separate get_sb_mode() from x86 so all EFI-based architectures can reuse
  the same function.
- Refactor arch/arm64/kernel/ima_arch.c based on Ard's patch[1].

Test platforms:
- ARM64:  QEMU [aarch64-virt] + EDK2/OVMF
- ARM64:  NXP LX2160A-RDB + EDK2
- X86_64: Dell Lattitude 7490 + (BIOS 1.14.0 01/22/2020)

[1] https://www.spinics.net/lists/linux-efi/msg20645.html

Chester Lin (3):
  efi: generalize efi_get_secureboot
  ima: remove get_sb_mode() and create ima_get_efi_secureboot()
  arm64/ima: add ima_arch support

 arch/arm64/Kconfig|  1 +
 arch/arm64/kernel/Makefile|  2 +
 arch/arm64/kernel/ima_arch.c  | 43 +
 arch/x86/kernel/ima_arch.c| 69 +---
 drivers/firmware/efi/libstub/Makefile |  2 +-
 drivers/firmware/efi/libstub/efi-stub.c   |  2 +-
 drivers/firmware/efi/libstub/efistub.h| 22 ---
 drivers/firmware/efi/libstub/secureboot.c | 76 ---
 drivers/firmware/efi/libstub/x86-stub.c   |  2 +-
 include/linux/efi.h   | 41 +++-
 include/linux/ima.h   | 10 +++
 security/integrity/ima/Makefile   |  1 +
 security/integrity/ima/ima_efi.c  | 26 
 13 files changed, 154 insertions(+), 143 deletions(-)
 create mode 100644 arch/arm64/kernel/ima_arch.c
 delete mode 100644 drivers/firmware/efi/libstub/secureboot.c
 create mode 100644 security/integrity/ima/ima_efi.c

-- 
2.28.0



[PATCH v3 1/3] efi: generalize efi_get_secureboot

2020-10-30 Thread Chester Lin
Generalize the efi_get_secureboot() function so not only efistub but also
other subsystems can use it.

Signed-off-by: Chester Lin 
---
 drivers/firmware/efi/libstub/Makefile |  2 +-
 drivers/firmware/efi/libstub/efi-stub.c   |  2 +-
 drivers/firmware/efi/libstub/efistub.h| 22 ---
 drivers/firmware/efi/libstub/secureboot.c | 76 ---
 drivers/firmware/efi/libstub/x86-stub.c   |  2 +-
 include/linux/efi.h   | 41 +++-
 6 files changed, 57 insertions(+), 88 deletions(-)
 delete mode 100644 drivers/firmware/efi/libstub/secureboot.c

diff --git a/drivers/firmware/efi/libstub/Makefile 
b/drivers/firmware/efi/libstub/Makefile
index 8a94388e38b3..88e47b0ca09d 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -49,7 +49,7 @@ OBJECT_FILES_NON_STANDARD := y
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT:= n
 
-lib-y  := efi-stub-helper.o gop.o secureboot.o tpm.o \
+lib-y  := efi-stub-helper.o gop.o tpm.o \
   file.o mem.o random.o randomalloc.o pci.o \
   skip_spaces.o lib-cmdline.o lib-ctype.o \
   alignedmem.o relocate.o vsprintf.o
diff --git a/drivers/firmware/efi/libstub/efi-stub.c 
b/drivers/firmware/efi/libstub/efi-stub.c
index 914a343c7785..ad96f1d786a9 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -196,7 +196,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
/* Ask the firmware to clear memory on unclean shutdown */
efi_enable_reset_attack_mitigation();
 
-   secure_boot = efi_get_secureboot();
+   secure_boot = efi_get_secureboot(get_efi_var);
 
/*
 * Unauthenticated device tree data is a security hazard, so ignore
diff --git a/drivers/firmware/efi/libstub/efistub.h 
b/drivers/firmware/efi/libstub/efistub.h
index 2d7abcd99de9..b1833b51e6d6 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -91,14 +91,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
fdt_setprop((fdt), (node_offset), (name), &(var), sizeof(var))
 #endif
 
-#define get_efi_var(name, vendor, ...) \
-   efi_rt_call(get_variable, (efi_char16_t *)(name),   \
-   (efi_guid_t *)(vendor), __VA_ARGS__)
-
-#define set_efi_var(name, vendor, ...) \
-   efi_rt_call(set_variable, (efi_char16_t *)(name),   \
-   (efi_guid_t *)(vendor), __VA_ARGS__)
-
 #define efi_get_handle_at(array, idx)  \
(efi_is_native() ? (array)[idx] \
: (efi_handle_t)(unsigned long)((u32 *)(array))[idx])
@@ -112,6 +104,20 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
((handle = efi_get_handle_at((array), i)) || true); \
 i++)
 
+static inline
+efi_status_t get_efi_var(efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
+unsigned long *size, void *data)
+{
+   return efi_rt_call(get_variable, name, vendor, attr, size, data);
+}
+
+static inline
+efi_status_t set_efi_var(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
+unsigned long size, void *data)
+{
+   return efi_rt_call(set_variable, name, vendor, attr, size, data);
+}
+
 static inline
 void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
 {
diff --git a/drivers/firmware/efi/libstub/secureboot.c 
b/drivers/firmware/efi/libstub/secureboot.c
deleted file mode 100644
index 5efc524b14be..
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ /dev/null
@@ -1,76 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Secure boot handling.
- *
- * Copyright (C) 2013,2014 Linaro Limited
- * Roy Franz 
- */
-#include 
-#include 
-
-#include "efistub.h"
-
-/* BIOS variables */
-static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-static const efi_char16_t efi_SecureBoot_name[] = L"SecureBoot";
-static const efi_char16_t efi_SetupMode_name[] = L"SetupMode";
-
-/* SHIM variables */
-static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
-static const efi_char16_t shim_MokSBState_name[] = L"MokSBState";
-
-/*
- * Determine whether we're in secure boot mode.
- *
- * Please keep the logic in sync with
- * arch/x86/xen/efi.c:xen_efi_get_secureboot().
- */
-enum efi_secureboot_mode efi_get_secureboot(void)
-{
-   u32 attr;
-   u8 secboot, setupmode, moksbstate;
-   unsigned long size;
-   efi_status_t status;
-
-   size = sizeof(secboot);
-   status = get_efi_var(efi_SecureBoot_name, _variable_guid,
-NULL, , );
-   if (status == EFI_NOT_FOUND)
-  

Re: [PATCH v2 1/2] efi: add secure boot get helper

2020-10-15 Thread Chester Lin
Hi Ard and Mimi,

On Wed, Oct 14, 2020 at 07:56:17AM -0400, Mimi Zohar wrote:
> On Wed, 2020-10-14 at 13:00 +0200, Ard Biesheuvel wrote:
> > Hello Chester,
> > 
> > Thanks for looking into this.
> > 
> > Some comments below.
> > 
> > On Wed, 14 Oct 2020 at 12:41, Chester Lin  wrote:
> > >
> > > Separate the get_sb_mode() from arch/x86 and treat it as a common function
> > > [rename to efi_get_secureboot_mode] so all EFI-based architectures can
> > > reuse the same logic.
> > >
> > > Signed-off-by: Chester Lin 
> > > ---
> > >  arch/x86/kernel/ima_arch.c | 47 ++
> > >  drivers/firmware/efi/efi.c | 43 ++
> > >  include/linux/efi.h|  5 
> > >  3 files changed, 50 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
> > > index 7dfb1e808928..ed4623ecda6e 100644
> > > --- a/arch/x86/kernel/ima_arch.c
> > > +++ b/arch/x86/kernel/ima_arch.c
> > > @@ -8,49 +8,6 @@
> > >
> > >  extern struct boot_params boot_params;
> > >
> > > -static enum efi_secureboot_mode get_sb_mode(void)
> > > -{
> > > -   efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
> > > -   efi_status_t status;
> > > -   unsigned long size;
> > > -   u8 secboot, setupmode;
> > > -
> > > -   size = sizeof(secboot);
> > > -
> > > -   if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
> > > -   pr_info("ima: secureboot mode unknown, no efi\n");
> > > -   return efi_secureboot_mode_unknown;
> > > -   }
> > > -
> > > -   /* Get variable contents into buffer */
> > > -   status = efi.get_variable(L"SecureBoot", _variable_guid,
> > > - NULL, , );
> > > -   if (status == EFI_NOT_FOUND) {
> > > -   pr_info("ima: secureboot mode disabled\n");
> > > -   return efi_secureboot_mode_disabled;
> > > -   }
> > > -
> > > -   if (status != EFI_SUCCESS) {
> > > -   pr_info("ima: secureboot mode unknown\n");
> > > -   return efi_secureboot_mode_unknown;
> > > -   }
> > > -
> > > -   size = sizeof(setupmode);
> > > -   status = efi.get_variable(L"SetupMode", _variable_guid,
> > > - NULL, , );
> > > -
> > > -   if (status != EFI_SUCCESS)  /* ignore unknown SetupMode */
> > > -   setupmode = 0;
> > > -
> > > -   if (secboot == 0 || setupmode == 1) {
> > > -   pr_info("ima: secureboot mode disabled\n");
> > > -   return efi_secureboot_mode_disabled;
> > > -   }
> > > -
> > > -   pr_info("ima: secureboot mode enabled\n");
> > > -   return efi_secureboot_mode_enabled;
> > > -}
> > > -
> > >  bool arch_ima_get_secureboot(void)
> > >  {
> > > static enum efi_secureboot_mode sb_mode;
> > > @@ -60,7 +17,7 @@ bool arch_ima_get_secureboot(void)
> > > sb_mode = boot_params.secure_boot;
> > >
> > > if (sb_mode == efi_secureboot_mode_unset)
> > > -   sb_mode = get_sb_mode();
> > > +   sb_mode = efi_get_secureboot_mode();
> > > initialized = true;
> > > }
> > >
> > > @@ -70,7 +27,7 @@ bool arch_ima_get_secureboot(void)
> > > return false;
> > >  }
> > >
> > > -/* secureboot arch rules */
> > > +/* secure and trusted boot arch rules */
> > >  static const char * const sb_arch_rules[] = {
> > >  #if !IS_ENABLED(CONFIG_KEXEC_SIG)
> > > "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
> > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > > index 5e5480a0a32d..68ffa6a069c0 100644
> > > --- a/drivers/firmware/efi/efi.c
> > > +++ b/drivers/firmware/efi/efi.c
> > > @@ -1022,3 +1022,46 @@ static int __init 
> > > register_update_efi_random_seed(void)
> > >  }
> > >  late_initcall(register_update_efi_random_seed);
> > >  #endif
> > > +
> > > +enum efi_secureboot_mo

Re: [PATCH v2 1/2] efi: add secure boot get helper

2020-10-14 Thread Chester Lin
Hi Ard and Mimi,

On Wed, Oct 14, 2020 at 06:40:31PM +0800, Chester Lin wrote:
> Separate the get_sb_mode() from arch/x86 and treat it as a common function
> [rename to efi_get_secureboot_mode] so all EFI-based architectures can
> reuse the same logic.
> 
> Signed-off-by: Chester Lin 
> ---
>  arch/x86/kernel/ima_arch.c | 47 ++
>  drivers/firmware/efi/efi.c | 43 ++
>  include/linux/efi.h|  5 
>  3 files changed, 50 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
> index 7dfb1e808928..ed4623ecda6e 100644
> --- a/arch/x86/kernel/ima_arch.c
> +++ b/arch/x86/kernel/ima_arch.c
> @@ -8,49 +8,6 @@
>  
>  extern struct boot_params boot_params;
>  
> -static enum efi_secureboot_mode get_sb_mode(void)
> -{
> - efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
> - efi_status_t status;
> - unsigned long size;
> - u8 secboot, setupmode;
> -
> - size = sizeof(secboot);
> -
> - if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
> - pr_info("ima: secureboot mode unknown, no efi\n");
> - return efi_secureboot_mode_unknown;
> - }
> -
> - /* Get variable contents into buffer */
> - status = efi.get_variable(L"SecureBoot", _variable_guid,
> -   NULL, , );
> - if (status == EFI_NOT_FOUND) {
> - pr_info("ima: secureboot mode disabled\n");
> - return efi_secureboot_mode_disabled;
> - }
> -
> - if (status != EFI_SUCCESS) {
> - pr_info("ima: secureboot mode unknown\n");
> - return efi_secureboot_mode_unknown;
> - }
> -
> - size = sizeof(setupmode);
> - status = efi.get_variable(L"SetupMode", _variable_guid,
> -   NULL, , );
> -
> - if (status != EFI_SUCCESS)  /* ignore unknown SetupMode */
> - setupmode = 0;
> -
> - if (secboot == 0 || setupmode == 1) {
> - pr_info("ima: secureboot mode disabled\n");
> - return efi_secureboot_mode_disabled;
> - }
> -
> - pr_info("ima: secureboot mode enabled\n");
> - return efi_secureboot_mode_enabled;
> -}
> -
>  bool arch_ima_get_secureboot(void)
>  {
>   static enum efi_secureboot_mode sb_mode;
> @@ -60,7 +17,7 @@ bool arch_ima_get_secureboot(void)
>   sb_mode = boot_params.secure_boot;
>  
>   if (sb_mode == efi_secureboot_mode_unset)
> - sb_mode = get_sb_mode();
> + sb_mode = efi_get_secureboot_mode();
>   initialized = true;
>   }
>  
> @@ -70,7 +27,7 @@ bool arch_ima_get_secureboot(void)
>   return false;
>  }
>  
> -/* secureboot arch rules */
> +/* secure and trusted boot arch rules */
>  static const char * const sb_arch_rules[] = {
>  #if !IS_ENABLED(CONFIG_KEXEC_SIG)
>   "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 5e5480a0a32d..68ffa6a069c0 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c

I hope you don't mind that I temporarily move the get_sb_mode() to efi
since I'm not sure which place is better. Please let me know if any
suggestions.

Thanks,
Chester

> @@ -1022,3 +1022,46 @@ static int __init register_update_efi_random_seed(void)
>  }
>  late_initcall(register_update_efi_random_seed);
>  #endif
> +
> +enum efi_secureboot_mode efi_get_secureboot_mode(void)
> +{
> + efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
> + efi_status_t status;
> + unsigned long size;
> + u8 secboot, setupmode;
> +
> + size = sizeof(secboot);
> +
> + if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
> + pr_info("ima: secureboot mode unknown, no efi\n");
> + return efi_secureboot_mode_unknown;
> + }
> +
> + /* Get variable contents into buffer */
> + status = efi.get_variable(L"SecureBoot", _variable_guid,
> +   NULL, , );
> + if (status == EFI_NOT_FOUND) {
> + pr_info("ima: secureboot mode disabled\n");
> + return efi_secureboot_mode_disabled;
> + }
> +
> + if (status != EFI_SUCCESS) {
> + pr_info("ima: secureboot mode unknown\n");
> + return efi_secureboot_mode_unknown;
> + }
> +
> + size = sizeof(setupmode);
> + status = efi.

[PATCH v2 0/2] add ima_arch support for ARM64

2020-10-14 Thread Chester Lin
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.

Changes in v2:
- Separate get_sb_mode() from x86 so all EFI-based architectures can reuse
  the same function.
- Refactor arch/arm64/kernel/ima_arch.c based on Ard's patch[1].

Test platforms:
- QEMU [aarch64-virt] + EDK2/OVMF
- NXP LX2160A-RDB + EDK2

[1] https://www.spinics.net/lists/linux-efi/msg20645.html

Chester Lin (2):
  efi: add secure boot get helper
  arm64/ima: add ima_arch support

 arch/arm64/Kconfig   |  1 +
 arch/arm64/kernel/Makefile   |  2 ++
 arch/arm64/kernel/ima_arch.c | 46 +++
 arch/x86/kernel/ima_arch.c   | 47 ++--
 drivers/firmware/efi/efi.c   | 43 +
 include/linux/efi.h  |  5 
 6 files changed, 99 insertions(+), 45 deletions(-)
 create mode 100644 arch/arm64/kernel/ima_arch.c

-- 
2.26.1



[PATCH v2 1/2] efi: add secure boot get helper

2020-10-14 Thread Chester Lin
Separate the get_sb_mode() from arch/x86 and treat it as a common function
[rename to efi_get_secureboot_mode] so all EFI-based architectures can
reuse the same logic.

Signed-off-by: Chester Lin 
---
 arch/x86/kernel/ima_arch.c | 47 ++
 drivers/firmware/efi/efi.c | 43 ++
 include/linux/efi.h|  5 
 3 files changed, 50 insertions(+), 45 deletions(-)

diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
index 7dfb1e808928..ed4623ecda6e 100644
--- a/arch/x86/kernel/ima_arch.c
+++ b/arch/x86/kernel/ima_arch.c
@@ -8,49 +8,6 @@
 
 extern struct boot_params boot_params;
 
-static enum efi_secureboot_mode get_sb_mode(void)
-{
-   efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-   efi_status_t status;
-   unsigned long size;
-   u8 secboot, setupmode;
-
-   size = sizeof(secboot);
-
-   if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
-   pr_info("ima: secureboot mode unknown, no efi\n");
-   return efi_secureboot_mode_unknown;
-   }
-
-   /* Get variable contents into buffer */
-   status = efi.get_variable(L"SecureBoot", _variable_guid,
- NULL, , );
-   if (status == EFI_NOT_FOUND) {
-   pr_info("ima: secureboot mode disabled\n");
-   return efi_secureboot_mode_disabled;
-   }
-
-   if (status != EFI_SUCCESS) {
-   pr_info("ima: secureboot mode unknown\n");
-   return efi_secureboot_mode_unknown;
-   }
-
-   size = sizeof(setupmode);
-   status = efi.get_variable(L"SetupMode", _variable_guid,
- NULL, , );
-
-   if (status != EFI_SUCCESS)  /* ignore unknown SetupMode */
-   setupmode = 0;
-
-   if (secboot == 0 || setupmode == 1) {
-   pr_info("ima: secureboot mode disabled\n");
-   return efi_secureboot_mode_disabled;
-   }
-
-   pr_info("ima: secureboot mode enabled\n");
-   return efi_secureboot_mode_enabled;
-}
-
 bool arch_ima_get_secureboot(void)
 {
static enum efi_secureboot_mode sb_mode;
@@ -60,7 +17,7 @@ bool arch_ima_get_secureboot(void)
sb_mode = boot_params.secure_boot;
 
if (sb_mode == efi_secureboot_mode_unset)
-   sb_mode = get_sb_mode();
+   sb_mode = efi_get_secureboot_mode();
initialized = true;
}
 
@@ -70,7 +27,7 @@ bool arch_ima_get_secureboot(void)
return false;
 }
 
-/* secureboot arch rules */
+/* secure and trusted boot arch rules */
 static const char * const sb_arch_rules[] = {
 #if !IS_ENABLED(CONFIG_KEXEC_SIG)
"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 5e5480a0a32d..68ffa6a069c0 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -1022,3 +1022,46 @@ static int __init register_update_efi_random_seed(void)
 }
 late_initcall(register_update_efi_random_seed);
 #endif
+
+enum efi_secureboot_mode efi_get_secureboot_mode(void)
+{
+   efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+   efi_status_t status;
+   unsigned long size;
+   u8 secboot, setupmode;
+
+   size = sizeof(secboot);
+
+   if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
+   pr_info("ima: secureboot mode unknown, no efi\n");
+   return efi_secureboot_mode_unknown;
+   }
+
+   /* Get variable contents into buffer */
+   status = efi.get_variable(L"SecureBoot", _variable_guid,
+ NULL, , );
+   if (status == EFI_NOT_FOUND) {
+   pr_info("ima: secureboot mode disabled\n");
+   return efi_secureboot_mode_disabled;
+   }
+
+   if (status != EFI_SUCCESS) {
+   pr_info("ima: secureboot mode unknown\n");
+   return efi_secureboot_mode_unknown;
+   }
+
+   size = sizeof(setupmode);
+   status = efi.get_variable(L"SetupMode", _variable_guid,
+ NULL, , );
+
+   if (status != EFI_SUCCESS)  /* ignore unknown SetupMode */
+   setupmode = 0;
+
+   if (secboot == 0 || setupmode == 1) {
+   pr_info("ima: secureboot mode disabled\n");
+   return efi_secureboot_mode_disabled;
+   }
+
+   pr_info("ima: secureboot mode enabled\n");
+   return efi_secureboot_mode_enabled;
+}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index d7c0e73af2b9..a73e5ae04672 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1076,8 +1076,13 @@ static inline int efi_runtime_map_copy(void *buf, size_t 
bufsz)
 

[PATCH v2 2/2] arm64/ima: add ima_arch support

2020-10-14 Thread Chester Lin
Add arm64 IMA arch support. The code and arch policy is mainly inherited
from x86.

Signed-off-by: Chester Lin 
---
 arch/arm64/Kconfig   |  1 +
 arch/arm64/kernel/Makefile   |  2 ++
 arch/arm64/kernel/ima_arch.c | 46 
 3 files changed, 49 insertions(+)
 create mode 100644 arch/arm64/kernel/ima_arch.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 08fa3a1c50f0..15b7a3bbb7e5 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -199,6 +199,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 bbaf0bc4ad60..0f6cbb50668c 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -69,3 +69,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 ..f3103c8a2cab
--- /dev/null
+++ b/arch/arm64/kernel/ima_arch.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 IBM Corporation
+ */
+#include 
+#include 
+#include 
+
+bool arch_ima_get_secureboot(void)
+{
+   static enum efi_secureboot_mode sb_mode;
+   static bool initialized;
+
+   if (!initialized && efi_enabled(EFI_BOOT)) {
+   sb_mode = efi_get_secureboot_mode();
+   initialized = true;
+   }
+
+   if (sb_mode == efi_secureboot_mode_enabled)
+   return true;
+   else
+   return false;
+}
+
+/* secure and trusted boot 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



Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params

2020-10-04 Thread Chester Lin
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  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 
> > 
> > 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.00] 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,0x900 console=ttyAMA0 
> ima_appraise=off
> [0.00] ima: Secure boot enabled: ignoring ima_appraise=off boot 
> parameter option
> [0.00] 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

Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params

2020-09-14 Thread Chester Lin
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  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 
> 
> 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.00] 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,0x900 console=ttyAMA0 
ima_appraise=off
[0.00] ima: Secure boot enabled: ignoring ima_appraise=off boot 
parameter option
[0.00] 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
> >
> 



[PATCH 1/6] efistub: pass uefi secureboot flag via fdt params

2020-09-04 Thread Chester Lin
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 
---
 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



[PATCH 6/6] docs/arm: add the description of uefi-secure-boot param

2020-09-04 Thread Chester Lin
Add the description of "linux,uefi-secure-boot" param.

Signed-off-by: Chester Lin 
---
 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-ver32-bit   Version of the mmap descriptor format.
+
+linux,uefi-secure-boot  32-bit   UEFI Secure Boot [0: Disabled 1: Enabled]
 ==  ==   
===
-- 
2.26.1



[PATCH 5/6] arm64/ima: add ima arch support

2020-09-04 Thread Chester Lin
Add arm64 IMA arch support. The arch policy is inherited from x86.

Signed-off-by: Chester Lin 
---
 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_BOOTif 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 ..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 
+#include 
+
+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



[PATCH 3/6] efi: add secure boot flag

2020-09-04 Thread Chester Lin
Add a new EFI flag to indicate whether secure boot is enabled by UEFI
firmware or not.

Signed-off-by: Chester Lin 
---
 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_RESERVE11  /* Is the kernel configured to 
ignore soft reservations? */
 #define EFI_PRESERVE_BS_REGIONS12  /* Are EFI boot-services memory 
segments available? */
+#define EFI_SECURE_BOOT13  /* Is EFI secure-boot enabled? 
*/
 
 #ifdef CONFIG_EFI
 /*
-- 
2.26.1



[PATCH 4/6] efi/arm: check secure boot status in efi init

2020-09-04 Thread Chester Lin
set EFI_SECURE_BOOT flag when UEFI secure boot is eanbled on ARM.

Signed-off-by: Chester Lin 
---
 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, );
+
reserve_regions();
efi_esrt_init();
 
-- 
2.26.1



[PATCH 0/6] add ima_arch support for ARM64

2020-09-04 Thread Chester Lin
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



[PATCH 2/6] efi/arm: a helper to parse secure boot param in fdt params

2020-09-04 Thread Chester Lin
Add a helper to query the UEFI secureboot param from the chosen node in
FDT.

Signed-off-by: Chester Lin 
---
 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", , 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



Re: [PATCH] riscv: save space on the magic number field of image header

2019-09-06 Thread Chester Lin
Hi Anup,

On Fri, Sep 06, 2019 at 01:50:37PM +0530, Anup Patel wrote:
> On Fri, Sep 6, 2019 at 12:50 PM Chester Lin  wrote:
> >
> > Change the symbol from "RISCV" to "RSCV" so the magic number can be 32-bit
> > long, which is consistent with other architectures.
> >
> > Signed-off-by: Chester Lin 
> > ---
> >  arch/riscv/include/asm/image.h | 9 +
> >  arch/riscv/kernel/head.S   | 5 ++---
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/image.h b/arch/riscv/include/asm/image.h
> > index ef28e106f247..ec8bbfe86dde 100644
> > --- a/arch/riscv/include/asm/image.h
> > +++ b/arch/riscv/include/asm/image.h
> > @@ -3,7 +3,8 @@
> >  #ifndef __ASM_IMAGE_H
> >  #define __ASM_IMAGE_H
> >
> > -#define RISCV_IMAGE_MAGIC  "RISCV"
> > +#define RISCV_IMAGE_MAGIC  "RSCV"
> > +
> >
> >  #define RISCV_IMAGE_FLAG_BE_SHIFT  0
> >  #define RISCV_IMAGE_FLAG_BE_MASK   0x1
> > @@ -39,9 +40,9 @@
> >   * @version:   version
> >   * @res1:  reserved
> >   * @res2:  reserved
> > - * @magic: Magic number
> >   * @res3:  reserved (will be used for additional RISC-V 
> > specific
> >   * header)
> > + * @magic: Magic number
> >   * @res4:  reserved (will be used for PE COFF offset)
> >   *
> >   * The intention is for this header format to be shared between multiple
> > @@ -57,8 +58,8 @@ struct riscv_image_header {
> > u32 version;
> > u32 res1;
> > u64 res2;
> > -   u64 magic;
> > -   u32 res3;
> > +   u64 res3;
> > +   u32 magic;
> > u32 res4;
> >  };
> >  #endif /* __ASSEMBLY__ */
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index 0f1ba17e476f..1f8fffbecf68 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -39,9 +39,8 @@ ENTRY(_start)
> > .word RISCV_HEADER_VERSION
> > .word 0
> > .dword 0
> > -   .asciz RISCV_IMAGE_MAGIC
> > -   .word 0
> > -   .balign 4
> > +   .dword 0
> > +   .ascii RISCV_IMAGE_MAGIC
> > .word 0
> >
> >  .global _start_kernel
> > --
> > 2.22.0
> >
> 
> This change is not at all backward compatible with
> existing booti implementation in U-Boot.
> 
> It changes:
> 1. Magic offset
> 2. Magic value itself
> 

Thank you for the reminder. I have submitted a patch to U-Boot as well. Since
my email post to the uboot mailing list is still under review by the list
moderator, here I just list my code change of uboot:

diff --git a/arch/riscv/lib/image.c b/arch/riscv/lib/image.c
index d063beb7df..e8a8cb7190 100644
--- a/arch/riscv/lib/image.c
+++ b/arch/riscv/lib/image.c
@@ -14,8 +14,8 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-/* ASCII version of "RISCV" defined in Linux kernel */
-#define LINUX_RISCV_IMAGE_MAGIC 0x5643534952
+/* ASCII version of "RSCV" defined in Linux kernel */
+#define LINUX_RISCV_IMAGE_MAGIC 0x56435352
 
 struct linux_image_h {
uint32_tcode0;  /* Executable code */
@@ -25,8 +25,8 @@ struct linux_image_h {
uint64_tres1;   /* reserved */
uint64_tres2;   /* reserved */
uint64_tres3;   /* reserved */
-   uint64_tmagic;  /* Magic number */
-   uint32_tres4;   /* reserved */
+   uint64_tres4;   /* reserved */
+   uint32_tmagic;  /* Magic number */
uint32_tres5;   /* reserved */
 };


> We don't see this header changing much apart from
> res1/res2 becoming flags in-future. The PE COFF header
> will be append to this header in-future and it will have lot
> more information.
> 

I think a smaller magic field will let res4 have more room [32bit->64bit], which
could offer more options for RISC-V's boot-flow development in the future. This
change also synchronizes with arm64's image header.

> Regards,
> Anup
> 


[PATCH] riscv: save space on the magic number field of image header

2019-09-06 Thread Chester Lin
Change the symbol from "RISCV" to "RSCV" so the magic number can be 32-bit
long, which is consistent with other architectures.

Signed-off-by: Chester Lin 
---
 arch/riscv/include/asm/image.h | 9 +
 arch/riscv/kernel/head.S   | 5 ++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/include/asm/image.h b/arch/riscv/include/asm/image.h
index ef28e106f247..ec8bbfe86dde 100644
--- a/arch/riscv/include/asm/image.h
+++ b/arch/riscv/include/asm/image.h
@@ -3,7 +3,8 @@
 #ifndef __ASM_IMAGE_H
 #define __ASM_IMAGE_H
 
-#define RISCV_IMAGE_MAGIC  "RISCV"
+#define RISCV_IMAGE_MAGIC  "RSCV"
+
 
 #define RISCV_IMAGE_FLAG_BE_SHIFT  0
 #define RISCV_IMAGE_FLAG_BE_MASK   0x1
@@ -39,9 +40,9 @@
  * @version:   version
  * @res1:  reserved
  * @res2:  reserved
- * @magic: Magic number
  * @res3:  reserved (will be used for additional RISC-V specific
  * header)
+ * @magic: Magic number
  * @res4:  reserved (will be used for PE COFF offset)
  *
  * The intention is for this header format to be shared between multiple
@@ -57,8 +58,8 @@ struct riscv_image_header {
u32 version;
u32 res1;
u64 res2;
-   u64 magic;
-   u32 res3;
+   u64 res3;
+   u32 magic;
u32 res4;
 };
 #endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 0f1ba17e476f..1f8fffbecf68 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -39,9 +39,8 @@ ENTRY(_start)
.word RISCV_HEADER_VERSION
.word 0
.dword 0
-   .asciz RISCV_IMAGE_MAGIC
-   .word 0
-   .balign 4
+   .dword 0
+   .ascii RISCV_IMAGE_MAGIC
.word 0
 
 .global _start_kernel
-- 
2.22.0



Re: [PATCH] arm: skip nomap memblocks while finding the lowmem/highmem boundary

2019-08-21 Thread Chester Lin
On Thu, Aug 22, 2019 at 11:45:34AM +0800, Chester Lin wrote:
> adjust_lowmem_bounds() checks every memblocks in order to find the boundary
> between lowmem and highmem. However some memblocks could be marked as NOMAP
> so they are not used by kernel, which should be skipped while calculating
> the boundary.
> 
> Signed-off-by: Chester Lin 
> ---
>  arch/arm/mm/mmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 426d9085396b..b86dba44d828 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1181,6 +1181,9 @@ void __init adjust_lowmem_bounds(void)
>   phys_addr_t block_start = reg->base;
>   phys_addr_t block_end = reg->base + reg->size;
>  
> + if (memblock_is_nomap(reg))
> + continue;
> +
>   if (reg->base < vmalloc_limit) {
>   if (block_end > lowmem_limit)
>   /*
> -- 
> 2.22.0
>

Hi Russell, Mike and Ard,

Per the discussion in the thread "[PATH] efi/arm: fix allocation failure ...",
(https://lkml.org/lkml/2019/8/21/163), I presume that the change to disregard
NOMAP memblocks in adjust_lowmem_bounds() should be separated as a single patch.

Please let me know if any suggestion, thank you.




[PATCH] arm: skip nomap memblocks while finding the lowmem/highmem boundary

2019-08-21 Thread Chester Lin
adjust_lowmem_bounds() checks every memblocks in order to find the boundary
between lowmem and highmem. However some memblocks could be marked as NOMAP
so they are not used by kernel, which should be skipped while calculating
the boundary.

Signed-off-by: Chester Lin 
---
 arch/arm/mm/mmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 426d9085396b..b86dba44d828 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1181,6 +1181,9 @@ void __init adjust_lowmem_bounds(void)
phys_addr_t block_start = reg->base;
phys_addr_t block_end = reg->base + reg->size;
 
+   if (memblock_is_nomap(reg))
+   continue;
+
if (reg->base < vmalloc_limit) {
if (block_end > lowmem_limit)
/*
-- 
2.22.0



Re: [PATCH v2 0/3] ACPI: New eject flow to remove devices cautiously

2019-08-05 Thread Chester Lin
On Wed, Jul 03, 2019 at 10:14:39AM +, Chester Lin wrote:
> Currently there are two ways to handle ACPI device ejection. When an eject
> event happens on a container, the kernel just sends KOBJ_CHANGE to
> userland and userland should handle offline operation. For other device
> types, acpi_scan_try_to_offline() is called and it tries to put target
> device(s) offline and then removes all nodes once they are all offline.
> 
> However we found that sometimes applications could intensively access
> resources on ejectable devices therefore they could have risk if ejection
> suddenly happens and removes devices without any notification. In stead
> of executing the offline callbakcs directly, we want to introduce a new
> approach, which sends change events to notify all target nodes beforehand
> and hands over offline handling to userland so that userland can have a
> chance to schedule an offline task based on current workload. The online
> function to recover from failure is also changed, it follows the same
> approach to send change events rather than putting devices online directly
> , which means userland will also need to take care of online handling.
> 
> To ensure that eject function can work properly since normal users might
> not have their own offline/online handling, we will submit a generic udev
> rule to systemd upstream as default in order to deal with change events
> and take [offline/online] action accordingly. But the Hot-Removing part
> still remains so the hotplug function can run to it once target nodes are
> all offline.
> 
> To easily monitor eject status and start over an eject process, there's a
> status trace mechanism in this eject flow, which helps to count current
> online devices under the ejectable target, and it can reschedule an eject
> event when all nodes within the device tree have been put offline.
> 
> v2:
> - device_sysfs: Add descriptions in /Document/ABI/testing/sysfs-bus-acpi
> - device_sysfs: Replace the declartion with DEVICE_ATTR_RW and add cancel
>   option in eject_store.
> - scan: Add a retry mechanism when userspace fail to put device offline.
> - scan: Add ready-to-remove state.
> 
> Chester Lin (3):
>   ACPI / hotplug: Send change events for offline/online requests when
> eject is triggered
>   ACPI / hotplug: Eject status trace and auto-remove approach
>   ACPI / device_sysfs: Add eject_show and add a cancel option in
> eject_store
> 
>  Documentation/ABI/testing/sysfs-bus-acpi |   9 +-
>  drivers/acpi/container.c |   2 +-
>  drivers/acpi/device_sysfs.c  |  94 ++-
>  drivers/acpi/glue.c  | 146 +++
>  drivers/acpi/internal.h  |  34 ++-
>  drivers/acpi/scan.c  | 318 +--
>  drivers/base/core.c  |   4 +
>  include/acpi/acpi_bus.h  |   3 +-
>  include/linux/acpi.h |   6 +
>  9 files changed, 523 insertions(+), 93 deletions(-)
>

Gentle ping. I will appreciate any comment on this series.

Thanks,
Chester


Re: [PATCH v2 0/3] ACPI: New eject flow to remove devices cautiously

2019-07-03 Thread Chester Lin
On Wed, Jul 03, 2019 at 10:14:39AM +, Chester Lin wrote:
> Currently there are two ways to handle ACPI device ejection. When an eject
> event happens on a container, the kernel just sends KOBJ_CHANGE to
> userland and userland should handle offline operation. For other device
> types, acpi_scan_try_to_offline() is called and it tries to put target
> device(s) offline and then removes all nodes once they are all offline.
> 
> However we found that sometimes applications could intensively access
> resources on ejectable devices therefore they could have risk if ejection
> suddenly happens and removes devices without any notification. In stead
> of executing the offline callbakcs directly, we want to introduce a new
> approach, which sends change events to notify all target nodes beforehand
> and hands over offline handling to userland so that userland can have a
> chance to schedule an offline task based on current workload. The online
> function to recover from failure is also changed, it follows the same
> approach to send change events rather than putting devices online directly
> , which means userland will also need to take care of online handling.
> 
> To ensure that eject function can work properly since normal users might
> not have their own offline/online handling, we will submit a generic udev
> rule to systemd upstream as default in order to deal with change events
> and take [offline/online] action accordingly. But the Hot-Removing part
> still remains so the hotplug function can run to it once target nodes are
> all offline.
> 


Here are default rules we are going to propose:

# 80-acpi-hotplug-eject.rules
# Generic rules for handling ACPI hotplug eject.

SUBSYSTEM=="*", ACTION=="change", ENV{EVENT}=="offline", ATTR{online}=="1", \
DEVPATH=="*", ATTR{online}="0"

SUBSYSTEM=="*", ACTION=="change", ENV{EVENT}=="online", ATTR{online}=="0", \
DEVPATH=="*", ATTR{online}="1"


> To easily monitor eject status and start over an eject process, there's a
> status trace mechanism in this eject flow, which helps to count current
> online devices under the ejectable target, and it can reschedule an eject
> event when all nodes within the device tree have been put offline.
> 
> v2:
> - device_sysfs: Add descriptions in /Document/ABI/testing/sysfs-bus-acpi
> - device_sysfs: Replace the declartion with DEVICE_ATTR_RW and add cancel
>   option in eject_store.
> - scan: Add a retry mechanism when userspace fail to put device offline.
> - scan: Add ready-to-remove state.
> 
> Chester Lin (3):
>   ACPI / hotplug: Send change events for offline/online requests when
> eject is triggered
>   ACPI / hotplug: Eject status trace and auto-remove approach
>   ACPI / device_sysfs: Add eject_show and add a cancel option in
> eject_store
> 
>  Documentation/ABI/testing/sysfs-bus-acpi |   9 +-
>  drivers/acpi/container.c |   2 +-
>  drivers/acpi/device_sysfs.c  |  94 ++-
>  drivers/acpi/glue.c  | 146 +++
>  drivers/acpi/internal.h  |  34 ++-
>  drivers/acpi/scan.c  | 318 +--
>  drivers/base/core.c  |   4 +
>  include/acpi/acpi_bus.h  |   3 +-
>  include/linux/acpi.h |   6 +
>  9 files changed, 523 insertions(+), 93 deletions(-)
> 
> -- 
> 2.20.1
> 


[PATCH v2 0/3] ACPI: New eject flow to remove devices cautiously

2019-07-03 Thread Chester Lin
Currently there are two ways to handle ACPI device ejection. When an eject
event happens on a container, the kernel just sends KOBJ_CHANGE to
userland and userland should handle offline operation. For other device
types, acpi_scan_try_to_offline() is called and it tries to put target
device(s) offline and then removes all nodes once they are all offline.

However we found that sometimes applications could intensively access
resources on ejectable devices therefore they could have risk if ejection
suddenly happens and removes devices without any notification. In stead
of executing the offline callbakcs directly, we want to introduce a new
approach, which sends change events to notify all target nodes beforehand
and hands over offline handling to userland so that userland can have a
chance to schedule an offline task based on current workload. The online
function to recover from failure is also changed, it follows the same
approach to send change events rather than putting devices online directly
, which means userland will also need to take care of online handling.

To ensure that eject function can work properly since normal users might
not have their own offline/online handling, we will submit a generic udev
rule to systemd upstream as default in order to deal with change events
and take [offline/online] action accordingly. But the Hot-Removing part
still remains so the hotplug function can run to it once target nodes are
all offline.

To easily monitor eject status and start over an eject process, there's a
status trace mechanism in this eject flow, which helps to count current
online devices under the ejectable target, and it can reschedule an eject
event when all nodes within the device tree have been put offline.

v2:
- device_sysfs: Add descriptions in /Document/ABI/testing/sysfs-bus-acpi
- device_sysfs: Replace the declartion with DEVICE_ATTR_RW and add cancel
  option in eject_store.
- scan: Add a retry mechanism when userspace fail to put device offline.
- scan: Add ready-to-remove state.

Chester Lin (3):
  ACPI / hotplug: Send change events for offline/online requests when
eject is triggered
  ACPI / hotplug: Eject status trace and auto-remove approach
  ACPI / device_sysfs: Add eject_show and add a cancel option in
eject_store

 Documentation/ABI/testing/sysfs-bus-acpi |   9 +-
 drivers/acpi/container.c |   2 +-
 drivers/acpi/device_sysfs.c  |  94 ++-
 drivers/acpi/glue.c  | 146 +++
 drivers/acpi/internal.h  |  34 ++-
 drivers/acpi/scan.c  | 318 +--
 drivers/base/core.c  |   4 +
 include/acpi/acpi_bus.h  |   3 +-
 include/linux/acpi.h |   6 +
 9 files changed, 523 insertions(+), 93 deletions(-)

-- 
2.20.1



[PATCH v2 3/3] ACPI / device_sysfs: Add eject_show and add a cancel option in eject_store

2019-07-03 Thread Chester Lin
Add an eject_show attribute for users to monitor current status because
sometimes it could take time to finish an ejection so we need to know
whether it is still in progress or not. For userspace who might need to
cancel an onging ejection, we also offer an option in eject_store.

Signed-off-by: Chester Lin 
---
 Documentation/ABI/testing/sysfs-bus-acpi |  9 ++-
 drivers/acpi/device_sysfs.c  | 94 +---
 drivers/acpi/internal.h  |  4 +-
 drivers/acpi/scan.c  | 38 +-
 4 files changed, 129 insertions(+), 16 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-acpi 
b/Documentation/ABI/testing/sysfs-bus-acpi
index e7898cfe5fb1..32fdf4af962e 100644
--- a/Documentation/ABI/testing/sysfs-bus-acpi
+++ b/Documentation/ABI/testing/sysfs-bus-acpi
@@ -53,9 +53,12 @@ What:/sys/bus/acpi/devices/.../eject
 Date:  December 2006
 Contact:   Rafael J. Wysocki 
 Description:
-   Writing 1 to this attribute will trigger hot removal of
-   this device object.  This file exists for every device
-   object that has _EJ0 method.
+   (R) Allows users to read eject status of the device object.
+   (W) Writing 1 to this attribute will trigger hot removal of
+   this device object. Writing 2 to this attribute will cancel hot
+   removal work if it's still in offline process and the original
+   state of this device object will be recovered. This file exists
+   for every device object that has _EJ0 method.
 
 What:  /sys/bus/acpi/devices/.../status
 Date:  Jan, 2014
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 96869f1538b9..6801b268fe9d 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -365,17 +365,13 @@ static ssize_t power_state_show(struct device *dev,
 
 static DEVICE_ATTR_RO(power_state);
 
-static ssize_t
-acpi_eject_store(struct device *d, struct device_attribute *attr,
-   const char *buf, size_t count)
+static ssize_t eject_show(struct device *d,
+   struct device_attribute *attr, char *buf)
 {
struct acpi_device *acpi_device = to_acpi_device(d);
acpi_object_type not_used;
acpi_status status;
 
-   if (!count || buf[0] != '1')
-   return -EINVAL;
-
if ((!acpi_device->handler || !acpi_device->handler->hotplug.enabled)
&& !acpi_device->driver)
return -ENODEV;
@@ -384,18 +380,96 @@ acpi_eject_store(struct device *d, struct 
device_attribute *attr,
if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
return -ENODEV;
 
+   return sprintf(buf, "%s\n", acpi_eject_status_string(acpi_device));
+}
+
+static ssize_t
+eject_store(struct device *d, struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct acpi_device *acpi_device = to_acpi_device(d);
+   struct eject_data *eject_node = NULL;
+   acpi_object_type not_used;
+   acpi_status status;
+   bool cancel_eject = false;
+   ssize_t ret;
+
+   if (!count)
+   return -EINVAL;
+
+   switch (buf[0]) {
+   case '1':
+   break;
+   case '2':
+   acpi_scan_lock_acquire();
+   eject_node = (struct eject_data *)acpi_device->eject_stat;
+
+   if (!eject_node) {
+   acpi_scan_lock_release();
+   ret = -EINVAL;
+   goto eject_end;
+   }
+
+   /*
+* Find a root to start cancellation from the top
+*/
+   if (eject_node->base.root_handle) {
+   acpi_device = acpi_bus_get_acpi_device(
+   eject_node->base.root_handle);
+
+   if (acpi_device)
+   eject_node =
+  (struct eject_data *)acpi_device->eject_stat;
+   else
+   eject_node = NULL;
+
+   }
+
+   if (eject_node &&
+  (eject_node->status == ACPI_EJECT_STATUS_GOING_OFFLINE ||
+   eject_node->status == ACPI_EJECT_STATUS_READY_REMOVE)) {
+   eject_node->status = ACPI_EJECT_STATUS_CANCEL;
+   cancel_eject = true;
+   }
+
+   acpi_scan_lock_release();
+   if (cancel_eject)
+   break;
+   default:
+   ret = -EINVAL;
+   goto eject_end;
+   };
+
+   if ((!acpi_device->handler || !acpi_device->handler->hotplug.enabled)
+   && !acpi_device->driver) {
+   ret = -ENODEV;
+   got

[PATCH v2 2/3] ACPI / hotplug: Eject status trace and auto-remove approach

2019-07-03 Thread Chester Lin
This is an eject-status trace mechanism which helps to count current online
devices under the ejection target, and it can automatically reschedules an
eject event when all related devices are offline. The number of online
nodes can be updated when any node has been put offline successfully.
Any online event or offline failure within the whole device tree during
ejection will stop the whole process and devices who have been put offline
will need be online again as recovery.

Signed-off-by: Chester Lin 
---
 drivers/acpi/glue.c | 146 ++
 drivers/acpi/internal.h |  30 
 drivers/acpi/scan.c | 152 +++-
 drivers/base/core.c |   4 ++
 include/acpi/acpi_bus.h |   1 +
 include/linux/acpi.h|   6 ++
 6 files changed, 336 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 36b24b0658cb..e1c419335128 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -360,6 +360,82 @@ static int acpi_device_notify_remove(struct device *dev)
return 0;
 }
 
+static void acpi_device_eject_tracer(struct device *dev,
+   enum kobject_action action)
+{
+   struct acpi_device *adev = ACPI_COMPANION(dev);
+   struct acpi_device *adev_root = NULL;
+   struct eject_data *eject_node = NULL;
+   struct eject_data *eject_root = NULL;
+
+   if (!adev)
+   return;
+
+   acpi_scan_lock_acquire();
+
+   eject_node = (struct eject_data *)adev->eject_stat;
+
+   if (!eject_node)
+   goto tracer_end;
+
+   if (eject_node->base.root_handle)
+   adev_root =
+   acpi_bus_get_acpi_device(eject_node->base.root_handle);
+
+   if (adev_root)
+   eject_root = (struct eject_data *)adev_root->eject_stat;
+
+   if (action == KOBJ_OFFLINE) {
+   eject_node->online_nodes--;
+
+   if (eject_node->online_nodes == 0)
+   eject_node->status = ACPI_EJECT_STATUS_READY_REMOVE;
+
+   /*
+* If the offline device is child, update its eject_root's
+* number of online nodes.
+*/
+   if (eject_root) {
+   eject_root->online_nodes--;
+
+   goto tracer_end;
+   }
+   /*
+* Adjust number of online nodes when any physical node under
+* eject_root is offline. Trigger acpi_eject_try() once all
+* nodes are offline.
+*/
+   if (eject_node->online_nodes == 0)
+   acpi_eject_retry(adev);
+
+   } else if (action == KOBJ_ONLINE) {
+   /* !eject_root means now eject_node is root. */
+   if (!eject_root)
+   eject_root = eject_node;
+
+   if (eject_root->status == ACPI_EJECT_STATUS_GOING_OFFLINE ||
+   eject_root->status == ACPI_EJECT_STATUS_READY_REMOVE) {
+   eject_root->status = ACPI_EJECT_STATUS_FAIL;
+   acpi_eject_retry(adev_root);
+   }
+   }
+tracer_end:
+   if (adev_root)
+   acpi_bus_put_acpi_device(adev_root);
+
+   acpi_scan_lock_release();
+}
+
+static inline void acpi_device_check_eject_status(struct device *dev)
+{
+   acpi_device_eject_tracer(dev, KOBJ_OFFLINE);
+}
+
+static inline void acpi_device_check_eject_recover(struct device *dev)
+{
+   acpi_device_eject_tracer(dev, KOBJ_ONLINE);
+}
+
 int acpi_platform_notify(struct device *dev, enum kobject_action action)
 {
switch (action) {
@@ -369,8 +445,78 @@ int acpi_platform_notify(struct device *dev, enum 
kobject_action action)
case KOBJ_REMOVE:
acpi_device_notify_remove(dev);
break;
+   case KOBJ_OFFLINE:
+   acpi_device_check_eject_status(dev);
+   break;
+   case KOBJ_ONLINE:
+   acpi_device_check_eject_recover(dev);
+   break;
default:
break;
}
return 0;
 }
+
+static void acpi_device_check_offline_fail(struct device *dev)
+{
+   struct acpi_device *adev = ACPI_COMPANION(dev);
+   struct acpi_device *adev_root = NULL;
+   struct acpi_device *adev_child = NULL;
+   struct eject_data *eject_node = NULL;
+   bool has_online_child = false;
+   char *envp[] = { "EVENT=offline", NULL };
+
+   if (!adev)
+   return;
+
+   acpi_scan_lock_acquire();
+
+   eject_node = (struct eject_data *)adev->eject_stat;
+
+   if (!eject_node ||
+   eject_node->status != ACPI_EJECT_STATUS_GOING_OFFLINE)
+   goto check_end;
+
+   if (!eject_node->retry) {
+   /* Check if the device contains any online child. */
+   

[PATCH v2 1/3] ACPI / hotplug: Send change events for offline/online requests when eject is triggered

2019-07-03 Thread Chester Lin
Here we change offline/online handling in device hotplug by sending change
events to userland as notification so that userland can have control and
determine when will be a good time to put them offline/online based on
current workload. In this approach the real offline/online opertions are
handed over to userland so that userland can have more time to prepare
before any device change actually happens.

All child devices under the ejection target are traversed and notified
hierarchically based on ACPI namespace in ascending order when an eject
event happens.

Signed-off-by: Chester Lin 
---
 drivers/acpi/container.c |   2 +-
 drivers/acpi/internal.h  |   2 +-
 drivers/acpi/scan.c  | 140 +--
 include/acpi/acpi_bus.h  |   2 +-
 4 files changed, 65 insertions(+), 81 deletions(-)

diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 9ea5f55d97e3..53ca9b1ae1bf 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -33,7 +33,7 @@ static int acpi_container_offline(struct container_dev *cdev)
 
/* Check all of the dependent devices' physical companions. */
list_for_each_entry(child, >children, node)
-   if (!acpi_scan_is_offline(child, false))
+   if (!acpi_scan_is_offline(child))
return -EBUSY;
 
return 0;
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index f6157d4d637a..656d237b073d 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -83,7 +83,7 @@ void acpi_apd_init(void);
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
 bool acpi_queue_hotplug_work(struct work_struct *work);
 void acpi_device_hotplug(struct acpi_device *adev, u32 src);
-bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
+bool acpi_scan_is_offline(struct acpi_device *adev);
 
 acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context);
 void acpi_scan_table_handler(u32 event, void *table, void *context);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0e28270b0fd8..d7628146eb5f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -113,11 +113,10 @@ int acpi_scan_add_handler_with_hotplug(struct 
acpi_scan_handler *handler,
return 0;
 }
 
-bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
+bool acpi_scan_is_offline(struct acpi_device *adev)
 {
struct acpi_device_physical_node *pn;
bool offline = true;
-   char *envp[] = { "EVENT=offline", NULL };
 
/*
 * acpi_container_offline() calls this for all of the container's
@@ -127,9 +126,6 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool 
uevent)
 
list_for_each_entry(pn, >physical_node_list, node)
if (device_supports_offline(pn->dev) && !pn->dev->offline) {
-   if (uevent)
-   kobject_uevent_env(>dev->kobj, KOBJ_CHANGE, 
envp);
-
offline = false;
break;
}
@@ -138,13 +134,12 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool 
uevent)
return offline;
 }
 
-static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
-   void **ret_p)
+static acpi_status acpi_bus_offline_notify(acpi_handle handle, u32 lvl,
+   void *data, void **ret_p)
 {
struct acpi_device *device = NULL;
struct acpi_device_physical_node *pn;
-   bool second_pass = (bool)data;
-   acpi_status status = AE_OK;
+   char *envp[] = { "EVENT=offline", NULL };
 
if (acpi_bus_get_device(handle, ))
return AE_OK;
@@ -155,100 +150,93 @@ static acpi_status acpi_bus_offline(acpi_handle handle, 
u32 lvl, void *data,
}
 
mutex_lock(>physical_node_lock);
-
list_for_each_entry(pn, >physical_node_list, node) {
-   int ret;
-
-   if (second_pass) {
-   /* Skip devices offlined by the first pass. */
-   if (pn->put_online)
-   continue;
-   } else {
-   pn->put_online = false;
-   }
-   ret = device_offline(pn->dev);
-   if (ret >= 0) {
-   pn->put_online = !ret;
-   } else {
-   *ret_p = pn->dev;
-   if (second_pass) {
-   status = AE_ERROR;
-   break;
-   }
+   if (device_supports_offline(pn->dev) && !(pn->dev->offline)) {
+   kobject_uevent_env(>dev->kobj, KOBJ_CHANGE, envp);
+   pn->changed_offline = true;
}
}
-
mutex_unlock(>physic

Re: [PATCH 3/3] ACPI / device_sysfs: Add eject show attr to monitor eject status

2019-06-02 Thread Chester Lin
On Fri, May 31, 2019 at 06:38:59AM -0700, Greg KH wrote:
> On Fri, May 31, 2019 at 02:56:42PM +0800, Chester Lin wrote:
> > An acpi_eject_show attribute for users to monitor current status because
> > sometimes it might take time to finish an ejection so we need to know
> > whether it is still in progress or not.
> > 
> > Signed-off-by: Chester Lin 
> > ---
> >  drivers/acpi/device_sysfs.c | 20 +++-
> >  drivers/acpi/internal.h |  1 +
> >  drivers/acpi/scan.c | 27 +++
> >  3 files changed, 47 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> > index 78c2653bf020..70b22eec6bbc 100644
> > --- a/drivers/acpi/device_sysfs.c
> > +++ b/drivers/acpi/device_sysfs.c
> > @@ -403,7 +403,25 @@ acpi_eject_store(struct device *d, struct 
> > device_attribute *attr,
> > return status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
> >  }
> >  
> > -static DEVICE_ATTR(eject, 0200, NULL, acpi_eject_store);
> > +static ssize_t acpi_eject_show(struct device *d,
> > +   struct device_attribute *attr, char *buf)
> > +{
> > +   struct acpi_device *acpi_device = to_acpi_device(d);
> > +   acpi_object_type not_used;
> > +   acpi_status status;
> > +
> > +   if ((!acpi_device->handler || !acpi_device->handler->hotplug.enabled)
> > +   && !acpi_device->driver)
> > +   return -ENODEV;
> > +
> > +   status = acpi_get_type(acpi_device->handle, _used);
> > +   if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
> > +   return -ENODEV;
> > +
> > +   return sprintf(buf, "%s\n", acpi_eject_status_string(acpi_device));
> > +}
> > +
> > +static DEVICE_ATTR(eject, 0644, acpi_eject_show, acpi_eject_store);
> 
> DEVICE_ATTR_RW()?
> 
> And you need to document the new sysfs file in Documentation/ABI/
> 
> thanks,
> 
> greg k-h
> 

Hi Greg,

Thank you for the reminder and I will fix these two in v2.

Regards,
Chester


[PATCH 0/3] ACPI: New eject flow to remove devices cautiously

2019-05-31 Thread Chester Lin
Currently there are two ways to handle ACPI device ejection. When an eject
event happens on a container, the kernel just sends KOBJ_CHANGE to
userland and userland should handle offline operation. For other device
types, acpi_scan_try_to_offline() is called and it tries to put target
device(s) offline and then removes all nodes once they are all offline.

However we found that sometimes applications could intensively access
resources on ejectable devices therefore they could have risk if ejection
suddenly happens and removes devices without any notification. In stead
of executing the offline callbakcs directly, we want to introduce a new
approach, which sends change events to notify all target nodes beforehand
and hands over offline handling to userland so that userland can have a
chance to schedule an offline task based on current workload. The online
function to recover from failure is also changed, it follows the same
approach to send change events rather than putting devices online directly
, which means userland will also need to take care of online handling.

To ensure that eject function can work properly since normal users might
not have their own offline/online handling, we will submit a generic udev
rule to systemd upstream as default in order to deal with change events
and take [offline/online] action accordingly. But the Hot-Removing part
still remains so the hotplug function can run to it once target nodes are
all offline.

To easily monitor eject status and start over an eject process, there's a
status trace mechanism in this eject flow, which helps to count current
online devices under the ejectable target, and it can reschedule an eject
event when all nodes within the device tree have been put offline.

Chester Lin (3):
  ACPI / hotplug: Send change events for offline/online requests when
eject is triggered
  ACPI / hotplug: Eject status trace and auto-remove approach
  ACPI / device_sysfs: Add eject show attr to monitor eject status

 drivers/acpi/container.c|   2 +-
 drivers/acpi/device_sysfs.c |  20 ++-
 drivers/acpi/glue.c |  81 ++
 drivers/acpi/internal.h |  31 +++-
 drivers/acpi/scan.c | 298 ++--
 drivers/base/core.c |   2 +
 include/acpi/acpi_bus.h |   3 +-
 7 files changed, 356 insertions(+), 81 deletions(-)

-- 
2.20.1



[PATCH 3/3] ACPI / device_sysfs: Add eject show attr to monitor eject status

2019-05-31 Thread Chester Lin
An acpi_eject_show attribute for users to monitor current status because
sometimes it might take time to finish an ejection so we need to know
whether it is still in progress or not.

Signed-off-by: Chester Lin 
---
 drivers/acpi/device_sysfs.c | 20 +++-
 drivers/acpi/internal.h |  1 +
 drivers/acpi/scan.c | 27 +++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 78c2653bf020..70b22eec6bbc 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -403,7 +403,25 @@ acpi_eject_store(struct device *d, struct device_attribute 
*attr,
return status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
 }
 
-static DEVICE_ATTR(eject, 0200, NULL, acpi_eject_store);
+static ssize_t acpi_eject_show(struct device *d,
+   struct device_attribute *attr, char *buf)
+{
+   struct acpi_device *acpi_device = to_acpi_device(d);
+   acpi_object_type not_used;
+   acpi_status status;
+
+   if ((!acpi_device->handler || !acpi_device->handler->hotplug.enabled)
+   && !acpi_device->driver)
+   return -ENODEV;
+
+   status = acpi_get_type(acpi_device->handle, _used);
+   if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
+   return -ENODEV;
+
+   return sprintf(buf, "%s\n", acpi_eject_status_string(acpi_device));
+}
+
+static DEVICE_ATTR(eject, 0644, acpi_eject_show, acpi_eject_store);
 
 static ssize_t
 acpi_device_hid_show(struct device *dev, struct device_attribute *attr, char 
*buf)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 0dcec4243b23..e7037d68c3d9 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -293,5 +293,6 @@ struct eject_data {
 };
 
 void acpi_eject_retry(struct acpi_device *adev);
+char *acpi_eject_status_string(struct acpi_device *adev);
 
 #endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index c60110db1cd6..27c4c1148879 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -370,6 +370,33 @@ static int acpi_scan_offline_check(struct acpi_device 
*device)
return ret;
 }
 
+char *acpi_eject_status_string(struct acpi_device *adev)
+{
+   struct eject_data *eject_obj;
+   char *status_string = "none";
+
+   mutex_lock(_scan_lock);
+   eject_obj = (struct eject_data *) adev->eject_stat;
+
+   if (eject_obj) {
+   switch (eject_obj->status) {
+   case ACPI_EJECT_STATUS_NONE:
+   break;
+   case ACPI_EJECT_STATUS_GOING_OFFLINE:
+   status_string = "going offline";
+   break;
+   case ACPI_EJECT_STATUS_FAIL:
+   status_string = "failure";
+   break;
+   default:
+   status_string = "(unknown)";
+   }
+   }
+
+   mutex_unlock(_scan_lock);
+   return status_string;
+}
+
 static int acpi_scan_hot_remove(struct acpi_device *device)
 {
acpi_handle handle = device->handle;
-- 
2.20.1



[PATCH 2/3] ACPI / hotplug: Eject status trace and auto-remove approach

2019-05-31 Thread Chester Lin
This is an eject-status trace mechanism which helps to count current online
devices under the ejection target, and it can automatically reschedules an
eject event when all related devices are offline. The number of online
nodes can be updated when any node has been put offline successfully.
Any online event within the whole device tree during ejection will stop the
whole process and devices who have been put offline will need be online
again as recovery.

Signed-off-by: Chester Lin 
---
 drivers/acpi/glue.c |  81 +++
 drivers/acpi/internal.h |  28 
 drivers/acpi/scan.c | 139 +++-
 drivers/base/core.c |   2 +
 include/acpi/acpi_bus.h |   1 +
 5 files changed, 248 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index edd10b3c7ec8..50745b12ae84 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -361,6 +361,81 @@ static int acpi_device_notify_remove(struct device *dev)
return 0;
 }
 
+static void acpi_device_eject_tracer(struct device *dev,
+   enum kobject_action action)
+{
+   struct acpi_device *adev = ACPI_COMPANION(dev);
+   struct acpi_device *adev_root = NULL;
+   struct eject_data *eject_node = NULL;
+   struct eject_data *eject_root = NULL;
+
+   if (!adev || !adev->eject_stat)
+   return;
+
+   acpi_scan_lock_acquire();
+
+   eject_node = (struct eject_data *)adev->eject_stat;
+
+   if (eject_node->base.root_handle)
+   adev_root =
+   acpi_bus_get_acpi_device(eject_node->base.root_handle);
+
+   if (adev_root)
+   eject_root = (struct eject_data *)adev_root->eject_stat;
+
+   if (action == KOBJ_OFFLINE) {
+   /*
+* If the offline device is child, update its eject_root's
+* number of online nodes.
+*/
+   if (eject_root) {
+   if (eject_root->online_nodes)
+   eject_root->online_nodes--;
+
+   if (eject_root->online_nodes <=
+   adev_root->physical_node_count)
+   acpi_eject_retry(adev_root);
+
+   goto tracer_end;
+   }
+   /*
+* Adjust number of online nodes when any physical node under
+* eject_root is offline. Trigger acpi_eject_try() once all
+* nodes are offline.
+*/
+   if (eject_node->online_nodes)
+   eject_node->online_nodes--;
+
+   if (!eject_node->online_nodes)
+   acpi_eject_retry(adev);
+
+   } else if (action == KOBJ_ONLINE) {
+   /* !eject_root means now eject_node is root */
+   if (!eject_root)
+   eject_root = eject_node;
+
+   if (eject_root->status == ACPI_EJECT_STATUS_GOING_OFFLINE) {
+   eject_root->status = ACPI_EJECT_STATUS_FAIL;
+   acpi_eject_retry(adev_root);
+   }
+   }
+tracer_end:
+   if (adev_root)
+   acpi_bus_put_acpi_device(adev_root);
+
+   acpi_scan_lock_release();
+}
+
+static inline void acpi_device_check_eject_status(struct device *dev)
+{
+   acpi_device_eject_tracer(dev, KOBJ_OFFLINE);
+}
+
+static inline void acpi_device_check_eject_recover(struct device *dev)
+{
+   acpi_device_eject_tracer(dev, KOBJ_ONLINE);
+}
+
 int acpi_platform_notify(struct device *dev, enum kobject_action action)
 {
switch (action) {
@@ -370,6 +445,12 @@ int acpi_platform_notify(struct device *dev, enum 
kobject_action action)
case KOBJ_REMOVE:
acpi_device_notify_remove(dev);
break;
+   case KOBJ_OFFLINE:
+   acpi_device_check_eject_status(dev);
+   break;
+   case KOBJ_ONLINE:
+   acpi_device_check_eject_recover(dev);
+   break;
default:
break;
}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 47014776fecb..0dcec4243b23 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -266,4 +266,32 @@ void acpi_init_lpit(void);
 static inline void acpi_init_lpit(void) { }
 #endif
 
+/* --
+ * Eject Status
+ * --
+ */
+enum acpi_eject_status {
+   ACPI_EJECT_STATUS_NONE = 0,
+   ACPI_EJECT_STATUS_GOING_OFFLINE,
+   ACPI_EJECT_STATUS_FAIL
+};
+
+enum acpi_eject_node_type {
+   ACPI_EJECT_CHILD_NODE = 0,
+   ACPI_EJECT_ROOT_NODE
+};
+
+struct eject_data_base {
+   enum acpi_eject_node_type type;
+   acpi_handle root_handle;
+};
+
+struc

[PATCH 1/3] ACPI / hotplug: Send change events for offline/online requests when eject is triggered

2019-05-31 Thread Chester Lin
Here we change offline/online handling in device hotplug by sending change
events to userland as notification so that userland can have control and
determine when will be a good time to put them offline/online based on
current workload. In this approach the real offline/online opertions are
handed over to userland so that userland can have more time to prepare
before any device change actually happens.

All child devices under the ejection target are traversed and notified
hierarchically based on ACPI namespace in ascending order when an eject
event happens.

Signed-off-by: Chester Lin 
---
 drivers/acpi/container.c |   2 +-
 drivers/acpi/internal.h  |   2 +-
 drivers/acpi/scan.c  | 140 +--
 include/acpi/acpi_bus.h  |   2 +-
 4 files changed, 65 insertions(+), 81 deletions(-)

diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 12c240903c18..f8c7768b4c8e 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -46,7 +46,7 @@ static int acpi_container_offline(struct container_dev *cdev)
 
/* Check all of the dependent devices' physical companions. */
list_for_each_entry(child, >children, node)
-   if (!acpi_scan_is_offline(child, false))
+   if (!acpi_scan_is_offline(child))
return -EBUSY;
 
return 0;
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 6eaf06db7752..47014776fecb 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -92,7 +92,7 @@ void acpi_apd_init(void);
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
 bool acpi_queue_hotplug_work(struct work_struct *work);
 void acpi_device_hotplug(struct acpi_device *adev, u32 src);
-bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
+bool acpi_scan_is_offline(struct acpi_device *adev);
 
 acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context);
 void acpi_scan_table_handler(u32 event, void *table, void *context);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0e28270b0fd8..d7628146eb5f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -113,11 +113,10 @@ int acpi_scan_add_handler_with_hotplug(struct 
acpi_scan_handler *handler,
return 0;
 }
 
-bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
+bool acpi_scan_is_offline(struct acpi_device *adev)
 {
struct acpi_device_physical_node *pn;
bool offline = true;
-   char *envp[] = { "EVENT=offline", NULL };
 
/*
 * acpi_container_offline() calls this for all of the container's
@@ -127,9 +126,6 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool 
uevent)
 
list_for_each_entry(pn, >physical_node_list, node)
if (device_supports_offline(pn->dev) && !pn->dev->offline) {
-   if (uevent)
-   kobject_uevent_env(>dev->kobj, KOBJ_CHANGE, 
envp);
-
offline = false;
break;
}
@@ -138,13 +134,12 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool 
uevent)
return offline;
 }
 
-static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
-   void **ret_p)
+static acpi_status acpi_bus_offline_notify(acpi_handle handle, u32 lvl,
+   void *data, void **ret_p)
 {
struct acpi_device *device = NULL;
struct acpi_device_physical_node *pn;
-   bool second_pass = (bool)data;
-   acpi_status status = AE_OK;
+   char *envp[] = { "EVENT=offline", NULL };
 
if (acpi_bus_get_device(handle, ))
return AE_OK;
@@ -155,100 +150,93 @@ static acpi_status acpi_bus_offline(acpi_handle handle, 
u32 lvl, void *data,
}
 
mutex_lock(>physical_node_lock);
-
list_for_each_entry(pn, >physical_node_list, node) {
-   int ret;
-
-   if (second_pass) {
-   /* Skip devices offlined by the first pass. */
-   if (pn->put_online)
-   continue;
-   } else {
-   pn->put_online = false;
-   }
-   ret = device_offline(pn->dev);
-   if (ret >= 0) {
-   pn->put_online = !ret;
-   } else {
-   *ret_p = pn->dev;
-   if (second_pass) {
-   status = AE_ERROR;
-   break;
-   }
+   if (device_supports_offline(pn->dev) && !(pn->dev->offline)) {
+   kobject_uevent_env(>dev->kobj, KOBJ_CHANGE, envp);
+   pn->changed_offline = true;
}
}
-
mutex_unlock(>physic