Re: [U-Boot] [PATCH v2 1/1] efi_loader: LoadImage() parameter checks

2019-05-06 Thread Graf, Alexander


On 06.05.19 13:32, Heinrich Schuchardt wrote:


On 5/6/19 10:39 AM, Graf, Alexander wrote:

On 06.05.19 09:52, Heinrich Schuchardt wrote:

On 5/6/19 9:24 AM, Graf, Alexander wrote:

On 06.05.19 07:42, Heinrich Schuchardt wrote:

If the file path is NULL, return EFI_INVALID_PARAMETER.

There is a nice contradiction between UEFI Spec 2.7 Errata B and the
UEFI SCT specfication.

The UEFI spec has:
EFI_NOT_FOUND: Both SourceBuffer and DevicePath are NULL

UEFI SCT Specification (2017): 3.4.1 LoadImage, 5.1.4.1.2
BS.LoadImage – LoadImage() returns EFI_INVALID_PARAMETER with NULL FilePath.


If the file path is invalid, return EFI_NOT_FOUND.

UEFI Spec 2.7 Errata B:
EFI_DEVICE_ERROR - Image was not loaded because the device returned a
read error.

UEFI SCT Specification (2017): 3.4.1 LoadImage, 5.1.4.1.4
BS.LoadImage – LoadImage() returns EFI_NOT_FOUND with a non-existent
FilePath.


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


See the last sentence of the commit message.


I don't understand? The return values are not defined as part of generic
LoadImage(), but as part of EFI_LOAD_FILE_PROTOCOL.LoadFile() which is
pretty counterintuitive IMHO.

The definitions in the EFI_LOAD_FILE_PROTOCOL should not prescribe the
return values of LoadImage().

I guess we should stick with the UEFI spec in case of contradiction and
try to convince uefi.org that there is a bug in the SCT.



Vincent, do you have any further input for us here? :)


Thanks!

Alex




Best regards

Heinrich




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?

According to the UEFI 2.7A spec:
EFI_LOAD_IMAGE should be returned if the image is corrupt.

UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
5.1.4.1.6


Hm, but LoadFile() explicitly says:

   EFI_INVALID_PARAMETER | FilePath is not a valid device path, or
BufferSize is NULL.

Or are we beyond that point here?



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.


UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
5.1.4.1.1


Could you please add that reference to the commit message?



The change is required to reach UEFI SCT conformance.

Signed-off-by: Heinrich Schuchardt 

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?

No. We use efi_root as parent_image. We test if parent_image is an image
in LoadImage. So efi_root must be marked as image.


I see, that was not obvious :).

Alex




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


Re: [U-Boot] [PATCH v2 1/1] efi_loader: LoadImage() parameter checks

2019-05-06 Thread Heinrich Schuchardt


On 5/6/19 10:39 AM, Graf, Alexander wrote:
>
> On 06.05.19 09:52, Heinrich Schuchardt wrote:
>> On 5/6/19 9:24 AM, Graf, Alexander wrote:
>>> On 06.05.19 07:42, Heinrich Schuchardt wrote:
 If the file path is NULL, return EFI_INVALID_PARAMETER.

There is a nice contradiction between UEFI Spec 2.7 Errata B and the
UEFI SCT specfication.

The UEFI spec has:
EFI_NOT_FOUND: Both SourceBuffer and DevicePath are NULL

UEFI SCT Specification (2017): 3.4.1 LoadImage, 5.1.4.1.2
BS.LoadImage – LoadImage() returns EFI_INVALID_PARAMETER with NULL FilePath.

 If the file path is invalid, return EFI_NOT_FOUND.

UEFI Spec 2.7 Errata B:
EFI_DEVICE_ERROR - Image was not loaded because the device returned a
read error.

UEFI SCT Specification (2017): 3.4.1 LoadImage, 5.1.4.1.4
BS.LoadImage – LoadImage() returns EFI_NOT_FOUND with a non-existent
FilePath.

>>>
>>> These 2 are a documented inEFI_LOAD_FILE_PROTOCOL.LoadFile(). It might
>>> be a good idea to indicate that for later reference.
>>>
>> See the last sentence of the commit message.
>
>
> I don't understand? The return values are not defined as part of generic
> LoadImage(), but as part of EFI_LOAD_FILE_PROTOCOL.LoadFile() which is
> pretty counterintuitive IMHO.

The definitions in the EFI_LOAD_FILE_PROTOCOL should not prescribe the
return values of LoadImage().

I guess we should stick with the UEFI spec in case of contradiction and
try to convince uefi.org that there is a bug in the SCT.

Best regards

Heinrich

>
>
>>
 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?
>> According to the UEFI 2.7A spec:
>> EFI_LOAD_IMAGE should be returned if the image is corrupt.
>>
>> UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
>> 5.1.4.1.6
>
>
> Hm, but LoadFile() explicitly says:
>
>   EFI_INVALID_PARAMETER | FilePath is not a valid device path, or
> BufferSize is NULL.
>
> Or are we beyond that point here?
>
>
>>
>>>
 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.
>>>
>> UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
>> 5.1.4.1.1
>
>
> Could you please add that reference to the commit message?
>
>
>>
 The change is required to reach UEFI SCT conformance.

 Signed-off-by: Heinrich Schuchardt 
>>>
>>> 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?
>> No. We use efi_root as parent_image. We test if parent_image is an image
>> in LoadImage. So efi_root must be marked as image.
>
>
> I see, that was not obvious :).
>
> Alex
>
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/1] efi_loader: LoadImage() parameter checks

2019-05-06 Thread Graf, Alexander


On 06.05.19 09:52, Heinrich Schuchardt wrote:

On 5/6/19 9:24 AM, Graf, Alexander wrote:

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.


See the last sentence of the commit message.



I don't understand? The return values are not defined as part of generic 
LoadImage(), but as part of EFI_LOAD_FILE_PROTOCOL.LoadFile() which is 
pretty counterintuitive IMHO.






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?

According to the UEFI 2.7A spec:
EFI_LOAD_IMAGE should be returned if the image is corrupt.

UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
5.1.4.1.6



Hm, but LoadFile() explicitly says:

  EFI_INVALID_PARAMETER | FilePath is not a valid device path, or 
BufferSize is NULL.


Or are we beyond that point here?







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.


UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
5.1.4.1.1



Could you please add that reference to the commit message?





The change is required to reach UEFI SCT conformance.

Signed-off-by: Heinrich Schuchardt 


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?

No. We use efi_root as parent_image. We test if parent_image is an image
in LoadImage. So efi_root must be marked as image.



I see, that was not obvious :).

Alex


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


Re: [U-Boot] [PATCH v2 1/1] efi_loader: LoadImage() parameter checks

2019-05-06 Thread Graf, Alexander


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 



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(_root,
-  /* Device path protocol */
-  _guid_device_path, dp,
-  /* Device path to text protocol */
-  _guid_device_path_to_text_protocol,
-  (void *)_device_path_to_text,
-  /* Device path utilities protocol */
-  _guid_device_path_utilities_protocol,
-  (void *)_device_path_utilities,
-  /* Unicode collation protocol */
-  _guid_unicode_collation_protocol,
-  (void *)_unicode_collation_protocol,
+   ret = EFI_CALL(efi_install_multiple_protocol_interfaces
+   (_root,
+/* Device path protocol */
+_guid_device_path, dp,
+/* Device path to text protocol */
+

Re: [U-Boot] [PATCH v2 1/1] efi_loader: LoadImage() parameter checks

2019-05-06 Thread Heinrich Schuchardt
On 5/6/19 9:24 AM, Graf, Alexander wrote:
> 
> 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.
> 

See the last sentence of the commit message.

> 
>> 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?

According to the UEFI 2.7A spec:
EFI_LOAD_IMAGE should be returned if the image is corrupt.

UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
5.1.4.1.6

> 
> 
>> 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.
>

UEFI SCT II Case Specification (June 2017), 3.4.1 LoadImage, Number
5.1.4.1.1

> 
>>
>> The change is required to reach UEFI SCT conformance.
>>
>> Signed-off-by: Heinrich Schuchardt 
> 
> 
> 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?

No. We use efi_root as parent_image. We test if parent_image is an image
in LoadImage. So efi_root must be marked as image.

> 
> 
>>   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(_root,
>> -   /* Device path protocol */
>> -   _guid_device_path, dp,
>> -   /* Device path to text protocol */
>> -   _guid_device_path_to_text_protocol,
>> -   (void *)_device_path_to_text,
>> -   /* Device path 

[U-Boot] [PATCH v2 1/1] efi_loader: LoadImage() parameter checks

2019-05-05 Thread Heinrich Schuchardt
If the file path is NULL, return EFI_INVALID_PARAMETER.
If the file path is invalid, return EFI_NOT_FOUND.
If the size of the source buffer is 0, return EFI_LOAD_ERROR.
If the parent image handle does not refer to a loaded image return
EFI_INVALID_PARAMETER.

The change is required to reach UEFI SCT conformance.

Signed-off-by: Heinrich Schuchardt 
---
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,
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(_root,
-  /* Device path protocol */
-  _guid_device_path, dp,
-  /* Device path to text protocol */
-  _guid_device_path_to_text_protocol,
-  (void *)_device_path_to_text,
-  /* Device path utilities protocol */
-  _guid_device_path_utilities_protocol,
-  (void *)_device_path_utilities,
-  /* Unicode collation protocol */
-  _guid_unicode_collation_protocol,
-  (void *)_unicode_collation_protocol,
+   ret = EFI_CALL(efi_install_multiple_protocol_interfaces
+   (_root,
+/* Device path protocol */
+_guid_device_path, dp,
+/* Device path to text protocol */
+_guid_device_path_to_text_protocol,
+(void *)_device_path_to_text,
+/* Device path utilities protocol */
+_guid_device_path_utilities_protocol,
+(void *)_device_path_utilities,
+/* Unicode collation protocol */
+_guid_unicode_collation_protocol,
+(void *)_unicode_collation_protocol,
 #if CONFIG_IS_ENABLED(EFI_LOADER_HII)
-  /* HII string protocol */
-  _guid_hii_string_protocol,
-  (void *)_hii_string,
-  /* HII database protocol */
-