All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] efi_loader: improve error handling in try_load_entry()
@ 2024-04-20 14:00 Heinrich Schuchardt
  2024-04-20 15:28 ` Ilias Apalodimas
  0 siblings, 1 reply; 2+ messages in thread
From: Heinrich Schuchardt @ 2024-04-20 14:00 UTC (permalink / raw
  To: Ilias Apalodimas
  Cc: Masahisa Kojima, Raymond Mao, AKASHI Takahiro, Joao Marcos Costa,
	u-boot, Heinrich Schuchardt

The image is not unloaded if a security violation occurs.

If efi_set_load_options() fails, we do not free the memory allocated for
the optional data. We do not unload the image.

If a load option is not active, we use a random value from the stack to
allocate memory for the optional data of the load option.

* Unload the image if a security violation occurs.
* Free load_options if efi_set_load_options() fails.
* Unload the image if efi_set_load_options() fails.
* Do not allocate load_options with a random size from the stack
  if the boot entry is not active.

Fixes: 53f6a5aa8626 ("efi_loader: Replace config option for initrd loading")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/efi_loader/efi_bootmgr.c                  | 97 +++++++++----------
 test/py/tests/test_efi_secboot/test_signed.py | 28 +++---
 .../test_efi_secboot/test_signed_intca.py     | 10 +-
 .../tests/test_efi_secboot/test_unsigned.py   |  6 +-
 4 files changed, 70 insertions(+), 71 deletions(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 4ac519228a6..ca2ebdaa32f 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -613,9 +613,12 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
 	void *load_option;
 	efi_uintn_t size;
 	efi_status_t ret;
+	u32 attributes;
 
-	efi_create_indexed_name(varname, sizeof(varname), "Boot", n);
+	*handle = NULL;
+	*load_options = NULL;
 
+	efi_create_indexed_name(varname, sizeof(varname), "Boot", n);
 	load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
 	if (!load_option)
 		return EFI_LOAD_ERROR;
@@ -626,55 +629,54 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
 		goto error;
 	}
 
-	if (lo.attributes & LOAD_OPTION_ACTIVE) {
-		u32 attributes;
-
-		log_debug("trying to load \"%ls\" from %pD\n", lo.label,
-			  lo.file_path);
-
-		if (EFI_DP_TYPE(lo.file_path, MEDIA_DEVICE, FILE_PATH)) {
-			/* file_path doesn't contain a device path */
-			ret = try_load_from_short_path(lo.file_path, handle);
-		} else if (EFI_DP_TYPE(lo.file_path, MESSAGING_DEVICE, MSG_URI)) {
-			if (IS_ENABLED(CONFIG_EFI_HTTP_BOOT))
-				ret = try_load_from_uri_path(
-					(struct efi_device_path_uri *)lo.file_path,
-					lo.label, handle);
-			else
-				ret = EFI_LOAD_ERROR;
-		} else {
-			ret = try_load_from_media(lo.file_path, handle);
-		}
-		if (ret != EFI_SUCCESS) {
-			log_warning("Loading %ls '%ls' failed\n",
-				    varname, lo.label);
-			goto error;
-		}
+	if (!(lo.attributes & LOAD_OPTION_ACTIVE)) {
+		ret = EFI_LOAD_ERROR;
+		goto error;
+	}
 
-		attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
-			     EFI_VARIABLE_RUNTIME_ACCESS;
-		ret = efi_set_variable_int(u"BootCurrent",
-					   &efi_global_variable_guid,
-					   attributes, sizeof(n), &n, false);
-		if (ret != EFI_SUCCESS)
-			goto unload;
-		/* try to register load file2 for initrd's */
-		if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
-			ret = efi_initrd_register();
-			if (ret != EFI_SUCCESS)
-				goto unload;
-		}
+	log_debug("trying to load \"%ls\" from %pD\n", lo.label, lo.file_path);
 
-		log_info("Booting: %ls\n", lo.label);
+	if (EFI_DP_TYPE(lo.file_path, MEDIA_DEVICE, FILE_PATH)) {
+		/* file_path doesn't contain a device path */
+		ret = try_load_from_short_path(lo.file_path, handle);
+	} else if (EFI_DP_TYPE(lo.file_path, MESSAGING_DEVICE, MSG_URI)) {
+		if (IS_ENABLED(CONFIG_EFI_HTTP_BOOT))
+			ret = try_load_from_uri_path(
+				(struct efi_device_path_uri *)lo.file_path,
+				lo.label, handle);
+		else
+			ret = EFI_LOAD_ERROR;
 	} else {
-		ret = EFI_LOAD_ERROR;
+		ret = try_load_from_media(lo.file_path, handle);
+	}
+	if (ret != EFI_SUCCESS) {
+		log_warning("Loading %ls '%ls' failed\n",
+			    varname, lo.label);
+		goto error;
+	}
+
+	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+		     EFI_VARIABLE_RUNTIME_ACCESS;
+	ret = efi_set_variable_int(u"BootCurrent", &efi_global_variable_guid,
+				   attributes, sizeof(n), &n, false);
+	if (ret != EFI_SUCCESS)
+		goto error;
+
+	/* try to register load file2 for initrd's */
+	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
+		ret = efi_initrd_register();
+		if (ret != EFI_SUCCESS)
+			goto error;
 	}
 
-	/* Set load options */
+	log_info("Booting: %ls\n", lo.label);
+
+	/* Ignore the optional data in auto-generated boot options */
 	if (size >= sizeof(efi_guid_t) &&
 	    !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated))
 		size = 0;
 
+	/* Set optional data in loaded file protocol */
 	if (size) {
 		*load_options = malloc(size);
 		if (!*load_options) {
@@ -683,18 +685,15 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
 		}
 		memcpy(*load_options, lo.optional_data, size);
 		ret = efi_set_load_options(*handle, size, *load_options);
-	} else {
-		*load_options = NULL;
+		if (ret != EFI_SUCCESS)
+			free(load_options);
 	}
 
 error:
-	free(load_option);
-
-	return ret;
-
-unload:
-	if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
+	if (ret != EFI_SUCCESS && *handle &&
+	    EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
 		log_err("Unloading image failed\n");
+
 	free(load_option);
 
 	return ret;
diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
index 2f862a259ad..5000a4ab7b6 100644
--- a/test/py/tests/test_efi_secboot/test_signed.py
+++ b/test/py/tests/test_efi_secboot/test_signed.py
@@ -62,13 +62,13 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert('\'HELLO1\' failed' in ''.join(output))
-            assert('efi_start_image() returned: 26' in ''.join(output))
+            assert('efi_bootmgr_load() returned: 26' in ''.join(output))
             output = u_boot_console.run_command_list([
                 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi -s ""',
                 'efidebug boot order 2',
                 'efidebug test bootmgr'])
             assert '\'HELLO2\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 2b'):
             # Test Case 2b, authenticated by db
@@ -80,7 +80,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 2',
                 'efidebug test bootmgr'])
             assert '\'HELLO2\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
             output = u_boot_console.run_command_list([
                 'efidebug boot order 1',
                 'bootefi bootmgr'])
@@ -108,7 +108,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 3b'):
             # Test Case 3b, rejected by dbx even if db allows
@@ -120,7 +120,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
     def test_efi_signed_image_auth4(self, u_boot_console, efi_boot_env):
         """
@@ -146,7 +146,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
     def test_efi_signed_image_auth5(self, u_boot_console, efi_boot_env):
         """
@@ -196,7 +196,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 5d'):
             # Test Case 5d, rejected if both of signatures are revoked
@@ -208,7 +208,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         # Try rejection in reverse order.
         u_boot_console.restart_uboot()
@@ -233,7 +233,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
     def test_efi_signed_image_auth6(self, u_boot_console, efi_boot_env):
         """
@@ -268,7 +268,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 6c'):
             # Test Case 6c, rejected by image's digest in dbx
@@ -282,7 +282,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
     def test_efi_signed_image_auth7(self, u_boot_console, efi_boot_env):
         """
@@ -310,7 +310,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         # sha512 of an x509 cert in dbx
         u_boot_console.restart_uboot()
@@ -333,7 +333,7 @@ class TestEfiSignedImage(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
     def test_efi_signed_image_auth8(self, u_boot_console, efi_boot_env):
         """
@@ -368,4 +368,4 @@ class TestEfiSignedImage(object):
                 'efidebug test bootmgr'])
             assert(not 'hELLO, world!' in ''.join(output))
             assert('\'HELLO1\' failed' in ''.join(output))
-            assert('efi_start_image() returned: 26' in ''.join(output))
+            assert('efi_bootmgr_load() returned: 26' in ''.join(output))
diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
index 8d9a5f3e7fe..cf906205bc2 100644
--- a/test/py/tests/test_efi_secboot/test_signed_intca.py
+++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
@@ -43,7 +43,7 @@ class TestEfiSignedImageIntca(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO_a\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 1b'):
             # Test Case 1b, signed and authenticated by root CA
@@ -74,7 +74,7 @@ class TestEfiSignedImageIntca(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO_abc\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 2b'):
             # Test Case 2b, signed and authenticated by root CA
@@ -84,7 +84,7 @@ class TestEfiSignedImageIntca(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO_abc\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 2c'):
             # Test Case 2c, signed and authenticated by root CA
@@ -122,7 +122,7 @@ class TestEfiSignedImageIntca(object):
             assert 'Hello, world!' in ''.join(output)
             # Or,
             # assert '\'HELLO_abc\' failed' in ''.join(output)
-            # assert 'efi_start_image() returned: 26' in ''.join(output)
+            # assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
 
         with u_boot_console.log.section('Test Case 3b'):
             # Test Case 3b, revoked by root CA in dbx
@@ -132,4 +132,4 @@ class TestEfiSignedImageIntca(object):
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
             assert '\'HELLO_abc\' failed' in ''.join(output)
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
index 7c078f220d0..b4320ae4054 100644
--- a/test/py/tests/test_efi_secboot/test_unsigned.py
+++ b/test/py/tests/test_efi_secboot/test_unsigned.py
@@ -42,7 +42,7 @@ class TestEfiUnsignedImage(object):
             output = u_boot_console.run_command_list([
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
             assert 'Hello, world!' not in ''.join(output)
 
     def test_efi_unsigned_image_auth2(self, u_boot_console, efi_boot_env):
@@ -95,7 +95,7 @@ class TestEfiUnsignedImage(object):
             output = u_boot_console.run_command_list([
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
             assert 'Hello, world!' not in ''.join(output)
 
         with u_boot_console.log.section('Test Case 3b'):
@@ -113,5 +113,5 @@ class TestEfiUnsignedImage(object):
             output = u_boot_console.run_command_list([
                 'efidebug boot order 1',
                 'efidebug test bootmgr'])
-            assert 'efi_start_image() returned: 26' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
             assert 'Hello, world!' not in ''.join(output)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 1/1] efi_loader: improve error handling in try_load_entry()
  2024-04-20 14:00 [PATCH 1/1] efi_loader: improve error handling in try_load_entry() Heinrich Schuchardt
@ 2024-04-20 15:28 ` Ilias Apalodimas
  0 siblings, 0 replies; 2+ messages in thread
From: Ilias Apalodimas @ 2024-04-20 15:28 UTC (permalink / raw
  To: Heinrich Schuchardt
  Cc: Masahisa Kojima, Raymond Mao, AKASHI Takahiro, Joao Marcos Costa,
	u-boot

Hi Heinrich,

On Sat, 20 Apr 2024 at 17:01, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The image is not unloaded if a security violation occurs.
>
> If efi_set_load_options() fails, we do not free the memory allocated for
> the optional data. We do not unload the image.
>
> If a load option is not active, we use a random value from the stack to
> allocate memory for the optional data of the load option.
>
> * Unload the image if a security violation occurs.
> * Free load_options if efi_set_load_options() fails.
> * Unload the image if efi_set_load_options() fails.
> * Do not allocate load_options with a random size from the stack
>   if the boot entry is not active.

Where is that happening?
Don't we always init the size in efi_get_var() &
efi_deserialize_load_option()_ which are called early?
efi_get_var() especially sets size = 0 even on failures

>
> Fixes: 53f6a5aa8626 ("efi_loader: Replace config option for initrd loading")

I think we also need a fix tag for 0ad64007feb93 as well?

> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/efi_loader/efi_bootmgr.c                  | 97 +++++++++----------
>  test/py/tests/test_efi_secboot/test_signed.py | 28 +++---
>  .../test_efi_secboot/test_signed_intca.py     | 10 +-
>  .../tests/test_efi_secboot/test_unsigned.py   |  6 +-
>  4 files changed, 70 insertions(+), 71 deletions(-)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 4ac519228a6..ca2ebdaa32f 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -613,9 +613,12 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
>         void *load_option;
>         efi_uintn_t size;
>         efi_status_t ret;
> +       u32 attributes;
>
> -       efi_create_indexed_name(varname, sizeof(varname), "Boot", n);
> +       *handle = NULL;
> +       *load_options = NULL;
>
> +       efi_create_indexed_name(varname, sizeof(varname), "Boot", n);

>         load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
>         if (!load_option)
>                 return EFI_LOAD_ERROR;
> @@ -626,55 +629,54 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
>                 goto error;
>         }
>
> -       if (lo.attributes & LOAD_OPTION_ACTIVE) {
> -               u32 attributes;
> -
> -               log_debug("trying to load \"%ls\" from %pD\n", lo.label,
> -                         lo.file_path);
> -
> -               if (EFI_DP_TYPE(lo.file_path, MEDIA_DEVICE, FILE_PATH)) {
> -                       /* file_path doesn't contain a device path */
> -                       ret = try_load_from_short_path(lo.file_path, handle);
> -               } else if (EFI_DP_TYPE(lo.file_path, MESSAGING_DEVICE, MSG_URI)) {
> -                       if (IS_ENABLED(CONFIG_EFI_HTTP_BOOT))
> -                               ret = try_load_from_uri_path(
> -                                       (struct efi_device_path_uri *)lo.file_path,
> -                                       lo.label, handle);
> -                       else
> -                               ret = EFI_LOAD_ERROR;
> -               } else {
> -                       ret = try_load_from_media(lo.file_path, handle);
> -               }
> -               if (ret != EFI_SUCCESS) {
> -                       log_warning("Loading %ls '%ls' failed\n",
> -                                   varname, lo.label);
> -                       goto error;
> -               }
> +       if (!(lo.attributes & LOAD_OPTION_ACTIVE)) {
> +               ret = EFI_LOAD_ERROR;
> +               goto error;
> +       }
>
> -               attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -                            EFI_VARIABLE_RUNTIME_ACCESS;
> -               ret = efi_set_variable_int(u"BootCurrent",
> -                                          &efi_global_variable_guid,
> -                                          attributes, sizeof(n), &n, false);
> -               if (ret != EFI_SUCCESS)
> -                       goto unload;
> -               /* try to register load file2 for initrd's */
> -               if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> -                       ret = efi_initrd_register();
> -                       if (ret != EFI_SUCCESS)
> -                               goto unload;
> -               }
> +       log_debug("trying to load \"%ls\" from %pD\n", lo.label, lo.file_path);
>
> -               log_info("Booting: %ls\n", lo.label);
> +       if (EFI_DP_TYPE(lo.file_path, MEDIA_DEVICE, FILE_PATH)) {
> +               /* file_path doesn't contain a device path */
> +               ret = try_load_from_short_path(lo.file_path, handle);
> +       } else if (EFI_DP_TYPE(lo.file_path, MESSAGING_DEVICE, MSG_URI)) {
> +               if (IS_ENABLED(CONFIG_EFI_HTTP_BOOT))
> +                       ret = try_load_from_uri_path(
> +                               (struct efi_device_path_uri *)lo.file_path,
> +                               lo.label, handle);
> +               else
> +                       ret = EFI_LOAD_ERROR;
>         } else {
> -               ret = EFI_LOAD_ERROR;
> +               ret = try_load_from_media(lo.file_path, handle);
> +       }
> +       if (ret != EFI_SUCCESS) {
> +               log_warning("Loading %ls '%ls' failed\n",
> +                           varname, lo.label);
> +               goto error;
> +       }
> +
> +       attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +                    EFI_VARIABLE_RUNTIME_ACCESS;
> +       ret = efi_set_variable_int(u"BootCurrent", &efi_global_variable_guid,
> +                                  attributes, sizeof(n), &n, false);
> +       if (ret != EFI_SUCCESS)
> +               goto error;
> +
> +       /* try to register load file2 for initrd's */
> +       if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> +               ret = efi_initrd_register();
> +               if (ret != EFI_SUCCESS)
> +                       goto error;
>         }
>
> -       /* Set load options */
> +       log_info("Booting: %ls\n", lo.label);
> +
> +       /* Ignore the optional data in auto-generated boot options */
>         if (size >= sizeof(efi_guid_t) &&
>             !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated))
>                 size = 0;
>
> +       /* Set optional data in loaded file protocol */
>         if (size) {
>                 *load_options = malloc(size);
>                 if (!*load_options) {
> @@ -683,18 +685,15 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
>                 }
>                 memcpy(*load_options, lo.optional_data, size);
>                 ret = efi_set_load_options(*handle, size, *load_options);
> -       } else {
> -               *load_options = NULL;
> +               if (ret != EFI_SUCCESS)
> +                       free(load_options);
>         }
>
>  error:
> -       free(load_option);
> -
> -       return ret;
> -
> -unload:
> -       if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
> +       if (ret != EFI_SUCCESS && *handle &&
> +           EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
>                 log_err("Unloading image failed\n");
> +
>         free(load_option);
>
>         return ret;
> diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
> index 2f862a259ad..5000a4ab7b6 100644
> --- a/test/py/tests/test_efi_secboot/test_signed.py
> +++ b/test/py/tests/test_efi_secboot/test_signed.py
> @@ -62,13 +62,13 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert('\'HELLO1\' failed' in ''.join(output))
> -            assert('efi_start_image() returned: 26' in ''.join(output))
> +            assert('efi_bootmgr_load() returned: 26' in ''.join(output))
>              output = u_boot_console.run_command_list([
>                  'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi -s ""',
>                  'efidebug boot order 2',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO2\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 2b'):
>              # Test Case 2b, authenticated by db
> @@ -80,7 +80,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 2',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO2\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>              output = u_boot_console.run_command_list([
>                  'efidebug boot order 1',
>                  'bootefi bootmgr'])
> @@ -108,7 +108,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 3b'):
>              # Test Case 3b, rejected by dbx even if db allows
> @@ -120,7 +120,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>      def test_efi_signed_image_auth4(self, u_boot_console, efi_boot_env):
>          """
> @@ -146,7 +146,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>      def test_efi_signed_image_auth5(self, u_boot_console, efi_boot_env):
>          """
> @@ -196,7 +196,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 5d'):
>              # Test Case 5d, rejected if both of signatures are revoked
> @@ -208,7 +208,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          # Try rejection in reverse order.
>          u_boot_console.restart_uboot()
> @@ -233,7 +233,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>      def test_efi_signed_image_auth6(self, u_boot_console, efi_boot_env):
>          """
> @@ -268,7 +268,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 6c'):
>              # Test Case 6c, rejected by image's digest in dbx
> @@ -282,7 +282,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>      def test_efi_signed_image_auth7(self, u_boot_console, efi_boot_env):
>          """
> @@ -310,7 +310,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          # sha512 of an x509 cert in dbx
>          u_boot_console.restart_uboot()
> @@ -333,7 +333,7 @@ class TestEfiSignedImage(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>      def test_efi_signed_image_auth8(self, u_boot_console, efi_boot_env):
>          """
> @@ -368,4 +368,4 @@ class TestEfiSignedImage(object):
>                  'efidebug test bootmgr'])
>              assert(not 'hELLO, world!' in ''.join(output))
>              assert('\'HELLO1\' failed' in ''.join(output))
> -            assert('efi_start_image() returned: 26' in ''.join(output))
> +            assert('efi_bootmgr_load() returned: 26' in ''.join(output))
> diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
> index 8d9a5f3e7fe..cf906205bc2 100644
> --- a/test/py/tests/test_efi_secboot/test_signed_intca.py
> +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
> @@ -43,7 +43,7 @@ class TestEfiSignedImageIntca(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO_a\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 1b'):
>              # Test Case 1b, signed and authenticated by root CA
> @@ -74,7 +74,7 @@ class TestEfiSignedImageIntca(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO_abc\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 2b'):
>              # Test Case 2b, signed and authenticated by root CA
> @@ -84,7 +84,7 @@ class TestEfiSignedImageIntca(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO_abc\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 2c'):
>              # Test Case 2c, signed and authenticated by root CA
> @@ -122,7 +122,7 @@ class TestEfiSignedImageIntca(object):
>              assert 'Hello, world!' in ''.join(output)
>              # Or,
>              # assert '\'HELLO_abc\' failed' in ''.join(output)
> -            # assert 'efi_start_image() returned: 26' in ''.join(output)
> +            # assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 3b'):
>              # Test Case 3b, revoked by root CA in dbx
> @@ -132,4 +132,4 @@ class TestEfiSignedImageIntca(object):
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
>              assert '\'HELLO_abc\' failed' in ''.join(output)
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
> diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
> index 7c078f220d0..b4320ae4054 100644
> --- a/test/py/tests/test_efi_secboot/test_unsigned.py
> +++ b/test/py/tests/test_efi_secboot/test_unsigned.py
> @@ -42,7 +42,7 @@ class TestEfiUnsignedImage(object):
>              output = u_boot_console.run_command_list([
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>              assert 'Hello, world!' not in ''.join(output)
>
>      def test_efi_unsigned_image_auth2(self, u_boot_console, efi_boot_env):
> @@ -95,7 +95,7 @@ class TestEfiUnsignedImage(object):
>              output = u_boot_console.run_command_list([
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>              assert 'Hello, world!' not in ''.join(output)
>
>          with u_boot_console.log.section('Test Case 3b'):
> @@ -113,5 +113,5 @@ class TestEfiUnsignedImage(object):
>              output = u_boot_console.run_command_list([
>                  'efidebug boot order 1',
>                  'efidebug test bootmgr'])
> -            assert 'efi_start_image() returned: 26' in ''.join(output)
> +            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
>              assert 'Hello, world!' not in ''.join(output)
> --
> 2.43.0
>

Please adjust the commit message either in a v2 or while merging.
Other than that

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-04-20 15:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-20 14:00 [PATCH 1/1] efi_loader: improve error handling in try_load_entry() Heinrich Schuchardt
2024-04-20 15:28 ` Ilias Apalodimas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.