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 |
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 ==
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 --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 ==
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(-)