[efi] Work around UEFI specification bug in LoadImage

iPXE currently assumes that any error returned from LoadImage()
indicates that the image was not loaded.  This assumption was correct
at the time the code was written and remained correct for UEFI
specifications up to and including version 2.1.

In version 2.3, the UEFI specification broke API and ABI compatibility
by defining that a return value of EFI_SECURITY_VIOLATION would now
indicate that the image had been loaded and a valid image handle had
been created, but that the image should not be started.

The wording in version 2.2 is ambiguous, and does not define whether
or not a return value of EFI_SECURITY_VIOLATION indicates that a valid
image handle has been created.

Attempt to work around all of these incompatible and partially
undefined APIs by calling UnloadImage if we get a return value of
EFI_SECURITY_VIOLATION.  Minimise the risk of passing an uninitialised
pointer to UnloadImage by setting ImageHandle to NULL prior to calling
LoadImage.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 18 insertions, 2 deletions

diff --git a/src/image/efi_image.c b/src/image/efi_image.c
index 47580c0..a112a8f 100644
--- a/src/image/efi_image.c
+++ b/src/image/efi_image.c
@@ -193,6 +193,7 @@ static int efi_image_exec ( struct image *image ) {
 	}
 
 	/* Attempt loading image */
+	handle = NULL;
 	if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, path,
 				       user_to_virt ( image->data, 0 ),
 				       image->len, &handle ) ) != 0 ) {
@@ -200,7 +201,11 @@ static int efi_image_exec ( struct image *image ) {
 		rc = -EEFI_LOAD ( efirc );
 		DBGC ( image, "EFIIMAGE %p could not load: %s\n",
 		       image, strerror ( rc ) );
-		goto err_load_image;
+		if ( efirc == EFI_SECURITY_VIOLATION ) {
+			goto err_load_image_security_violation;
+		} else {
+			goto err_load_image;
+		}
 	}
 
 	/* Get the loaded image protocol for the newly loaded image */
@@ -268,6 +273,7 @@ static int efi_image_exec ( struct image *image ) {
 	 * call UnloadImage()).  We therefore ignore any failures from
 	 * the UnloadImage() call itself.
 	 */
+ err_load_image_security_violation:
 	if ( rc != 0 )
 		bs->UnloadImage ( handle );
  err_load_image:
@@ -303,6 +309,7 @@ static int efi_image_probe ( struct image *image ) {
 	int rc;
 
 	/* Attempt loading image */
+	handle = NULL;
 	if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, &empty_path,
 				       user_to_virt ( image->data, 0 ),
 				       image->len, &handle ) ) != 0 ) {
@@ -310,7 +317,11 @@ static int efi_image_probe ( struct image *image ) {
 		rc = -EEFI_LOAD ( efirc );
 		DBGC ( image, "EFIIMAGE %p could not load: %s\n",
 		       image, strerror ( rc ) );
-		return rc;
+		if ( efirc == EFI_SECURITY_VIOLATION ) {
+			goto err_load_image_security_violation;
+		} else {
+			goto err_load_image;
+		}
 	}
 
 	/* Unload the image.  We can't leave it loaded, because we
@@ -319,6 +330,11 @@ static int efi_image_probe ( struct image *image ) {
 	bs->UnloadImage ( handle );
 
 	return 0;
+
+ err_load_image_security_violation:
+	bs->UnloadImage ( handle );
+ err_load_image:
+	return rc;
 }
 
 /** EFI image type */