diff mbox series

[1/5] efi_loader: check the status of disconnected drivers

Message ID 20230615143941.416924-1-ilias.apalodimas@linaro.org
State New
Headers show
Series [1/5] efi_loader: check the status of disconnected drivers | expand

Commit Message

Ilias Apalodimas June 15, 2023, 2:39 p.m. UTC
efi_uninstall_protocol() calls efi_disconnect_all_drivers() but never
checks the return value.  Honor that and return an appropriate error
if the associated controllers failed to disconnect

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_boottime.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt June 18, 2023, 6:39 a.m. UTC | #1
On 6/15/23 16:39, Ilias Apalodimas wrote:
> efi_uninstall_protocol() calls efi_disconnect_all_drivers() but never
> checks the return value.  Honor that and return an appropriate error
> if the associated controllers failed to disconnect
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   lib/efi_loader/efi_boottime.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 5006c0e1e4af..68198e6b5ff6 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1353,7 +1353,11 @@ static efi_status_t efi_uninstall_protocol
>   	if (r != EFI_SUCCESS)
>   		goto out;
>   	/* Disconnect controllers */
> -	efi_disconnect_all_drivers(efiobj, protocol, NULL);
> +	r = efi_disconnect_all_drivers(efiobj, protocol, NULL);
> +	if (r != EFI_SUCCESS) {
> +		r = EFI_DEVICE_ERROR;

This return code is not foreseen in this case by the UEFI specification.
I only found:

EFI_ACCESS_DENIED:
The interface was not removed because the interface is still being used
by a driver.

EDK II sets this code in CoreDisconnectControllersUsingProtocolInterface().

Best regards

Heinrich

> +		goto out;
> +	}
>   	/* Close protocol */
>   	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
>   		if (item->info.attributes ==
Ilias Apalodimas June 18, 2023, 2:19 p.m. UTC | #2
On Sun, Jun 18, 2023 at 08:39:27AM +0200, Heinrich Schuchardt wrote:
> On 6/15/23 16:39, Ilias Apalodimas wrote:
> > efi_uninstall_protocol() calls efi_disconnect_all_drivers() but never
> > checks the return value.  Honor that and return an appropriate error
> > if the associated controllers failed to disconnect
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   lib/efi_loader/efi_boottime.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index 5006c0e1e4af..68198e6b5ff6 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -1353,7 +1353,11 @@ static efi_status_t efi_uninstall_protocol
> >   	if (r != EFI_SUCCESS)
> >   		goto out;
> >   	/* Disconnect controllers */
> > -	efi_disconnect_all_drivers(efiobj, protocol, NULL);
> > +	r = efi_disconnect_all_drivers(efiobj, protocol, NULL);
> > +	if (r != EFI_SUCCESS) {
> > +		r = EFI_DEVICE_ERROR;
> 
> This return code is not foreseen in this case by the UEFI specification.
> I only found:
> 
> EFI_ACCESS_DENIED:
> The interface was not removed because the interface is still being used
> by a driver.
> 
> EDK II sets this code in CoreDisconnectControllersUsingProtocolInterface().

Right, I only looked at DisconnectController() returns values...
You are right, I'll switch this and the check in the selftests.

Thanks
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> > +		goto out;
> > +	}
> >   	/* Close protocol */
> >   	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
> >   		if (item->info.attributes ==
>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 5006c0e1e4af..68198e6b5ff6 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1353,7 +1353,11 @@  static efi_status_t efi_uninstall_protocol
 	if (r != EFI_SUCCESS)
 		goto out;
 	/* Disconnect controllers */
-	efi_disconnect_all_drivers(efiobj, protocol, NULL);
+	r = efi_disconnect_all_drivers(efiobj, protocol, NULL);
+	if (r != EFI_SUCCESS) {
+		r = EFI_DEVICE_ERROR;
+		goto out;
+	}
 	/* Close protocol */
 	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
 		if (item->info.attributes ==