| Message ID | 1542657371-7019-4-git-send-email-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | selftest/ima: fail kexec_load syscall | expand |
On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > The secure boot mode may not be detected on boot for some reason (eg. > buggy firmware). This patch attempts one more time to detect the > secure boot mode. Do we have cases where this has actually been seen? I'm not sure what the circumstances are that would result in this behaviour.
On Thu, Mar 7, 2019 at 2:38 PM Justin Forbes <jforbes@redhat.com> wrote: > On Thu, Mar 7, 2019 at 4:29 PM Matthew Garrett <mjg59@google.com> wrote: >> >> On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote: >> > >> > The secure boot mode may not be detected on boot for some reason (eg. >> > buggy firmware). This patch attempts one more time to detect the >> > secure boot mode. >> >> Do we have cases where this has actually been seen? I'm not sure what >> the circumstances are that would result in this behaviour. > > > We have never seen it in practice, though we only ever do anything with it with x86, so it is possible that some other platforms maybe? I'm not sure that it buys us anything to check this in both the boot stub and the running kernel. If a platform *is* giving us different results, anything else relying on the information from the boot stub is also going to be broken, so we should do this centrally rather than in the IMA code.
On Thu, 2019-03-07 at 14:44 -0800, Matthew Garrett wrote: > On Thu, Mar 7, 2019 at 2:38 PM Justin Forbes <jforbes@redhat.com> wrote: > > On Thu, Mar 7, 2019 at 4:29 PM Matthew Garrett <mjg59@google.com> wrote: > >> > >> On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > >> > > >> > The secure boot mode may not be detected on boot for some reason (eg. > >> > buggy firmware). This patch attempts one more time to detect the > >> > secure boot mode. > >> > >> Do we have cases where this has actually been seen? I'm not sure what > >> the circumstances are that would result in this behaviour. > > > > > > We have never seen it in practice, though we only ever do anything with it with x86, so it is possible that some other platforms maybe? > > I'm not sure that it buys us anything to check this in both the boot > stub and the running kernel. If a platform *is* giving us different > results, anything else relying on the information from the boot stub > is also going to be broken, so we should do this centrally rather than > in the IMA code. I added this last attempt because I'm seeing this on my laptop, with some older, buggy firmware. Mimi
On Thu, Mar 7, 2019 at 2:48 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > I added this last attempt because I'm seeing this on my laptop, with > some older, buggy firmware. Is the issue that it gives incorrect results on the first read, or is the issue that it gives incorrect results before ExitBootServices() is called? If the former then we should read twice in the boot stub, if the latter then we should figure out a way to do this immediately after ExitBootServices() instead.
On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote: > On Thu, Mar 7, 2019 at 2:48 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > I added this last attempt because I'm seeing this on my laptop, with > > some older, buggy firmware. > > Is the issue that it gives incorrect results on the first read, or is > the issue that it gives incorrect results before ExitBootServices() is > called? If the former then we should read twice in the boot stub, if > the latter then we should figure out a way to do this immediately > after ExitBootServices() instead. Detecting the secure boot mode isn't the problem. On boot, I am seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits "Secure boot could not be determined". In efi_main() the secure_boot mode is initially unset, so efi_get_secureboot() is called. efi_get_secureboot() returns the secure_boot mode correctly as enabled. The problem seems to be in saving the secure_boot mode for later use. Mimi
On Fri, Mar 8, 2019 at 5:40 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote: > > Is the issue that it gives incorrect results on the first read, or is > > the issue that it gives incorrect results before ExitBootServices() is > > called? If the former then we should read twice in the boot stub, if > > the latter then we should figure out a way to do this immediately > > after ExitBootServices() instead. > > Detecting the secure boot mode isn't the problem. On boot, I am > seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits > "Secure boot could not be determined". > > In efi_main() the secure_boot mode is initially unset, so > efi_get_secureboot() is called. efi_get_secureboot() returns the > secure_boot mode correctly as enabled. The problem seems to be in > saving the secure_boot mode for later use. Hm. And this only happens on certain firmware versions? If something's stepping on boot_params then we have bigger problems.
On Fri, 2019-03-08 at 09:51 -0800, Matthew Garrett wrote: > On Fri, Mar 8, 2019 at 5:40 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote: > > > Is the issue that it gives incorrect results on the first read, or is > > > the issue that it gives incorrect results before ExitBootServices() is > > > called? If the former then we should read twice in the boot stub, if > > > the latter then we should figure out a way to do this immediately > > > after ExitBootServices() instead. > > > > Detecting the secure boot mode isn't the problem. On boot, I am > > seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits > > "Secure boot could not be determined". > > > > In efi_main() the secure_boot mode is initially unset, so > > efi_get_secureboot() is called. efi_get_secureboot() returns the > > secure_boot mode correctly as enabled. The problem seems to be in > > saving the secure_boot mode for later use. > > Hm. And this only happens on certain firmware versions? If something's > stepping on boot_params then we have bigger problems. FYI, efi_printk() works before exit_boot(), but not afterwards. The system hangs. Mimi
On Fri, Mar 8, 2019 at 10:43 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > FYI, efi_printk() works before exit_boot(), but not afterwards. The > system hangs. efi_printk() uses boot services to print, so that's not unexpected :) It would probably be sensible to return an error rather than crash, though…
On Fri, 2019-03-08 at 09:51 -0800, Matthew Garrett wrote: > On Fri, Mar 8, 2019 at 5:40 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote: > > > Is the issue that it gives incorrect results on the first read, or is > > > the issue that it gives incorrect results before ExitBootServices() is > > > called? If the former then we should read twice in the boot stub, if > > > the latter then we should figure out a way to do this immediately > > > after ExitBootServices() instead. > > > > Detecting the secure boot mode isn't the problem. On boot, I am > > seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits > > "Secure boot could not be determined". > > > > In efi_main() the secure_boot mode is initially unset, so > > efi_get_secureboot() is called. efi_get_secureboot() returns the > > secure_boot mode correctly as enabled. The problem seems to be in > > saving the secure_boot mode for later use. > > Hm. And this only happens on certain firmware versions? If something's > stepping on boot_params then we have bigger problems. I was seeing this problem before and after updating the system firmware on my laptop last summer. If updating the firmware had resolved the problem, I wouldn't have included this patch. Mimi
On Mon, Mar 11, 2019 at 9:55 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Fri, 2019-03-08 at 09:51 -0800, Matthew Garrett wrote: > > Hm. And this only happens on certain firmware versions? If something's > > stepping on boot_params then we have bigger problems. > > I was seeing this problem before and after updating the system > firmware on my laptop last summer. If updating the firmware had > resolved the problem, I wouldn't have included this patch. Ah, sorry, when you said that you saw this with older firmware I thought that meant that newer firmware fixed it. What machine are you testing on?
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index f0910a1e1db7..eb51b0e1189c 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -151,4 +151,6 @@ ifeq ($(CONFIG_X86_64),y) obj-y += vsmp_64.o endif +ifdef CONFIG_EFI obj-$(CONFIG_IMA) += ima_arch.o +endif diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c index 6c248616ee57..e47cd9390ab4 100644 --- a/arch/x86/kernel/ima_arch.c +++ b/arch/x86/kernel/ima_arch.c @@ -7,10 +7,52 @@ extern struct boot_params boot_params; +static enum efi_secureboot_mode get_sb_mode(void) +{ + efi_char16_t efi_SecureBoot_name[] = L"SecureBoot"; + efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID; + efi_status_t status; + unsigned long size; + u8 secboot; + + size = sizeof(secboot); + + /* Get variable contents into buffer */ + status = efi.get_variable(efi_SecureBoot_name, &efi_variable_guid, + NULL, &size, &secboot); + 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; + } + + if (secboot == 0) { + 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) { - if (efi_enabled(EFI_BOOT) && - (boot_params.secure_boot == efi_secureboot_mode_enabled)) + 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; diff --git a/include/linux/ima.h b/include/linux/ima.h index 5ab9134d4fd7..b5e16b8c50b7 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -30,7 +30,7 @@ extern void ima_post_path_mknod(struct dentry *dentry); extern void ima_add_kexec_buffer(struct kimage *image); #endif -#ifdef CONFIG_X86 +#if defined(CONFIG_X86) && defined(CONFIG_EFI) extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else
The secure boot mode may not be detected on boot for some reason (eg. buggy firmware). This patch attempts one more time to detect the secure boot mode. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- arch/x86/kernel/Makefile | 2 ++ arch/x86/kernel/ima_arch.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- include/linux/ima.h | 2 +- 3 files changed, 47 insertions(+), 3 deletions(-)