On 06.05.19 07:42, Heinrich Schuchardt wrote:
If the file path is NULL, return EFI_INVALID_PARAMETER.
If the file path is invalid, return EFI_NOT_FOUND.


These 2 are a documented inEFI_LOAD_FILE_PROTOCOL.LoadFile(). It might be a good idea to indicate that for later reference.


If the size of the source buffer is 0, return EFI_LOAD_ERROR.


The spec says this should be EFI_INVALID_PARAMETER, no? Is this a spec or an SCT bug?


If the parent image handle does not refer to a loaded image return
EFI_INVALID_PARAMETER.


I agree that this is a good change, but where is the spec reference? I couldn't find anything.



The change is required to reach UEFI SCT conformance.

Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>


IMHO this really should be a patch set with multiple individual changes, each changing one piece of spec conformance at a time.


That way if we happen to break anything along the way, bisecting would point us to exactly the right point where things fell apart.


---
v2
        efi_load_image_from_path() must initalize *buffer even in case of
        an error
---
  include/efi_loader.h           |  1 +
  lib/efi_loader/efi_boottime.c  | 17 ++++++++----
  lib/efi_loader/efi_root_node.c | 48 ++++++++++++++++++----------------
  3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index d3a1d4c465..07ef14ba1c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -187,6 +187,7 @@ struct efi_handler {
   */
  enum efi_object_type {
        EFI_OBJECT_TYPE_UNDEFINED = 0,
+       EFI_OBJECT_TYPE_U_BOOT_FIRMWARE,


This looks like an unrelated change?


        EFI_OBJECT_TYPE_LOADED_IMAGE,
        EFI_OBJECT_TYPE_STARTED_IMAGE,
  };
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 78a4063949..f75e6c0169 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1680,10 +1680,13 @@ efi_status_t efi_load_image_from_path(struct 
efi_device_path *file_path,
        *buffer = NULL;
        *size = 0;

+       if (!file_path)
+               return EFI_INVALID_PARAMETER;
+
        /* Open file */
        f = efi_file_from_path(file_path);
        if (!f)
-               return EFI_DEVICE_ERROR;
+               return EFI_NOT_FOUND;

        /* Get file size */
        bs = 0;
@@ -1760,13 +1763,13 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy,
        EFI_ENTRY("%d, %p, %pD, %p, %zd, %p", boot_policy, parent_image,
                  file_path, source_buffer, source_size, image_handle);

-       if (!image_handle || !parent_image) {
+       if (!image_handle || !efi_search_obj(parent_image)) {
                ret = EFI_INVALID_PARAMETER;
                goto error;
        }
-
-       if (!source_buffer && !file_path) {
-               ret = EFI_NOT_FOUND;
+       /* The parent image handle must refer to a loaded image */
+       if (!parent_image->type) {
+               ret = EFI_INVALID_PARAMETER;
                goto error;
        }

@@ -1776,6 +1779,10 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy,
                if (ret != EFI_SUCCESS)
                        goto error;
        } else {
+               if (!source_size) {
+                       ret = EFI_LOAD_ERROR;
+                       goto error;
+               }
                dest_buffer = source_buffer;
        }
        /* split file_path which contains both the device and file parts */
diff --git a/lib/efi_loader/efi_root_node.c b/lib/efi_loader/efi_root_node.c
index e0fcbb85a4..38514e0820 100644
--- a/lib/efi_loader/efi_root_node.c
+++ b/lib/efi_loader/efi_root_node.c
@@ -28,6 +28,7 @@ struct efi_root_dp {
   */
  efi_status_t efi_root_node_register(void)
  {
+       efi_status_t ret;
        struct efi_root_dp *dp;

        /* Create device path protocol */
@@ -47,28 +48,31 @@ efi_status_t efi_root_node_register(void)
        dp->end.length = sizeof(struct efi_device_path);

        /* Create root node and install protocols */
-       return EFI_CALL(efi_install_multiple_protocol_interfaces(&efi_root,
-                      /* Device path protocol */
-                      &efi_guid_device_path, dp,
-                      /* Device path to text protocol */
-                      &efi_guid_device_path_to_text_protocol,
-                      (void *)&efi_device_path_to_text,
-                      /* Device path utilities protocol */
-                      &efi_guid_device_path_utilities_protocol,
-                      (void *)&efi_device_path_utilities,
-                      /* Unicode collation protocol */
-                      &efi_guid_unicode_collation_protocol,
-                      (void *)&efi_unicode_collation_protocol,
+       ret = EFI_CALL(efi_install_multiple_protocol_interfaces
+                       (&efi_root,
+                        /* Device path protocol */
+                        &efi_guid_device_path, dp,
+                        /* Device path to text protocol */
+                        &efi_guid_device_path_to_text_protocol,
+                        (void *)&efi_device_path_to_text,
+                        /* Device path utilities protocol */
+                        &efi_guid_device_path_utilities_protocol,
+                        (void *)&efi_device_path_utilities,
+                        /* Unicode collation protocol */
+                        &efi_guid_unicode_collation_protocol,
+                        (void *)&efi_unicode_collation_protocol,
  #if CONFIG_IS_ENABLED(EFI_LOADER_HII)
-                      /* HII string protocol */
-                      &efi_guid_hii_string_protocol,
-                      (void *)&efi_hii_string,
-                      /* HII database protocol */
-                      &efi_guid_hii_database_protocol,
-                      (void *)&efi_hii_database,
-                      /* HII configuration routing protocol */
-                      &efi_guid_hii_config_routing_protocol,
-                      (void *)&efi_hii_config_routing,
+                        /* HII string protocol */
+                        &efi_guid_hii_string_protocol,
+                        (void *)&efi_hii_string,
+                        /* HII database protocol */
+                        &efi_guid_hii_database_protocol,
+                        (void *)&efi_hii_database,
+                        /* HII configuration routing protocol */
+                        &efi_guid_hii_config_routing_protocol,
+                        (void *)&efi_hii_config_routing,
  #endif
-                      NULL));
+                        NULL));
+       efi_root->type = EFI_OBJECT_TYPE_U_BOOT_FIRMWARE;
+       return ret;


I guess this is part of the unrelated change?



Alex



  }
--
2.20.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to