diff mbox

[05/16] efi: Add EFI_SECURE_BOOT bit

Message ID 147933287290.19316.3360403691390019935.stgit@warthog.procyon.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

David Howells Nov. 16, 2016, 9:47 p.m. UTC
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(+)


--
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

Comments

Ard Biesheuvel Nov. 17, 2016, 9:58 p.m. UTC | #1
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
Josh Boyer Nov. 18, 2016, 11:58 a.m. UTC | #2
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
Ard Biesheuvel Nov. 18, 2016, 12:10 p.m. UTC | #3
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
David Howells Nov. 18, 2016, 5:28 p.m. UTC | #4
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 mbox

Patch

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
 /*