Message ID | 1487268503-21570-1-git-send-email-ard.biesheuvel@linaro.org |
---|---|
State | Superseded |
Headers | show |
On Thu, Feb 16, 2017 at 06:08:23PM +0000, Ard Biesheuvel wrote: > The newly refactored code that infers the firmware's Secure Boot state > prints the following error when the variables 'SecureBoot' is missing. > > EFI stub: ERROR: Could not determine UEFI Secure Boot status. > > However, this variable is only guaranteed to be defined on a system that > is Secure Boot capable to begin with, and so it is not an error if it is > missing. So report Secure Boot as being disabled in this case, without > printing any error messages. In fact I pointed out this change of behaviour on ARM during the review process: https://lkml.org/lkml/2016/12/8/702 But David thought otherwise. :-( Best regards, Lukas > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > v2: treat SecureBoot present but SetupMode missing as 'unknown' not 'disabled' > > drivers/firmware/efi/libstub/secureboot.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c > index 766ac06dac84..8fa8af4b3ca8 100644 > --- a/drivers/firmware/efi/libstub/secureboot.c > +++ b/drivers/firmware/efi/libstub/secureboot.c > @@ -47,6 +47,8 @@ enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg) > size = sizeof(secboot); > status = get_efi_var(efi_SecureBoot_name, &efi_variable_guid, > NULL, &size, &secboot); > + if (status == EFI_NOT_FOUND) > + return efi_secureboot_mode_disabled; > if (status != EFI_SUCCESS) > goto out_efi_err; > > @@ -80,7 +82,5 @@ enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg) > > out_efi_err: > pr_efi_err(sys_table_arg, "Could not determine UEFI Secure Boot status.\n"); > - if (status == EFI_NOT_FOUND) > - return efi_secureboot_mode_disabled; > return efi_secureboot_mode_unknown; > } > -- > 2.7.4 -- 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
On 17 February 2017 at 05:13, Lukas Wunner <lukas@wunner.de> wrote: > On Thu, Feb 16, 2017 at 06:08:23PM +0000, Ard Biesheuvel wrote: >> The newly refactored code that infers the firmware's Secure Boot state >> prints the following error when the variables 'SecureBoot' is missing. >> >> EFI stub: ERROR: Could not determine UEFI Secure Boot status. >> >> However, this variable is only guaranteed to be defined on a system that >> is Secure Boot capable to begin with, and so it is not an error if it is >> missing. So report Secure Boot as being disabled in this case, without >> printing any error messages. > > In fact I pointed out this change of behaviour on ARM during the > review process: > > https://lkml.org/lkml/2016/12/8/702 > > But David thought otherwise. :-( > I do remember that discussion. But I think David catered for that by returning enabled/disabled/unknown, deferring the decision how to deal with 'unknown' to the caller. But I did not appreciate at the time that this was a change in behavior nonetheless, and printing errors that are not errors only confuses people. -- 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
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > But I did not appreciate at the time that this was a change in > behavior nonetheless, and printing errors that are not errors only > confuses people. Perhaps just take the error message out and leave it to the main kernel to deal with? David -- 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
> On 17 Feb 2017, at 08:56, David Howells <dhowells@redhat.com> wrote: > > Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> But I did not appreciate at the time that this was a change in >> behavior nonetheless, and printing errors that are not errors only >> confuses people. > > Perhaps just take the error message out and leave it to the main kernel to > deal with? > No, a non-existent SecureBoot variable is simply not an error condition -- 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
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c index 766ac06dac84..8fa8af4b3ca8 100644 --- a/drivers/firmware/efi/libstub/secureboot.c +++ b/drivers/firmware/efi/libstub/secureboot.c @@ -47,6 +47,8 @@ enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg) size = sizeof(secboot); status = get_efi_var(efi_SecureBoot_name, &efi_variable_guid, NULL, &size, &secboot); + if (status == EFI_NOT_FOUND) + return efi_secureboot_mode_disabled; if (status != EFI_SUCCESS) goto out_efi_err; @@ -80,7 +82,5 @@ enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg) out_efi_err: pr_efi_err(sys_table_arg, "Could not determine UEFI Secure Boot status.\n"); - if (status == EFI_NOT_FOUND) - return efi_secureboot_mode_disabled; return efi_secureboot_mode_unknown; }
The newly refactored code that infers the firmware's Secure Boot state prints the following error when the variables 'SecureBoot' is missing. EFI stub: ERROR: Could not determine UEFI Secure Boot status. However, this variable is only guaranteed to be defined on a system that is Secure Boot capable to begin with, and so it is not an error if it is missing. So report Secure Boot as being disabled in this case, without printing any error messages. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- v2: treat SecureBoot present but SetupMode missing as 'unknown' not 'disabled' drivers/firmware/efi/libstub/secureboot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.7.4 -- 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