* [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage()
@ 2020-06-05 23:52 Laszlo Ersek
2020-06-07 22:09 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Laszlo Ersek @ 2020-06-05 23:52 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Philippe Mathieu-Daudé
When an image fails Secure Boot validation, LoadImage() returns
EFI_SECURITY_VIOLATION if the platform policy is
DEFER_EXECUTE_ON_SECURITY_VIOLATION.
If the platform policy is DENY_EXECUTE_ON_SECURITY_VIOLATION, then
LoadImage() returns EFI_ACCESS_DENIED (and the image does not remain
loaded).
(Before <https://bugzilla.tianocore.org/show_bug.cgi?id=2129>, this
difference would be masked, as DxeImageVerificationLib would incorrectly
return EFI_SECURITY_VIOLATION for DENY_EXECUTE_ON_SECURITY_VIOLATION as
well.)
In X86QemuLoadImageLib, proceed to the legacy Linux/x86 Boot Protocol upon
seeing EFI_ACCESS_DENIED too.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2785
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
Repo: https://pagure.io/lersek/edk2.git
Branch: x86_qlil_access_denied
OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
index ef753be7ea90..931553c0c1fb 100644
--- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
+++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
@@ -319,13 +319,19 @@ QemuLoadKernelImage (
return EFI_NOT_FOUND;
case EFI_SECURITY_VIOLATION:
+ //
+ // Since the image has been loaded, we need to unload it before proceeding
+ // to the EFI_ACCESS_DENIED case below.
+ //
+ gBS->UnloadImage (KernelImageHandle);
+ //
+ // Fall through
+ //
+ case EFI_ACCESS_DENIED:
//
// We are running with UEFI secure boot enabled, and the image failed to
// authenticate. For compatibility reasons, we fall back to the legacy
- // loader in this case. Since the image has been loaded, we need to unload
- // it before proceeding
- //
- gBS->UnloadImage (KernelImageHandle);
+ // loader in this case.
//
// Fall through
//
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage()
2020-06-05 23:52 [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage() Laszlo Ersek
@ 2020-06-07 22:09 ` Philippe Mathieu-Daudé
2020-06-08 8:15 ` Ard Biesheuvel
2020-06-09 20:20 ` [edk2-devel] " Laszlo Ersek
2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-07 22:09 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io; +Cc: Ard Biesheuvel, Jordan Justen
On 6/6/20 1:52 AM, Laszlo Ersek wrote:
> When an image fails Secure Boot validation, LoadImage() returns
> EFI_SECURITY_VIOLATION if the platform policy is
> DEFER_EXECUTE_ON_SECURITY_VIOLATION.
>
> If the platform policy is DENY_EXECUTE_ON_SECURITY_VIOLATION, then
> LoadImage() returns EFI_ACCESS_DENIED (and the image does not remain
> loaded).
>
> (Before <https://bugzilla.tianocore.org/show_bug.cgi?id=2129>, this
> difference would be masked, as DxeImageVerificationLib would incorrectly
> return EFI_SECURITY_VIOLATION for DENY_EXECUTE_ON_SECURITY_VIOLATION as
> well.)
>
> In X86QemuLoadImageLib, proceed to the legacy Linux/x86 Boot Protocol upon
> seeing EFI_ACCESS_DENIED too.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2785
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Repo: https://pagure.io/lersek/edk2.git
> Branch: x86_qlil_access_denied
>
> OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> index ef753be7ea90..931553c0c1fb 100644
> --- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> +++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> @@ -319,13 +319,19 @@ QemuLoadKernelImage (
> return EFI_NOT_FOUND;
>
> case EFI_SECURITY_VIOLATION:
> + //
> + // Since the image has been loaded, we need to unload it before proceeding
> + // to the EFI_ACCESS_DENIED case below.
> + //
> + gBS->UnloadImage (KernelImageHandle);
> + //
> + // Fall through
Nice catch.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> + //
> + case EFI_ACCESS_DENIED:
> //
> // We are running with UEFI secure boot enabled, and the image failed to
> // authenticate. For compatibility reasons, we fall back to the legacy
> - // loader in this case. Since the image has been loaded, we need to unload
> - // it before proceeding
> - //
> - gBS->UnloadImage (KernelImageHandle);
> + // loader in this case.
> //
> // Fall through
> //
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage()
2020-06-05 23:52 [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage() Laszlo Ersek
2020-06-07 22:09 ` Philippe Mathieu-Daudé
@ 2020-06-08 8:15 ` Ard Biesheuvel
2020-06-09 20:20 ` [edk2-devel] " Laszlo Ersek
2 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2020-06-08 8:15 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Jordan Justen, Philippe Mathieu-Daudé
On 6/6/20 1:52 AM, Laszlo Ersek wrote:
> When an image fails Secure Boot validation, LoadImage() returns
> EFI_SECURITY_VIOLATION if the platform policy is
> DEFER_EXECUTE_ON_SECURITY_VIOLATION.
>
> If the platform policy is DENY_EXECUTE_ON_SECURITY_VIOLATION, then
> LoadImage() returns EFI_ACCESS_DENIED (and the image does not remain
> loaded).
>
> (Before <https://bugzilla.tianocore.org/show_bug.cgi?id=2129>, this
> difference would be masked, as DxeImageVerificationLib would incorrectly
> return EFI_SECURITY_VIOLATION for DENY_EXECUTE_ON_SECURITY_VIOLATION as
> well.)
>
> In X86QemuLoadImageLib, proceed to the legacy Linux/x86 Boot Protocol upon
> seeing EFI_ACCESS_DENIED too.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2785
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>
> Notes:
> Repo: https://pagure.io/lersek/edk2.git
> Branch: x86_qlil_access_denied
>
> OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> index ef753be7ea90..931553c0c1fb 100644
> --- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> +++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> @@ -319,13 +319,19 @@ QemuLoadKernelImage (
> return EFI_NOT_FOUND;
>
> case EFI_SECURITY_VIOLATION:
> + //
> + // Since the image has been loaded, we need to unload it before proceeding
> + // to the EFI_ACCESS_DENIED case below.
> + //
> + gBS->UnloadImage (KernelImageHandle);
> + //
> + // Fall through
> + //
> + case EFI_ACCESS_DENIED:
> //
> // We are running with UEFI secure boot enabled, and the image failed to
> // authenticate. For compatibility reasons, we fall back to the legacy
> - // loader in this case. Since the image has been loaded, we need to unload
> - // it before proceeding
> - //
> - gBS->UnloadImage (KernelImageHandle);
> + // loader in this case.
> //
> // Fall through
> //
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage()
2020-06-05 23:52 [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage() Laszlo Ersek
2020-06-07 22:09 ` Philippe Mathieu-Daudé
2020-06-08 8:15 ` Ard Biesheuvel
@ 2020-06-09 20:20 ` Laszlo Ersek
2 siblings, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2020-06-09 20:20 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Philippe Mathieu-Daudé
On 06/06/20 01:52, Laszlo Ersek wrote:
> When an image fails Secure Boot validation, LoadImage() returns
> EFI_SECURITY_VIOLATION if the platform policy is
> DEFER_EXECUTE_ON_SECURITY_VIOLATION.
>
> If the platform policy is DENY_EXECUTE_ON_SECURITY_VIOLATION, then
> LoadImage() returns EFI_ACCESS_DENIED (and the image does not remain
> loaded).
>
> (Before <https://bugzilla.tianocore.org/show_bug.cgi?id=2129>, this
> difference would be masked, as DxeImageVerificationLib would incorrectly
> return EFI_SECURITY_VIOLATION for DENY_EXECUTE_ON_SECURITY_VIOLATION as
> well.)
>
> In X86QemuLoadImageLib, proceed to the legacy Linux/x86 Boot Protocol upon
> seeing EFI_ACCESS_DENIED too.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2785
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Repo: https://pagure.io/lersek/edk2.git
> Branch: x86_qlil_access_denied
>
> OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> index ef753be7ea90..931553c0c1fb 100644
> --- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> +++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> @@ -319,13 +319,19 @@ QemuLoadKernelImage (
> return EFI_NOT_FOUND;
>
> case EFI_SECURITY_VIOLATION:
> + //
> + // Since the image has been loaded, we need to unload it before proceeding
> + // to the EFI_ACCESS_DENIED case below.
> + //
> + gBS->UnloadImage (KernelImageHandle);
> + //
> + // Fall through
> + //
> + case EFI_ACCESS_DENIED:
> //
> // We are running with UEFI secure boot enabled, and the image failed to
> // authenticate. For compatibility reasons, we fall back to the legacy
> - // loader in this case. Since the image has been loaded, we need to unload
> - // it before proceeding
> - //
> - gBS->UnloadImage (KernelImageHandle);
> + // loader in this case.
> //
> // Fall through
> //
>
Merged as commit dafce295e6f4 via
<https://github.com/tianocore/edk2/pull/671>.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-09 20:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-05 23:52 [PATCH] OvmfPkg/X86QemuLoadImageLib: handle EFI_ACCESS_DENIED from LoadImage() Laszlo Ersek
2020-06-07 22:09 ` Philippe Mathieu-Daudé
2020-06-08 8:15 ` Ard Biesheuvel
2020-06-09 20:20 ` [edk2-devel] " Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox