Windows: Correctly clear backend transfer private information

* 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
This commit is contained in:
Toby Gray
2013-04-11 13:40:31 +01:00
committed by Pete Batard
parent a66cfa05fe
commit e68c666792
5 changed files with 13 additions and 12 deletions
+3 -2
View 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);
}
+1 -1
View 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);
+1 -1
View 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(
+7 -7
View 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;
}
+1 -1
View File
@@ -1 +1 @@
#define LIBUSB_NANO 10643
#define LIBUSB_NANO 10644