From b51c743e4210756a98f4f60c69a34745e4b27a55 Mon Sep 17 00:00:00 2001 From: Chris Dickens Date: Fri, 15 Jan 2021 17:18:34 -0800 Subject: [PATCH] Windows: Filter out non-libusb I/O completions The I/O completion port thread gets notified of every I/O operation that completes for a given HANDLE, but not all I/O operations originate from within libusb. For example, libusbK performs a number of I/O control calls during initialization and specific device operations, such as setting an interface alternate setting. These non-libusb operations need to be ignored as the OVERLAPPED associated with such operations is not tied to a libusb_transfer structure. Resolve this situation by using the libusb device handle as the I/O completion port's unique key and keeping a list of active transfers for each device handle. When the I/O competion port thread is notified of an I/O completion, it will first match the OVERLAPPED to an outstanding transfer before acting on it. Closes #844 Signed-off-by: Chris Dickens --- libusb/os/windows_common.c | 46 +++++++++++++++++++++++++++++++++++++++++--- libusb/os/windows_common.h | 22 ++++++++++++++++++--- libusb/os/windows_usbdk.c | 2 +- libusb/os/windows_winusb.c | 48 +++++++++++++++++++++++----------------------- libusb/version_nano.h | 2 +- 5 files changed, 88 insertions(+), 32 deletions(-) diff --git a/libusb/os/windows_common.c b/libusb/os/windows_common.c index efb746b..a5e2a7c 100644 --- a/libusb/os/windows_common.c +++ b/libusb/os/windows_common.c @@ -415,8 +415,11 @@ static unsigned __stdcall windows_iocp_thread(void *arg) DWORD num_bytes; ULONG_PTR completion_key; OVERLAPPED *overlapped; + struct libusb_device_handle *dev_handle; + struct windows_device_handle_priv *handle_priv; struct windows_transfer_priv *transfer_priv; struct usbi_transfer *itransfer; + bool found; usbi_dbg("I/O completion thread started"); @@ -434,7 +437,29 @@ static unsigned __stdcall windows_iocp_thread(void *arg) break; } - transfer_priv = container_of(overlapped, struct windows_transfer_priv, overlapped); + // Find the transfer associated with the OVERLAPPED that just completed. + // If we cannot find a match, the I/O operation originated from outside of libusb + // (e.g. within libusbK) and we need to ignore it. + dev_handle = (struct libusb_device_handle *)completion_key; + handle_priv = usbi_get_device_handle_priv(dev_handle); + found = false; + usbi_mutex_lock(&dev_handle->lock); + list_for_each_entry(transfer_priv, &handle_priv->active_transfers, list, struct windows_transfer_priv) { + if (overlapped == &transfer_priv->overlapped) { + // This OVERLAPPED belongs to us, remove the transfer from the device handle's list + list_del(&transfer_priv->list); + found = true; + break; + } + } + usbi_mutex_unlock(&dev_handle->lock); + + if (!found) { + usbi_dbg("ignoring overlapped %p for handle %p (device %u.%u)", + overlapped, dev_handle, dev_handle->dev->bus_number, dev_handle->dev->device_address); + continue; + } + itransfer = (struct usbi_transfer *)((unsigned char *)transfer_priv + PTR_ALIGN(sizeof(*transfer_priv))); usbi_dbg("transfer %p completed, length %lu", USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer), ULONG_CAST(num_bytes)); @@ -613,6 +638,9 @@ static int windows_get_device_list(struct libusb_context *ctx, struct discovered static int windows_open(struct libusb_device_handle *dev_handle) { struct windows_context_priv *priv = usbi_get_context_priv(HANDLE_CTX(dev_handle)); + struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + + list_init(&handle_priv->active_transfers); return priv->backend->open(dev_handle); } @@ -697,8 +725,10 @@ static void windows_destroy_device(struct libusb_device *dev) static int windows_submit_transfer(struct usbi_transfer *itransfer) { struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); - struct libusb_context *ctx = TRANSFER_CTX(transfer); + struct libusb_device_handle *dev_handle = transfer->dev_handle; + struct libusb_context *ctx = HANDLE_CTX(dev_handle); struct windows_context_priv *priv = usbi_get_context_priv(ctx); + struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); struct windows_transfer_priv *transfer_priv = usbi_get_transfer_priv(itransfer); int r; @@ -721,8 +751,18 @@ static int windows_submit_transfer(struct usbi_transfer *itransfer) transfer_priv->handle = NULL; } + // Add transfer to the device handle's list + usbi_mutex_lock(&dev_handle->lock); + list_add_tail(&transfer_priv->list, &handle_priv->active_transfers); + usbi_mutex_unlock(&dev_handle->lock); + r = priv->backend->submit_transfer(itransfer); if (r != LIBUSB_SUCCESS) { + // Remove the unsuccessful transfer from the device handle's list + usbi_mutex_lock(&dev_handle->lock); + list_del(&transfer_priv->list); + usbi_mutex_unlock(&dev_handle->lock); + // Always call the backend's clear_transfer_priv() function on failure priv->backend->clear_transfer_priv(itransfer); transfer_priv->handle = NULL; @@ -880,6 +920,6 @@ const struct usbi_os_backend usbi_backend = { windows_handle_transfer_completion, sizeof(struct windows_context_priv), sizeof(union windows_device_priv), - sizeof(union windows_device_handle_priv), + sizeof(struct windows_device_handle_priv), sizeof(struct windows_transfer_priv), }; diff --git a/libusb/os/windows_common.h b/libusb/os/windows_common.h index d9958c6..c84f1f9 100644 --- a/libusb/os/windows_common.h +++ b/libusb/os/windows_common.h @@ -338,20 +338,36 @@ union windows_device_priv { struct winusb_device_priv winusb_priv; }; -union windows_device_handle_priv { - struct usbdk_device_handle_priv usbdk_priv; - struct winusb_device_handle_priv winusb_priv; +struct windows_device_handle_priv { + struct list_head active_transfers; + union { + struct usbdk_device_handle_priv usbdk_priv; + struct winusb_device_handle_priv winusb_priv; + }; }; struct windows_transfer_priv { OVERLAPPED overlapped; HANDLE handle; + struct list_head list; union { struct usbdk_transfer_priv usbdk_priv; struct winusb_transfer_priv winusb_priv; }; }; +static inline struct usbdk_device_handle_priv *get_usbdk_device_handle_priv(struct libusb_device_handle *dev_handle) +{ + struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + return &handle_priv->usbdk_priv; +} + +static inline struct winusb_device_handle_priv *get_winusb_device_handle_priv(struct libusb_device_handle *dev_handle) +{ + struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + return &handle_priv->winusb_priv; +} + static inline OVERLAPPED *get_transfer_priv_overlapped(struct usbi_transfer *itransfer) { struct windows_transfer_priv *transfer_priv = usbi_get_transfer_priv(itransfer); diff --git a/libusb/os/windows_usbdk.c b/libusb/os/windows_usbdk.c index 76aba16..9f52b48 100644 --- a/libusb/os/windows_usbdk.c +++ b/libusb/os/windows_usbdk.c @@ -413,7 +413,7 @@ static int usbdk_open(struct libusb_device_handle *dev_handle) device_priv->system_handle = usbdk_helper.GetRedirectorSystemHandle(device_priv->redirector_handle); - if (CreateIoCompletionPort(device_priv->system_handle, priv->completion_port, 0, 0) == NULL) { + if (CreateIoCompletionPort(device_priv->system_handle, priv->completion_port, (ULONG_PTR)dev_handle, 0) == NULL) { usbi_err(ctx, "failed to associate handle to I/O completion port: %s", windows_error_str(0)); usbdk_helper.StopRedirect(device_priv->redirector_handle); device_priv->system_handle = NULL; diff --git a/libusb/os/windows_winusb.c b/libusb/os/windows_winusb.c index ea51038..a370ede 100644 --- a/libusb/os/windows_winusb.c +++ b/libusb/os/windows_winusb.c @@ -488,9 +488,9 @@ static int get_interface_by_endpoint(struct libusb_config_descriptor *conf_desc, /* * Open a device and associate the HANDLE with the context's I/O completion port */ -static HANDLE windows_open(struct libusb_device *dev, const char *path, DWORD access) +static HANDLE windows_open(struct libusb_device_handle *dev_handle, const char *path, DWORD access) { - struct libusb_context *ctx = DEVICE_CTX(dev); + struct libusb_context *ctx = HANDLE_CTX(dev_handle); struct windows_context_priv *priv = usbi_get_context_priv(ctx); HANDLE handle; @@ -498,7 +498,7 @@ static HANDLE windows_open(struct libusb_device *dev, const char *path, DWORD ac if (handle == INVALID_HANDLE_VALUE) return handle; - if (CreateIoCompletionPort(handle, priv->completion_port, 0, 0) == NULL) { + if (CreateIoCompletionPort(handle, priv->completion_port, (ULONG_PTR)dev_handle, 0) == NULL) { usbi_err(ctx, "failed to associate handle to I/O completion port: %s", windows_error_str(0)); CloseHandle(handle); return INVALID_HANDLE_VALUE; @@ -593,7 +593,7 @@ static int get_sub_api(char *driver, int api) static int auto_claim(struct libusb_transfer *transfer, int *interface_number, int api_type) { struct winusb_device_handle_priv *handle_priv = - usbi_get_device_handle_priv(transfer->dev_handle); + get_winusb_device_handle_priv(transfer->dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev); int current_interface = *interface_number; int r = LIBUSB_SUCCESS; @@ -640,7 +640,7 @@ static void auto_release(struct usbi_transfer *itransfer) struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer); struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); libusb_device_handle *dev_handle = transfer->dev_handle; - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); int r; usbi_mutex_lock(&autoclaim_lock); @@ -2375,7 +2375,7 @@ static void winusbx_exit(void) static int winusbx_open(int sub_api, struct libusb_device_handle *dev_handle) { struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); HANDLE file_handle; int i; @@ -2385,7 +2385,7 @@ static int winusbx_open(int sub_api, struct libusb_device_handle *dev_handle) for (i = 0; i < USB_MAXINTERFACES; i++) { if ((priv->usb_interface[i].path != NULL) && (priv->usb_interface[i].apib->id == USB_API_WINUSBX)) { - file_handle = windows_open(dev_handle->dev, priv->usb_interface[i].path, GENERIC_READ | GENERIC_WRITE); + file_handle = windows_open(dev_handle, priv->usb_interface[i].path, GENERIC_READ | GENERIC_WRITE); if (file_handle == INVALID_HANDLE_VALUE) { usbi_err(HANDLE_CTX(dev_handle), "could not open device %s (interface %d): %s", priv->usb_interface[i].path, i, windows_error_str(0)); switch (GetLastError()) { @@ -2407,7 +2407,7 @@ static int winusbx_open(int sub_api, struct libusb_device_handle *dev_handle) static void winusbx_close(int sub_api, struct libusb_device_handle *dev_handle) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); HANDLE handle; int i; @@ -2452,7 +2452,7 @@ static void winusbx_close(int sub_api, struct libusb_device_handle *dev_handle) static int winusbx_configure_endpoints(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); HANDLE winusb_handle = handle_priv->interface_handle[iface].api_handle; UCHAR policy; @@ -2506,7 +2506,7 @@ static int winusbx_configure_endpoints(int sub_api, struct libusb_device_handle static int winusbx_claim_interface(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface) { struct libusb_context *ctx = HANDLE_CTX(dev_handle); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); bool is_using_usbccgp = (priv->apib->id == USB_API_COMPOSITE); HDEVINFO dev_info; @@ -2557,7 +2557,7 @@ static int winusbx_claim_interface(int sub_api, struct libusb_device_handle *dev *dev_interface_path_guid_start = '\0'; if (strncmp(dev_interface_path, priv->usb_interface[iface].path, strlen(dev_interface_path)) == 0) { - file_handle = windows_open(dev_handle->dev, filter_path, GENERIC_READ | GENERIC_WRITE); + file_handle = windows_open(dev_handle, filter_path, GENERIC_READ | GENERIC_WRITE); if (file_handle != INVALID_HANDLE_VALUE) { if (WinUSBX[sub_api].Initialize(file_handle, &winusb_handle)) { // Replace the existing file handle with the working one @@ -2622,7 +2622,7 @@ static int winusbx_claim_interface(int sub_api, struct libusb_device_handle *dev static int winusbx_release_interface(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); HANDLE winusb_handle; @@ -2643,7 +2643,7 @@ static int winusbx_release_interface(int sub_api, struct libusb_device_handle *d */ static int get_valid_interface(struct libusb_device_handle *dev_handle, int api_id) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); int i; @@ -2667,7 +2667,7 @@ static int get_valid_interface(struct libusb_device_handle *dev_handle, int api_ */ static int check_valid_interface(struct libusb_device_handle *dev_handle, unsigned short interface, int api_id) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); if (interface >= USB_MAXINTERFACES) @@ -2714,7 +2714,7 @@ static int winusbx_submit_control_transfer(int sub_api, struct usbi_transfer *it struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev); struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle); PWINUSB_SETUP_PACKET setup = (PWINUSB_SETUP_PACKET)transfer->buffer; ULONG size; HANDLE winusb_handle; @@ -2770,7 +2770,7 @@ static int winusbx_submit_control_transfer(int sub_api, struct usbi_transfer *it static int winusbx_set_interface_altsetting(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface, uint8_t altsetting) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); HANDLE winusb_handle; @@ -2826,7 +2826,7 @@ static int winusbx_submit_iso_transfer(int sub_api, struct usbi_transfer *itrans { struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev); HANDLE winusb_handle; OVERLAPPED *overlapped; @@ -3016,7 +3016,7 @@ static int winusbx_submit_bulk_transfer(int sub_api, struct usbi_transfer *itran { struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev); HANDLE winusb_handle; OVERLAPPED *overlapped; @@ -3057,7 +3057,7 @@ static int winusbx_submit_bulk_transfer(int sub_api, struct usbi_transfer *itran static int winusbx_clear_halt(int sub_api, struct libusb_device_handle *dev_handle, unsigned char endpoint) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); HANDLE winusb_handle; int current_interface; @@ -3084,7 +3084,7 @@ static int winusbx_clear_halt(int sub_api, struct libusb_device_handle *dev_hand static int winusbx_cancel_transfer(int sub_api, struct usbi_transfer *itransfer) { struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle); struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer); struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev); int current_interface = transfer_priv->interface_number; @@ -3114,7 +3114,7 @@ static int winusbx_cancel_transfer(int sub_api, struct usbi_transfer *itransfer) // TODO: (post hotplug): see if we can force eject the device and redetect it (reuse hotplug?) static int winusbx_reset_device(int sub_api, struct libusb_device_handle *dev_handle) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); HANDLE winusb_handle; int i, j; @@ -4339,7 +4339,7 @@ static int composite_submit_control_transfer(int sub_api, struct usbi_transfer * static int composite_submit_bulk_transfer(int sub_api, struct usbi_transfer *itransfer) { struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev); int current_interface; @@ -4360,7 +4360,7 @@ static int composite_submit_bulk_transfer(int sub_api, struct usbi_transfer *itr static int composite_submit_iso_transfer(int sub_api, struct usbi_transfer *itransfer) { struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev); int current_interface; @@ -4380,7 +4380,7 @@ static int composite_submit_iso_transfer(int sub_api, struct usbi_transfer *itra static int composite_clear_halt(int sub_api, struct libusb_device_handle *dev_handle, unsigned char endpoint) { - struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle); + struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle); struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev); int current_interface; diff --git a/libusb/version_nano.h b/libusb/version_nano.h index b8e72d6..4227ce9 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 11598 +#define LIBUSB_NANO 11599 -- 2.7.4