From ae9f12058d71d9c5971c3cf36191cd813ecc9554 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 3 Sep 2019 17:08:45 +0200 Subject: ArmVirtPkg/PlatformBootManagerLib: unload image on EFI_SECURITY_VIOLATION The LoadImage() boot service is a bit unusual in that it allocates resources in a particular failure case; namely, it produces a valid "ImageHandle" when it returns EFI_SECURITY_VIOLATION. This is supposed to happen e.g. when Secure Boot verification fails for the image, but the platform policy for the particular image origin (such as "fixed media" or "removable media") is DEFER_EXECUTE_ON_SECURITY_VIOLATION. The return code allows platform logic to selectively override the verification failure, and launch the image nonetheless. ArmVirtPkg/PlatformBootManagerLib does not override EFI_SECURITY_VIOLATION for the kernel image loaded from fw_cfg -- any LoadImage() error is considered fatal. When we simply treat EFI_SECURITY_VIOLATION like any other LoadImage() error, we leak the resources associated with "KernelImageHandle". From a resource usage perspective, EFI_SECURITY_VIOLATION must be considered "success", and rolled back. Implement this rollback, without breaking the proper "nesting" of error handling jumps and labels. Cc: Ard Biesheuvel Cc: Dandan Bi Cc: Leif Lindholm Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1992 Fixes: 23d04b58e27b382bbd3f9b16ba9adb1cb203dad5 Signed-off-by: Laszlo Ersek Acked-by: Ard Biesheuvel Reviewed-by: Dandan Bi Reviewed-by: Philippe Mathieu-Daude --- ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'ArmVirtPkg/Library') diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c index 3cc83e3b7b..d3851fd75f 100644 --- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c @@ -991,7 +991,14 @@ TryRunningQemuKernel ( ); if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "%a: LoadImage(): %r\n", __FUNCTION__, Status)); - goto FreeKernelDevicePath; + if (Status != EFI_SECURITY_VIOLATION) { + goto FreeKernelDevicePath; + } + // + // From the resource allocation perspective, EFI_SECURITY_VIOLATION means + // "success", so we must roll back the image loading. + // + goto UnloadKernelImage; } // -- cgit v1.2.3