Menu
▾
▴
[Libusb-devel] [PATCH v2] Prevent transfer from being submitted twice or improperly canceled
|
From: David M. <dc...@MI...> - 2009-01-12 05:46:26
|
From: David Moore <dc...@ac...>
This ensures that tpriv->urbs and tpriv->iso_urbs are always set to NULL
whenever a transfer is not submitted. In this way, submit_*_transfer()
and cancel_*_transfer() can error check to ensure that the transfer is
in the correct state to be either submitted or canceled, preventing
potential memory leaks or double frees.
Signed-off-by: David Moore <dc...@ac...>
---
This second version of the patch also deals with control transfers,
updates the documentation, and returns better error codes.
libusb/io.c | 8 +++-----
libusb/os/linux_usbfs.c | 38 ++++++++++++++++++++++++++++++++------
2 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index 89a94a2..320bc4b 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1101,12 +1101,10 @@ API_EXPORTED void libusb_free_transfer(struct libusb_transfer *transfer)
* Submit a transfer. This function will fire off the USB transfer and then
* return immediately.
*
- * It is undefined behaviour to submit a transfer that has already been
- * submitted but has not yet completed.
- *
* \param transfer the transfer to submit
* \returns 0 on success
* \returns LIBUSB_ERROR_NO_DEVICE if the device has been disconnected
+ * \returns LIBUSB_ERROR_BUSY if the transfer has already been submitted.
* \returns another LIBUSB_ERROR code on other failure
*/
API_EXPORTED int libusb_submit_transfer(struct libusb_transfer *transfer)
@@ -1134,8 +1132,6 @@ API_EXPORTED int libusb_submit_transfer(struct libusb_transfer *transfer)
/** \ingroup asyncio
* Asynchronously cancel a previously submitted transfer.
- * It is undefined behaviour to call this function on a transfer that is
- * already being cancelled or has already completed.
* This function returns immediately, but this does not indicate cancellation
* is complete. Your callback function will be invoked at some later time
* with a transfer status of
@@ -1144,6 +1140,8 @@ API_EXPORTED int libusb_submit_transfer(struct libusb_transfer *transfer)
*
* \param transfer the transfer to cancel
* \returns 0 on success
+ * \returns LIBUSB_ERROR_NOT_FOUND if the transfer is already complete or
+ * cancelled.
* \returns a LIBUSB_ERROR code on failure
*/
API_EXPORTED int libusb_cancel_transfer(struct libusb_transfer *transfer)
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index e015aff..46e770c 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -1245,6 +1245,7 @@ static void free_iso_urbs(struct linux_transfer_priv *tpriv)
}
free(tpriv->iso_urbs);
+ tpriv->iso_urbs = NULL;
}
static int submit_bulk_transfer(struct usbi_transfer *itransfer,
@@ -1260,6 +1261,9 @@ static int submit_bulk_transfer(struct usbi_transfer *itransfer,
int i;
size_t alloc_size;
+ if (tpriv->urbs)
+ return LIBUSB_ERROR_BUSY;
+
/* usbfs places a 16kb limit on bulk URBs. we divide up larger requests
* into smaller units to meet such restriction, then fire off all the
* units at once. it would be simpler if we just fired one unit at a time,
@@ -1311,6 +1315,7 @@ static int submit_bulk_transfer(struct usbi_transfer *itransfer,
if (i == 0) {
usbi_dbg("first URB failed, easy peasy");
free(urbs);
+ tpriv->urbs = NULL;
return r;
}
@@ -1365,6 +1370,9 @@ static int submit_iso_transfer(struct usbi_transfer *itransfer)
unsigned int packet_len;
unsigned char *urb_buffer = transfer->buffer;
+ if (tpriv->iso_urbs)
+ return LIBUSB_ERROR_BUSY;
+
/* usbfs places a 32kb limit on iso URBs. we divide up larger requests
* into smaller units to meet such restriction, then fire off all the
* units at once. it would be simpler if we just fired one unit at a time,
@@ -1512,6 +1520,9 @@ static int submit_control_transfer(struct usbi_transfer *itransfer)
struct usbfs_urb *urb;
int r;
+ if (tpriv->urbs)
+ return LIBUSB_ERROR_BUSY;
+
if (transfer->length - LIBUSB_CONTROL_SETUP_SIZE > MAX_CTRL_BUFFER_LENGTH)
return LIBUSB_ERROR_INVALID_PARAM;
@@ -1531,6 +1542,7 @@ static int submit_control_transfer(struct usbi_transfer *itransfer)
r = ioctl(dpriv->fd, IOCTL_USBFS_SUBMITURB, urb);
if (r < 0) {
free(urb);
+ tpriv->urbs = NULL;
if (errno == ENODEV)
return LIBUSB_ERROR_NO_DEVICE;
@@ -1571,6 +1583,9 @@ static int cancel_control_transfer(struct usbi_transfer *itransfer)
__device_handle_priv(transfer->dev_handle);
int r;
+ if (!tpriv->urbs)
+ return LIBUSB_ERROR_NOT_FOUND;
+
tpriv->reap_action = CANCELLED;
r = ioctl(dpriv->fd, IOCTL_USBFS_DISCARDURB, tpriv->urbs);
if(r) {
@@ -1587,7 +1602,7 @@ static int cancel_control_transfer(struct usbi_transfer *itransfer)
return 0;
}
-static void cancel_bulk_transfer(struct usbi_transfer *itransfer)
+static int cancel_bulk_transfer(struct usbi_transfer *itransfer)
{
struct linux_transfer_priv *tpriv = usbi_transfer_get_os_priv(itransfer);
struct libusb_transfer *transfer =
@@ -1596,6 +1611,9 @@ static void cancel_bulk_transfer(struct usbi_transfer *itransfer)
__device_handle_priv(transfer->dev_handle);
int i;
+ if (!tpriv->urbs)
+ return LIBUSB_ERROR_NOT_FOUND;
+
tpriv->reap_action = CANCELLED;
for (i = 0; i < tpriv->num_urbs; i++) {
int tmp = ioctl(dpriv->fd, IOCTL_USBFS_DISCARDURB, &tpriv->urbs[i]);
@@ -1603,9 +1621,10 @@ static void cancel_bulk_transfer(struct usbi_transfer *itransfer)
usbi_warn(TRANSFER_CTX(transfer),
"unrecognised discard errno %d", errno);
}
+ return 0;
}
-static void cancel_iso_transfer(struct usbi_transfer *itransfer)
+static int cancel_iso_transfer(struct usbi_transfer *itransfer)
{
struct linux_transfer_priv *tpriv = usbi_transfer_get_os_priv(itransfer);
struct libusb_transfer *transfer =
@@ -1614,6 +1633,9 @@ static void cancel_iso_transfer(struct usbi_transfer *itransfer)
__device_handle_priv(transfer->dev_handle);
int i;
+ if (!tpriv->iso_urbs)
+ return LIBUSB_ERROR_NOT_FOUND;
+
tpriv->reap_action = CANCELLED;
for (i = 0; i < tpriv->num_urbs; i++) {
int tmp = ioctl(dpriv->fd, IOCTL_USBFS_DISCARDURB, tpriv->iso_urbs[i]);
@@ -1621,6 +1643,7 @@ static void cancel_iso_transfer(struct usbi_transfer *itransfer)
usbi_warn(TRANSFER_CTX(transfer),
"unrecognised discard errno %d", errno);
}
+ return 0;
}
static int op_cancel_transfer(struct usbi_transfer *itransfer)
@@ -1633,11 +1656,9 @@ static int op_cancel_transfer(struct usbi_transfer *itransfer)
return cancel_control_transfer(itransfer);
case LIBUSB_TRANSFER_TYPE_BULK:
case LIBUSB_TRANSFER_TYPE_INTERRUPT:
- cancel_bulk_transfer(itransfer);
- return 0;
+ return cancel_bulk_transfer(itransfer);
case LIBUSB_TRANSFER_TYPE_ISOCHRONOUS:
- cancel_iso_transfer(itransfer);
- return 0;
+ return cancel_iso_transfer(itransfer);
default:
usbi_err(TRANSFER_CTX(transfer),
"unknown endpoint type %d", transfer->type);
@@ -1656,6 +1677,7 @@ static void op_clear_transfer_priv(struct usbi_transfer *itransfer)
case LIBUSB_TRANSFER_TYPE_BULK:
case LIBUSB_TRANSFER_TYPE_INTERRUPT:
free(tpriv->urbs);
+ tpriv->urbs = NULL;
break;
case LIBUSB_TRANSFER_TYPE_ISOCHRONOUS:
free_iso_urbs(tpriv);
@@ -1697,6 +1719,7 @@ static int handle_bulk_completion(struct usbi_transfer *itransfer,
usbi_dbg("CANCEL: last URB handled, reporting");
if (tpriv->reap_action == CANCELLED) {
free(tpriv->urbs);
+ tpriv->urbs = NULL;
usbi_handle_transfer_cancellation(itransfer);
return 0;
}
@@ -1762,6 +1785,7 @@ static int handle_bulk_completion(struct usbi_transfer *itransfer,
out:
free(tpriv->urbs);
+ tpriv->urbs = NULL;
usbi_handle_transfer_completion(itransfer, status);
return 0;
}
@@ -1860,6 +1884,7 @@ static int handle_control_completion(struct usbi_transfer *itransfer,
usbi_warn(ITRANSFER_CTX(itransfer),
"cancel: unrecognised urb status %d", urb->status);
free(tpriv->urbs);
+ tpriv->urbs = NULL;
usbi_handle_transfer_cancellation(itransfer);
return 0;
}
@@ -1887,6 +1912,7 @@ static int handle_control_completion(struct usbi_transfer *itransfer,
}
free(tpriv->urbs);
+ tpriv->urbs = NULL;
usbi_handle_transfer_completion(itransfer, status);
return 0;
}
--
1.6.0.6
|
