[U-Boot] [PATCH] efi_loader: fix TEST_PROTOCOL case in OpenProtocol()

[U-Boot] [PATCH] efi_loader: fix TEST_PROTOCOL case in OpenProtocol()

[Rob Clark]

In the TEST_PROTOCOL case, protocol_interface might be NULL, and at any
rate should not be touched.  So skip efi_protocol_open() in this case.

Fixes: "efi_loader: open_info in OpenProtocol"
Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 lib/efi_loader/efi_boottime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 72a0c932a6..f1f0e021b9 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1723,7 +1723,7 @@ efi_status_t EFIAPI efi_open_protocol(
 	}
 
 	r = efi_search_protocol(handle, protocol, &handler);
-	if (r != EFI_SUCCESS)
+	if (r != EFI_SUCCESS || attributes == EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
 		goto out;
 
 	r = efi_protocol_open(handler, protocol_interface, agent_handle,
-- 
2.13.5

[U-Boot] [PATCH] efi_loader: fix TEST_PROTOCOL case in OpenProtocol()

[Heinrich Schuchardt]

On 09/16/2017 03:26 PM, Rob Clark wrote:
> In the TEST_PROTOCOL case, protocol_interface might be NULL, and at any
> rate should not be touched.  So skip efi_protocol_open() in this case.
> 
> Fixes: "efi_loader: open_info in OpenProtocol"
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  lib/efi_loader/efi_boottime.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 72a0c932a6..f1f0e021b9 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1723,7 +1723,7 @@ efi_status_t EFIAPI efi_open_protocol(
>  	}
>  
>  	r = efi_search_protocol(handle, protocol, &handler);
> -	if (r != EFI_SUCCESS)
> +	if (r != EFI_SUCCESS || attributes == EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
>  		goto out;
>  
>  	r = efi_protocol_open(handler, protocol_interface, agent_handle,
> 

Thank you for pointing to a problem in my patch

efi_loader: open_info in OpenProtocol
https://patchwork.ozlabs.org/patch/806178/

Your patch makes OpenProtocol conform with this requirement cited from
the UEFI spec:

"TEST_PROTOCOL - Used by a driver to test for the existence of a
protocol interface on a handle. Interface is optional for this attribute
value, so it is ignored, and the caller should only use the return
status code. The caller is also not required to close the protocol
interface with CloseProtocol()".

The last sentence implies that we should not update the
OpenProtocolInformation for TEST_PROTOCOL. We should put this into a test.

There are anyway other review comments I have to consider when
resubmitting my patch series for the protocol services. So I would
prefer to just integrate this correction into my patch instead of adding
a separate one.

Best regards

Heinrich

[U-Boot] [PATCH] efi_loader: fix TEST_PROTOCOL case in OpenProtocol()

[Rob Clark]

On Sun, Sep 17, 2017 at 4:21 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/16/2017 03:26 PM, Rob Clark wrote:
>> In the TEST_PROTOCOL case, protocol_interface might be NULL, and at any
>> rate should not be touched.  So skip efi_protocol_open() in this case.
>>
>> Fixes: "efi_loader: open_info in OpenProtocol"
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  lib/efi_loader/efi_boottime.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 72a0c932a6..f1f0e021b9 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -1723,7 +1723,7 @@ efi_status_t EFIAPI efi_open_protocol(
>>       }
>>
>>       r = efi_search_protocol(handle, protocol, &handler);
>> -     if (r != EFI_SUCCESS)
>> +     if (r != EFI_SUCCESS || attributes == EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
>>               goto out;
>>
>>       r = efi_protocol_open(handler, protocol_interface, agent_handle,
>>
>
> Thank you for pointing to a problem in my patch
>
> efi_loader: open_info in OpenProtocol
> https://patchwork.ozlabs.org/patch/806178/
>
> Your patch makes OpenProtocol conform with this requirement cited from
> the UEFI spec:
>
> "TEST_PROTOCOL - Used by a driver to test for the existence of a
> protocol interface on a handle. Interface is optional for this attribute
> value, so it is ignored, and the caller should only use the return
> status code. The caller is also not required to close the protocol
> interface with CloseProtocol()".
>
> The last sentence implies that we should not update the
> OpenProtocolInformation for TEST_PROTOCOL. We should put this into a test.
>
> There are anyway other review comments I have to consider when
> resubmitting my patch series for the protocol services. So I would
> prefer to just integrate this correction into my patch instead of adding
> a separate one.

Sure, it's fine by me if you just want to squash this into your patch

BR,
-R