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

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; + goto out; + } /* Close protocol */ list_for_each_entry_safe(item, pos, &handler->open_infos, link) { if (item->info.attributes ==

efi_disconnect_controller() doesn't reconnect drivers in case of failure. Reconnect the disconnected drivers properly
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_loader/efi_boottime.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 68198e6b5ff6..df675d0ad488 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -97,6 +97,12 @@ static efi_status_t EFIAPI efi_disconnect_controller( efi_handle_t driver_image_handle, efi_handle_t child_handle);
+static efi_status_t EFIAPI efi_connect_controller( + efi_handle_t controller_handle, + efi_handle_t *driver_image_handle, + struct efi_device_path *remain_device_path, + bool recursive); + /* Called on every callback entry */ int __efi_entry_check(void) { @@ -1298,7 +1304,7 @@ static efi_status_t efi_disconnect_all_drivers const efi_guid_t *protocol, efi_handle_t child_handle) { - efi_uintn_t number_of_drivers; + efi_uintn_t number_of_drivers, tmp; efi_handle_t *driver_handle_buffer; efi_status_t r, ret;
@@ -1308,15 +1314,30 @@ static efi_status_t efi_disconnect_all_drivers return ret; if (!number_of_drivers) return EFI_SUCCESS; - ret = EFI_NOT_FOUND; + + tmp = number_of_drivers; while (number_of_drivers) { - r = EFI_CALL(efi_disconnect_controller( + ret = EFI_CALL(efi_disconnect_controller( handle, driver_handle_buffer[--number_of_drivers], child_handle)); - if (r == EFI_SUCCESS) - ret = r; + if (ret != EFI_SUCCESS) + goto reconnect; } + + free(driver_handle_buffer); + return ret; + +reconnect: + /* Reconnect all disconnected drivers */ + for (; number_of_drivers < tmp; number_of_drivers++) { + r = EFI_CALL(efi_connect_controller(handle, + &driver_handle_buffer[number_of_drivers], + NULL, true)); + if (r != EFI_SUCCESS) + EFI_PRINT("Failed to reconnect controller\n"); + } + free(driver_handle_buffer); return ret; }

When we are trying to uninstall a protocol interface from a controller handle we are trying to disconnect drivers related to that protocol. However, when we call efi_disconnect_all_drivers() we pass the protocol GUID. If 2 different drivers are using the same protocol interface and one of them can't be stopped (e.g by returning EFI_DEVICE_ERROR) we should stop uninstalling it.
Instead of explicitly passing the protocol GUID, pass NULL as an argument. That will force efi_get_drivers() to return all drivers consuming the interface regardless of the protocol GUID. While at it call efi_disconnect_all_drivers() with a handle instead of the efiobj
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- 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 df675d0ad488..b148824c7ec5 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1374,7 +1374,7 @@ static efi_status_t efi_uninstall_protocol if (r != EFI_SUCCESS) goto out; /* Disconnect controllers */ - r = efi_disconnect_all_drivers(efiobj, protocol, NULL); + r = efi_disconnect_all_drivers(handle, NULL, NULL); if (r != EFI_SUCCESS) { r = EFI_DEVICE_ERROR; goto out; -- 2.39.2

On 6/15/23 16:39, Ilias Apalodimas wrote:
When we are trying to uninstall a protocol interface from a controller handle we are trying to disconnect drivers related to that protocol. However, when we call efi_disconnect_all_drivers() we pass the protocol GUID. If 2 different drivers are using the same protocol interface and one of them can't be stopped (e.g by returning EFI_DEVICE_ERROR) we should stop uninstalling it.
Instead of explicitly passing the protocol GUID, pass NULL as an argument. That will force efi_get_drivers() to return all drivers consuming the interface regardless of the protocol GUID. While at it call efi_disconnect_all_drivers() with a handle instead of the efiobj
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
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 df675d0ad488..b148824c7ec5 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1374,7 +1374,7 @@ static efi_status_t efi_uninstall_protocol if (r != EFI_SUCCESS) goto out; /* Disconnect controllers */
- r = efi_disconnect_all_drivers(efiobj, protocol, NULL);
- r = efi_disconnect_all_drivers(handle, NULL, NULL);
When uninstalling a *single* protocol we don't want to disconnect drivers for *all* protocols on this handle but only for the specified one. This will not work if you don't pass the protocol GUID to DisconnectController().
Best regards
Heinrich
if (r != EFI_SUCCESS) { r = EFI_DEVICE_ERROR; goto out; -- 2.39.2

On Mon, 19 Jun 2023 at 10:04, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 6/15/23 16:39, Ilias Apalodimas wrote:
When we are trying to uninstall a protocol interface from a controller handle we are trying to disconnect drivers related to that protocol. However, when we call efi_disconnect_all_drivers() we pass the protocol GUID. If 2 different drivers are using the same protocol interface and one of them can't be stopped (e.g by returning EFI_DEVICE_ERROR) we should stop uninstalling it.
Instead of explicitly passing the protocol GUID, pass NULL as an argument. That will force efi_get_drivers() to return all drivers consuming the interface regardless of the protocol GUID. While at it call efi_disconnect_all_drivers() with a handle instead of the efiobj
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
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 df675d0ad488..b148824c7ec5 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1374,7 +1374,7 @@ static efi_status_t efi_uninstall_protocol if (r != EFI_SUCCESS) goto out; /* Disconnect controllers */
r = efi_disconnect_all_drivers(efiobj, protocol, NULL);
r = efi_disconnect_all_drivers(handle, NULL, NULL);
When uninstalling a *single* protocol we don't want to disconnect drivers for *all* protocols on this handle but only for the specified one. This will not work if you don't pass the protocol GUID to DisconnectController().
Right, I missread the spec and the selftests are probably wrong as well... I'll fix that on v2
Thanks /Ilias
Best regards
Heinrich
if (r != EFI_SUCCESS) { r = EFI_DEVICE_ERROR; goto out;
-- 2.39.2

Up to now we did not check the return value of DisconnectController. A previous patch is fixing that taking into account what happened during the controller disconnect. But that check takes place before our code is trying to figure out if the interface exists to begin with. In case a driver is not allowed to unbind -- e.g returning EFI_DEVICE_ERROR, we will end up returning that error instead of EFI_NOT_FOUND.
Add an extra check on the top of the function to make sure the protocol interface exists before trying to disconnect any drivers
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_loader/efi_boottime.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b148824c7ec5..d6d52d4bbac8 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1373,6 +1373,8 @@ static efi_status_t efi_uninstall_protocol r = efi_search_protocol(handle, protocol, &handler); if (r != EFI_SUCCESS) goto out; + if (handler->protocol_interface != protocol_interface) + return EFI_NOT_FOUND; /* Disconnect controllers */ r = efi_disconnect_all_drivers(handle, NULL, NULL); if (r != EFI_SUCCESS) {

On 6/15/23 16:39, Ilias Apalodimas wrote:
Up to now we did not check the return value of DisconnectController. A previous patch is fixing that taking into account what happened during the controller disconnect. But that check takes place before our code is trying to figure out if the interface exists to begin with. In case a driver is not allowed to unbind -- e.g returning EFI_DEVICE_ERROR, we will end up returning that error instead of EFI_NOT_FOUND.
Add an extra check on the top of the function to make sure the protocol interface exists before trying to disconnect any drivers
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_boottime.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b148824c7ec5..d6d52d4bbac8 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1373,6 +1373,8 @@ static efi_status_t efi_uninstall_protocol r = efi_search_protocol(handle, protocol, &handler); if (r != EFI_SUCCESS) goto out;
- if (handler->protocol_interface != protocol_interface)
/* Disconnect controllers */ r = efi_disconnect_all_drivers(handle, NULL, NULL); if (r != EFI_SUCCESS) {return EFI_NOT_FOUND;

We recently fixed a few issues wrt to controller handling. Add a few test cases to cover the new code. - add a second driver in the same controller handle which will refuse to unbind on the first protocol removal - add tests to verify controllers are reconnected when uninstalling a protocol fails - add tests to make sure EFI_NOT_FOUND is returned if a non existent interface is being removed
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_selftest/efi_selftest_controllers.c | 221 ++++++++++++++++++-- 1 file changed, 198 insertions(+), 23 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_controllers.c b/lib/efi_selftest/efi_selftest_controllers.c index 79bc86fb0c3a..d2a974079329 100644 --- a/lib/efi_selftest/efi_selftest_controllers.c +++ b/lib/efi_selftest/efi_selftest_controllers.c @@ -13,6 +13,8 @@ #include <efi_selftest.h>
#define NUMBER_OF_CHILD_CONTROLLERS 4 +#define CONTROLLER1_DRIVERS (1 + NUMBER_OF_CHILD_CONTROLLERS) +#define CONTROLLER2_DRIVERS 1
static int interface1 = 1; static int interface2 = 2; @@ -22,24 +24,32 @@ const efi_guid_t guid_driver_binding_protocol = static efi_guid_t guid_controller = EFI_GUID(0xe6ab1d96, 0x6bff, 0xdb42, 0xaa, 0x05, 0xc8, 0x1f, 0x7f, 0x45, 0x26, 0x34); + +static efi_guid_t guid_controller2 = + EFI_GUID(0xe6ab1d96, 0x6bff, 0xdb42, + 0xaa, 0x50, 0x8c, 0xf1, 0xf7, 0x54, 0x62, 0x43); + static efi_guid_t guid_child_controller = EFI_GUID(0x1d41f6f5, 0x2c41, 0xddfb, 0xe2, 0x9b, 0xb8, 0x0e, 0x2e, 0xe8, 0x3a, 0x85); static efi_handle_t handle_controller; static efi_handle_t handle_child_controller[NUMBER_OF_CHILD_CONTROLLERS]; static efi_handle_t handle_driver; +static efi_handle_t handle_driver2; + +static bool allow_remove;
/* - * Count child controllers + * Count controllers * - * @handle handle on which child controllers are installed + * @handle handle on which controllers and children are installed * @protocol protocol for which the child controllers were installed * @count number of child controllers + * @children: count children only * Return: status code */ -static efi_status_t count_child_controllers(efi_handle_t handle, - efi_guid_t *protocol, - efi_uintn_t *count) +static efi_status_t count_controllers(efi_handle_t handle, efi_guid_t *protocol, + efi_uintn_t *count, bool children) { efi_status_t ret; efi_uintn_t entry_count; @@ -52,10 +62,14 @@ static efi_status_t count_child_controllers(efi_handle_t handle, return ret; if (!entry_count) return EFI_SUCCESS; - while (entry_count) { - if (entry_buffer[--entry_count].attributes & - EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) - ++*count; + if (!children) { + *count = entry_count; + } else { + while (entry_count) { + if (entry_buffer[--entry_count].attributes & + EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) + ++*count; + } } ret = boottime->free_pool(entry_buffer); if (ret != EFI_SUCCESS) @@ -153,6 +167,22 @@ static efi_status_t EFIAPI start( return EFI_ST_FAILURE; } } + + /* Attach driver to controller */ + ret = boottime->open_protocol(controller_handle, &guid_controller2, + &interface, handle_driver2, + controller_handle, + EFI_OPEN_PROTOCOL_BY_DRIVER); + switch (ret) { + case EFI_SUCCESS: + return EFI_SUCCESS; + case EFI_ALREADY_STARTED: + case EFI_ACCESS_DENIED: + return ret; + default: + return EFI_UNSUPPORTED; + } + return ret; }
@@ -249,6 +279,50 @@ static efi_status_t EFIAPI stop( return EFI_SUCCESS; }
+/* + * Check if the driver supports the controller. + * + * @this driver binding protocol + * @controller_handle handle of the controller + * @remaining_device_path path specifying the child controller + * Return: status code + */ +static efi_status_t EFIAPI supported2(struct efi_driver_binding_protocol *this, + efi_handle_t controller_handle, + struct efi_device_path *remaining_dp) +{ + return EFI_SUCCESS; +} + +/* + * Refuse to disconnect the controller. + * + * @this driver binding protocol + * @controller_handle handle of the controller + * @number_of_children number of child controllers to remove + * @child_handle_buffer handles of the child controllers to remove + * Return: status code + */ +static efi_status_t EFIAPI stop2(struct efi_driver_binding_protocol *this, + efi_handle_t controller_handle, + size_t number_of_children, + efi_handle_t *child_handle_buffer) +{ + efi_status_t ret; + + if (!allow_remove) + return EFI_DEVICE_ERROR; + + /* Detach driver from controller */ + ret = boottime->close_protocol(controller_handle, &guid_controller2, + handle_driver2, controller_handle); + if (ret != EFI_SUCCESS) { + efi_st_error("Cannot close protocol\n"); + return ret; + } + return EFI_SUCCESS; +} + /* Driver binding protocol interface */ static struct efi_driver_binding_protocol binding_interface = { supported, @@ -259,6 +333,15 @@ static struct efi_driver_binding_protocol binding_interface = { NULL, };
+static struct efi_driver_binding_protocol binding_interface2 = { + supported2, + start, + stop2, + 0xffffffff, + NULL, + NULL, + }; + /* * Setup unit test. * @@ -273,6 +356,18 @@ static int setup(const efi_handle_t img_handle, boottime = systable->boottime; handle_controller = NULL; handle_driver = NULL; + handle_driver2 = NULL; + allow_remove = false; + + /* Create controller handles */ + ret = boottime->install_protocol_interface(&handle_controller, + &guid_controller2, + EFI_NATIVE_INTERFACE, + &interface1); + if (ret != EFI_SUCCESS) { + efi_st_error("InstallProtocolInterface failed\n"); + return EFI_ST_FAILURE; + }
/* Create controller handle */ ret = boottime->install_protocol_interface( @@ -291,6 +386,16 @@ static int setup(const efi_handle_t img_handle, return EFI_ST_FAILURE; }
+ /* Create driver handle which will fail on stop() */ + ret = boottime->install_protocol_interface(&handle_driver2, + &guid_driver_binding_protocol, + EFI_NATIVE_INTERFACE, + &binding_interface2); + if (ret != EFI_SUCCESS) { + efi_st_error("InstallProtocolInterface failed\n"); + return EFI_ST_FAILURE; + } + return EFI_ST_SUCCESS; }
@@ -310,7 +415,7 @@ static int setup(const efi_handle_t img_handle, */ static int execute(void) { - efi_status_t ret; + efi_status_t ret = EFI_SUCCESS; efi_uintn_t count;
/* Connect controller to driver */ @@ -319,9 +424,79 @@ static int execute(void) efi_st_error("Failed to connect controller\n"); return EFI_ST_FAILURE; } + /* Check number of drivers */ + ret = count_controllers(handle_controller, &guid_controller2, + &count, false); + if (ret != EFI_SUCCESS || count != CONTROLLER2_DRIVERS) { + efi_st_error("Failed to connect controller\n"); + return EFI_ST_FAILURE; + } + ret = count_controllers(handle_controller, &guid_controller, + &count, false); + if (ret != EFI_SUCCESS || count != CONTROLLER1_DRIVERS) { + efi_st_error("Failed to connect controller\n"); + return EFI_ST_FAILURE; + } + + /* Try to uninstall controller protocol which doesn't exist */ + ret = boottime->uninstall_protocol_interface(handle_controller, + &guid_controller2, + &interface2); + if (ret != EFI_NOT_FOUND) { + efi_st_error("Interface not checked when uninstalling protocol\n"); + return EFI_ST_FAILURE; + } + + /* Try to uninstall controller protocol which can't be stopped */ + ret = boottime->uninstall_protocol_interface(handle_controller, + &guid_controller, + &interface1); + if (ret != EFI_DEVICE_ERROR) { + efi_st_error("EFI_DRIVER_BINDING_PROTOCOL.Stop() not checked\n"); + return EFI_ST_FAILURE; + } + /* Check number of drivers again to make sure controolers reconnected */ + ret = count_controllers(handle_controller, &guid_controller2, + &count, false); + if (ret != EFI_SUCCESS || count != CONTROLLER2_DRIVERS) { + efi_st_error("Failed to reconnect controller\n"); + return EFI_ST_FAILURE; + } + ret = count_controllers(handle_controller, &guid_controller, + &count, false); + if (ret != EFI_SUCCESS || count != CONTROLLER1_DRIVERS) { + efi_st_error("Failed to reconnect controller\n"); + return EFI_ST_FAILURE; + } + + /* Try to uninstall controller protocol which can't be stopped */ + ret = boottime->uninstall_protocol_interface(handle_controller, + &guid_controller2, + &interface1); + if (ret != EFI_DEVICE_ERROR) { + efi_st_error("EFI_DRIVER_BINDING_PROTOCOL.Stop() not checked\n"); + return EFI_ST_FAILURE; + } + + /* Check number of drivers again to make sure controllers reconnected */ + ret = count_controllers(handle_controller, &guid_controller2, + &count, false); + if (ret != EFI_SUCCESS || count != CONTROLLER2_DRIVERS) { + efi_st_error("Failed to reconnect controller\n"); + return EFI_ST_FAILURE; + } + ret = count_controllers(handle_controller, &guid_controller, + &count, false); + if (ret != EFI_SUCCESS || count != CONTROLLER1_DRIVERS) { + efi_st_error("Failed to reconnect controller\n"); + return EFI_ST_FAILURE; + } + + allow_remove = true; + /* Check number of child controllers */ - ret = count_child_controllers(handle_controller, &guid_controller, - &count); + ret = count_controllers(handle_controller, &guid_controller, + &count, true); if (ret != EFI_SUCCESS || count != NUMBER_OF_CHILD_CONTROLLERS) { efi_st_error("Number of children %u != %u\n", (unsigned int)count, NUMBER_OF_CHILD_CONTROLLERS); @@ -335,8 +510,8 @@ static int execute(void) return EFI_ST_FAILURE; } /* Check number of child controllers */ - ret = count_child_controllers(handle_controller, &guid_controller, - &count); + ret = count_controllers(handle_controller, &guid_controller, + &count, true); if (ret != EFI_SUCCESS || count != NUMBER_OF_CHILD_CONTROLLERS - 1) { efi_st_error("Destroying single child controller failed\n"); return EFI_ST_FAILURE; @@ -348,8 +523,8 @@ static int execute(void) return EFI_ST_FAILURE; } /* Check number of child controllers */ - ret = count_child_controllers(handle_controller, &guid_controller, - &count); + ret = count_controllers(handle_controller, &guid_controller, + &count, true); if (ret != EFI_SUCCESS || count) { efi_st_error("Destroying child controllers failed\n"); return EFI_ST_FAILURE; @@ -362,8 +537,8 @@ static int execute(void) return EFI_ST_FAILURE; } /* Check number of child controllers */ - ret = count_child_controllers(handle_controller, &guid_controller, - &count); + ret = count_controllers(handle_controller, &guid_controller, + &count, true); if (ret != EFI_SUCCESS || count != NUMBER_OF_CHILD_CONTROLLERS) { efi_st_error("Number of children %u != %u\n", (unsigned int)count, NUMBER_OF_CHILD_CONTROLLERS); @@ -387,8 +562,8 @@ static int execute(void) return EFI_ST_FAILURE; } /* Check number of child controllers */ - ret = count_child_controllers(handle_controller, &guid_controller, - &count); + ret = count_controllers(handle_controller, &guid_controller, + &count, true); if (ret != EFI_SUCCESS || count != NUMBER_OF_CHILD_CONTROLLERS) { efi_st_error("Number of children %u != %u\n", (unsigned int)count, NUMBER_OF_CHILD_CONTROLLERS); @@ -402,14 +577,13 @@ static int execute(void) return EFI_ST_FAILURE; } /* Check number of child controllers */ - ret = count_child_controllers(handle_controller, &guid_controller, - &count); + ret = count_controllers(handle_controller, &guid_controller, + &count, true); if (ret == EFI_SUCCESS || count != 0) { efi_st_error("Uninstall failed\n"); return EFI_ST_FAILURE; }
- return EFI_ST_SUCCESS; }
@@ -420,6 +594,7 @@ static int execute(void) static int teardown(void) { efi_status_t ret; + /* Uninstall binding protocol */ ret = boottime->uninstall_protocol_interface(handle_driver, &guid_driver_binding_protocol,

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 ==
participants (2)
-
Heinrich Schuchardt
-
Ilias Apalodimas