Message ID | 147933287290.19316.3360403691390019935.stgit@warthog.procyon.org.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 16 November 2016 at 21:47, David Howells <dhowells@redhat.com> wrote: > From: Josh Boyer <jwboyer@fedoraproject.org> > > UEFI machines can be booted in Secure Boot mode. Add a EFI_SECURE_BOOT bit > for use with efi_enabled. > > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > Signed-off-by: David Howells <dhowells@redhat.com> > --- > > arch/x86/kernel/setup.c | 1 + > include/linux/efi.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 9521acce8378..539f29587712 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -1164,6 +1164,7 @@ void __init setup_arch(char **cmdline_p) > if (boot_params.secure_boot && > IS_ENABLED(CONFIG_EFI_SECURE_BOOT_LOCK_DOWN)) { > lock_kernel_down(); > + set_bit(EFI_SECURE_BOOT, &efi.flags); Why is this x86 only? And why is this bit only set if CONFIG_EFI_SECURE_BOOT_LOCK_DOWN is enabled? > pr_info("Secure boot enabled\n"); > } > > diff --git a/include/linux/efi.h b/include/linux/efi.h > index 11372fb8784c..5d7fb3e3400b 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -1079,6 +1079,7 @@ extern int __init efi_setup_pcdp_console(char *); > #define EFI_ARCH_1 7 /* First arch-specific bit */ > #define EFI_DBG 8 /* Print additional debug info at runtime */ > #define EFI_NX_PE_DATA 9 /* Can runtime data regions be mapped non-executable? */ > +#define EFI_SECURE_BOOT 10 /* Are we in Secure Boot mode? */ > > #ifdef CONFIG_EFI > /* > > -- > To unsubscribe from this list: send the line "unsubscribe linux-efi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 17, 2016 at 4:58 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 16 November 2016 at 21:47, David Howells <dhowells@redhat.com> wrote: >> From: Josh Boyer <jwboyer@fedoraproject.org> >> >> UEFI machines can be booted in Secure Boot mode. Add a EFI_SECURE_BOOT bit >> for use with efi_enabled. >> >> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> >> Signed-off-by: David Howells <dhowells@redhat.com> >> --- >> >> arch/x86/kernel/setup.c | 1 + >> include/linux/efi.h | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >> index 9521acce8378..539f29587712 100644 >> --- a/arch/x86/kernel/setup.c >> +++ b/arch/x86/kernel/setup.c >> @@ -1164,6 +1164,7 @@ void __init setup_arch(char **cmdline_p) >> if (boot_params.secure_boot && >> IS_ENABLED(CONFIG_EFI_SECURE_BOOT_LOCK_DOWN)) { >> lock_kernel_down(); >> + set_bit(EFI_SECURE_BOOT, &efi.flags); > > Why is this x86 only? And why is this bit only set if Because it was initially written like 3 years ago before ARM even had UEFI. Needs a refresh. > CONFIG_EFI_SECURE_BOOT_LOCK_DOWN is enabled? That part is new and something David added. Probably not necessary. josh -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18 November 2016 at 11:58, Josh Boyer <jwboyer@fedoraproject.org> wrote: > On Thu, Nov 17, 2016 at 4:58 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> On 16 November 2016 at 21:47, David Howells <dhowells@redhat.com> wrote: >>> From: Josh Boyer <jwboyer@fedoraproject.org> >>> >>> UEFI machines can be booted in Secure Boot mode. Add a EFI_SECURE_BOOT bit >>> for use with efi_enabled. >>> >>> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> >>> Signed-off-by: David Howells <dhowells@redhat.com> >>> --- >>> >>> arch/x86/kernel/setup.c | 1 + >>> include/linux/efi.h | 1 + >>> 2 files changed, 2 insertions(+) >>> >>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >>> index 9521acce8378..539f29587712 100644 >>> --- a/arch/x86/kernel/setup.c >>> +++ b/arch/x86/kernel/setup.c >>> @@ -1164,6 +1164,7 @@ void __init setup_arch(char **cmdline_p) >>> if (boot_params.secure_boot && >>> IS_ENABLED(CONFIG_EFI_SECURE_BOOT_LOCK_DOWN)) { >>> lock_kernel_down(); >>> + set_bit(EFI_SECURE_BOOT, &efi.flags); >> >> Why is this x86 only? And why is this bit only set if > > Because it was initially written like 3 years ago before ARM even had > UEFI. Needs a refresh. > Ah ok. I missed that part. In any case, we have been working very hard over the past couple of years to move all the UEFI stuff out of arch/x86, except for the pieces that *really* belong there. For this series, that means that a fair share of the changes will need to be reworked and moved under drivers/firmware/efi. Note that that also means you cannot use L"" string literals anymore, since arm64's UEFI stub is linked into the kernel proper, and the wide character formats are incompatible between UEFI and the wide char handling that occurs under fs/. Please check the existing secureboot_enabled() function Lukas referred to as an example how to emit wide string literals instead. >> CONFIG_EFI_SECURE_BOOT_LOCK_DOWN is enabled? > > That part is new and something David added. Probably not necessary. > Regardless of anything else, think is is useful to have a EFI_xx flag that is always set when secure boot is enabled. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > @@ -1164,6 +1164,7 @@ void __init setup_arch(char **cmdline_p) > > if (boot_params.secure_boot && > > IS_ENABLED(CONFIG_EFI_SECURE_BOOT_LOCK_DOWN)) { > > lock_kernel_down(); > > + set_bit(EFI_SECURE_BOOT, &efi.flags); > > Why is this x86 only? It probably doesn't really need to be, but that's what the patches I ported do. > And why is this bit only set if CONFIG_EFI_SECURE_BOOT_LOCK_DOWN is enabled? Actually, the EFI_SECURE_BOOT bit should probably be set outside of that portion of the if-condition. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 9521acce8378..539f29587712 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -1164,6 +1164,7 @@ void __init setup_arch(char **cmdline_p) if (boot_params.secure_boot && IS_ENABLED(CONFIG_EFI_SECURE_BOOT_LOCK_DOWN)) { lock_kernel_down(); + set_bit(EFI_SECURE_BOOT, &efi.flags); pr_info("Secure boot enabled\n"); } diff --git a/include/linux/efi.h b/include/linux/efi.h index 11372fb8784c..5d7fb3e3400b 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1079,6 +1079,7 @@ extern int __init efi_setup_pcdp_console(char *); #define EFI_ARCH_1 7 /* First arch-specific bit */ #define EFI_DBG 8 /* Print additional debug info at runtime */ #define EFI_NX_PE_DATA 9 /* Can runtime data regions be mapped non-executable? */ +#define EFI_SECURE_BOOT 10 /* Are we in Secure Boot mode? */ #ifdef CONFIG_EFI /*