Windows: Correctly clear backend transfer private information
authorToby Gray <toby.gray@realvnc.com>
Thu, 11 Apr 2013 12:40:31 +0000 (13:40 +0100)
committerPete Batard <pete@akeo.ie>
Thu, 11 Apr 2013 22:22:19 +0000 (23:22 +0100)
* Without this fix if a transfer is reused then there is a period of
  time between it being adding to the flying transfer list and
  submitted where the fd value will be the old fd value.
* If this occurs at the same time as all of the following conditions
  then the incorrect transfer will be handled as having completed:
* The old fd value in the reused transfer has been recycled for a
  currently pending transfer.
* This other pending transfer has a later timeout than the reused
  transfer (so therefore comes later in the flying transfer list).
* The other pending transfer completes, therefore signalling the fd.

As the flying transfer list is examined in order when handling events,
the resubmitted transfer with the old fd value will be considered as
completed. This will generally cause a NULL pointer dereference as the
OVERLAPPED structure was already freed.

Also see:
http://libusbx.1081486.n5.nabble.com/Libusbx-devel-PATCH-Fix-NULL-pointer-dereference-in-Windows-and-WinCE-backends-when-reusing-transfers-tt1041.html

libusb/os/poll_windows.c
libusb/os/poll_windows.h
libusb/os/wince_usb.c
libusb/os/windows_usb.c
libusb/version_nano.h

index 6bb91af..7ed19ba 100644 (file)
@@ -403,17 +403,18 @@ static void _free_index(int _index)
  *
  * Note that the associated Windows handle is not closed by this call
  */
-void usbi_free_fd(int fd)
+void usbi_free_fd(struct winfd *wfd)
 {
        int _index;
 
        CHECK_INIT_POLLING;
 
-       _index = _fd_to_index_and_lock(fd);
+       _index = _fd_to_index_and_lock(wfd->fd);
        if (_index < 0) {
                return;
        }
        _free_index(_index);
+       *wfd = INVALID_WINFD;
        LeaveCriticalSection(&_poll_fd[_index].mutex);
 }
 
index 1e92dda..deed206 100644 (file)
@@ -94,7 +94,7 @@ void init_polling(void);
 void exit_polling(void);
 struct winfd usbi_create_fd(HANDLE handle, int access_mode, 
        struct usbi_transfer *transfer, cancel_transfer *cancel_fn);
-void usbi_free_fd(int fd);
+void usbi_free_fd(struct winfd* winfd);
 struct winfd fd_to_winfd(int fd);
 struct winfd handle_to_winfd(HANDLE handle);
 struct winfd overlapped_to_winfd(OVERLAPPED* overlapped);
index e4f7c7b..2e6eedd 100644 (file)
@@ -593,7 +593,7 @@ static void wince_clear_transfer_priv(
        // No need to cancel transfer as it is either complete or abandoned
        wfd.itransfer = NULL;
        CloseHandle(wfd.handle);
-       usbi_free_fd(transfer_priv->pollable_fd.fd);
+       usbi_free_fd(&transfer_priv->pollable_fd);
 }
 
 static int wince_cancel_transfer(
index 7c2428d..e0d2a1c 100644 (file)
@@ -1911,7 +1911,7 @@ static void windows_clear_transfer_priv(struct usbi_transfer *itransfer)
 {
        struct windows_transfer_priv *transfer_priv = (struct windows_transfer_priv*)usbi_transfer_get_os_priv(itransfer);
 
-       usbi_free_fd(transfer_priv->pollable_fd.fd);
+       usbi_free_fd(&transfer_priv->pollable_fd);
        safe_free(transfer_priv->hid_buffer);
        // When auto claim is in use, attempt to release the auto-claimed interface
        auto_release(itransfer);
@@ -2884,7 +2884,7 @@ static int winusbx_submit_control_transfer(int sub_api, struct usbi_transfer *it
          && (setup->request == LIBUSB_REQUEST_SET_CONFIGURATION) ) {
                if (setup->value != priv->active_config) {
                        usbi_warn(ctx, "cannot set configuration other than the default one");
-                       usbi_free_fd(wfd.fd);
+                       usbi_free_fd(&wfd);
                        return LIBUSB_ERROR_INVALID_PARAM;
                }
                wfd.overlapped->Internal = STATUS_COMPLETED_SYNCHRONOUSLY;
@@ -2893,7 +2893,7 @@ static int winusbx_submit_control_transfer(int sub_api, struct usbi_transfer *it
                if (!WinUSBX[sub_api].ControlTransfer(wfd.handle, *setup, transfer->buffer + LIBUSB_CONTROL_SETUP_SIZE, size, NULL, wfd.overlapped)) {
                        if(GetLastError() != ERROR_IO_PENDING) {
                                usbi_warn(ctx, "ControlTransfer failed: %s", windows_error_str(0));
-                               usbi_free_fd(wfd.fd);
+                               usbi_free_fd(&wfd);
                                return LIBUSB_ERROR_IO;
                        }
                } else {
@@ -2978,7 +2978,7 @@ static int winusbx_submit_bulk_transfer(int sub_api, struct usbi_transfer *itran
        if (!ret) {
                if(GetLastError() != ERROR_IO_PENDING) {
                        usbi_err(ctx, "ReadPipe/WritePipe failed: %s", windows_error_str(0));
-                       usbi_free_fd(wfd.fd);
+                       usbi_free_fd(&wfd);
                        return LIBUSB_ERROR_IO;
                }
        } else {
@@ -3087,7 +3087,7 @@ static int winusbx_reset_device(int sub_api, struct libusb_device_handle *dev_ha
                {
                        // Cancel any pollable I/O
                        usbi_remove_pollfd(ctx, wfd.fd);
-                       usbi_free_fd(wfd.fd);
+                       usbi_free_fd(&wfd);
                        wfd = handle_to_winfd(winusb_handle);
                }
 
@@ -3964,7 +3964,7 @@ static int hid_submit_control_transfer(int sub_api, struct usbi_transfer *itrans
                transfer_priv->pollable_fd = wfd;
                transfer_priv->interface_number = (uint8_t)current_interface;
        } else {
-               usbi_free_fd(wfd.fd);
+               usbi_free_fd(&wfd);
        }
 
        return r;
@@ -4037,7 +4037,7 @@ static int hid_submit_bulk_transfer(int sub_api, struct usbi_transfer *itransfer
        if (!ret) {
                if (GetLastError() != ERROR_IO_PENDING) {
                        usbi_err(ctx, "HID transfer failed: %s", windows_error_str(0));
-                       usbi_free_fd(wfd.fd);
+                       usbi_free_fd(&wfd);
                        safe_free(transfer_priv->hid_buffer);
                        return LIBUSB_ERROR_IO;
                }
index d862eee..326c7f4 100644 (file)
@@ -1 +1 @@
-#define LIBUSB_NANO 10643
+#define LIBUSB_NANO 10644