fix(usb_host): Give semaphore on attempted close of non-opened device

If you call *usb_host_device_close()* for a device that isn't open, the function exits early,
without giving back the semaphore it took, which causes any other call that tries to take
that semaphore to hang indefinitely.

Strangely, there's redundant handling of this condition, with two checks in a row that both handle
the case where `_check_client_opened_device(client_obj, dev_addr)` returns `false`:

```c
    HOST_CHECK_FROM_CRIT(_check_client_opened_device(client_obj, dev_addr), ESP_ERR_NOT_FOUND);
    if (!_check_client_opened_device(client_obj, dev_addr)) {
        // Client never opened this device
        ret = ESP_ERR_INVALID_STATE;
        HOST_EXIT_CRITICAL();
        goto exit;
    }
…
exit:
    xSemaphoreGive(p_host_lib_obj->constant.mux_lock);
    return ret;
```

The first line is the one that exits early, as HOST_CHECK_FROM_CRIT returns its second parameter
if its first parameter is false, without giving back the semaphore (although it does exit
the critical section).

The subsequent block handles the exact same case, except that it ensures the semaphore is given
back before returning. Currently, this block is never reached.

Perhaps the first check was added, then someone noticed the issue and added the second check,
but they forgot to remove the first one.

In any case, this PR removes the first check, so the second check can properly handle this case
by giving back the semaphore before returning.

This bug appears to have been present in the initial commit of the USB Host library to the ESP-IDF
repo: accbaee57c

Of course, if you never try to close a non-opened device, then you won't encounter it!
Unfortunately, I have some code that tried to do that, which is how I found the issue.
This commit is contained in:
Myk Melez
2025-03-19 16:27:28 -07:00
committed by igor.masar
parent 710ed9bcad
commit 71d098ed63

View File

@ -1012,7 +1012,6 @@ esp_err_t usb_host_device_close(usb_host_client_handle_t client_hdl, usb_device_
HOST_ENTER_CRITICAL();
uint8_t dev_addr;
ESP_ERROR_CHECK(usbh_dev_get_addr(dev_hdl, &dev_addr));
HOST_CHECK_FROM_CRIT(_check_client_opened_device(client_obj, dev_addr), ESP_ERR_NOT_FOUND);
if (!_check_client_opened_device(client_obj, dev_addr)) {
// Client never opened this device
ret = ESP_ERR_INVALID_STATE;