Changeset a6afb4c in mainline for uspace/lib/usbhost/src/bus.c

Timestamp:

2018-01-23T14:02:35Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

4db49344

Parents:

e7e1fd3

git-author:

Ondřej Hlavatý <aearsis@…> (2018-01-23 13:35:50)

git-committer:

Ondřej Hlavatý <aearsis@…> (2018-01-23 14:02:35)

Message:

usbhost: check validity of arguments, cleanup

File:

• uspace/lib/usbhost/src/bus.c (modified) (13 diffs)

uspace/lib/usbhost/src/bus.c

@@ -42 +42 @@
 #include <errno.h>
 #include <mem.h>
+#include <macros.h>
 #include <stdio.h>
 #include <str_error.h>
@@ -345 +346 @@
  * For different arguments, the result is stable but not defined.
  */
-static int bus_endpoint_index(usb_endpoint_t ep, usb_direction_t dir)
+static size_t bus_endpoint_index(usb_endpoint_t ep, usb_direction_t dir)
 {
         return 2 * ep + (dir == USB_DIRECTION_OUT);
@@ -384 +385 @@
         endpoint_add_ref(ep);
 
+        const size_t idx = bus_endpoint_index(ep->endpoint, ep->direction);
+        if (idx >= ARRAY_SIZE(device->endpoints)) {
+                usb_log_warning("Invalid endpoint description (ep no %u out of "
+                    "bounds)", ep->endpoint);
+                goto drop;
+        }
+
         if (ep->max_transfer_size == 0) {
                 usb_log_warning("Invalid endpoint description (mps %zu, "
                         "%u packets)", ep->max_packet_size, ep->packets_per_uframe);
-                /* Bus reference */
-                endpoint_del_ref(ep);
-                return EINVAL;
+                goto drop;
         }
 
@@ -397 +403 @@
             usb_str_direction(ep->direction),
             ep->max_transfer_size);
-
-        const int idx = bus_endpoint_index(ep->endpoint, ep->direction);
 
         fibril_mutex_lock(&device->guard);
@@ -423 +427 @@
 
         return EOK;
+drop:
+        /* Bus reference */
+        endpoint_del_ref(ep);
+        return EINVAL;
 }
 
@@ -428 +436 @@
  * Search for an endpoint. Returns a reference.
  */
-endpoint_t *bus_find_endpoint(device_t *device, usb_endpoint_t endpoint, usb_direction_t dir)
+endpoint_t *bus_find_endpoint(device_t *device, usb_endpoint_t endpoint,
+    usb_direction_t dir)
 {
         assert(device);
 
-        const int idx = bus_endpoint_index(endpoint, dir);
-        const int ctrl_idx = bus_endpoint_index(endpoint, USB_DIRECTION_BOTH);
+        const size_t idx = bus_endpoint_index(endpoint, dir);
+        const size_t ctrl_idx = bus_endpoint_index(endpoint, USB_DIRECTION_BOTH);
+
+        endpoint_t *ep = NULL;
 
         fibril_mutex_lock(&device->guard);
-        endpoint_t *ep = device->endpoints[idx];
+        if (idx < ARRAY_SIZE(device->endpoints))
+                ep = device->endpoints[idx];
         /*
          * If the endpoint was not found, it's still possible it is a control
          * endpoint having direction BOTH.
          */
-        if (!ep) {
+        if (!ep && ctrl_idx < ARRAY_SIZE(device->endpoints)) {
                 ep = device->endpoints[ctrl_idx];
                 if (ep && ep->transfer_type != USB_TRANSFER_CONTROL)
@@ -478 +490 @@
             ep->max_transfer_size);
 
-        const int idx = bus_endpoint_index(ep->endpoint, ep->direction);
+        const size_t idx = bus_endpoint_index(ep->endpoint, ep->direction);
+
+        if (idx >= ARRAY_SIZE(device->endpoints))
+                return EINVAL;
 
         fibril_mutex_lock(&device->guard);
@@ -495 +510 @@
 
 /**
- * Reserve the default address on the bus. Also, report the speed of the device
- * that is listening on the default address.
- *
- * The speed is then used for devices enumerated while the address is reserved.
+ * Reserve the default address on the bus for the specified device (hub).
  */
 int bus_reserve_default_address(bus_t *bus, device_t *dev)
@@ -564 +576 @@
         assert(ep->device == device);
 
-        const int err = endpoint_send_batch(ep, target, direction, data, size, setup_data,
-            on_complete, arg, name);
+        /*
+         * This method is already callable from HC only, so we can force these
+         * conditions harder.
+         * Invalid values from devices shall be caught on DDF interface already.
+         */
+        assert(usb_target_is_valid(&target));
+        assert(usb_direction_is_valid(direction));
+        assert(direction != USB_DIRECTION_BOTH);
+        assert(size == 0 || data != NULL);
+        assert(arg == NULL || on_complete != NULL);
+        assert(name);
+
+        const int err = endpoint_send_batch(ep, target, direction,
+            data, size, setup_data, on_complete, arg, name);
 
         /* Temporary reference */
@@ -573 +597 @@
 }
 
+/**
+ * A structure to pass data from the completion callback to the caller.
+ */
 typedef struct {
         fibril_mutex_t done_mtx;
         fibril_condvar_t done_cv;
-        unsigned done;
+        bool done;
 
         size_t transferred_size;
@@ -592 +619 @@
         d->error = error;
         fibril_mutex_lock(&d->done_mtx);
-        d->done = 1;
+        d->done = true;
         fibril_condvar_broadcast(&d->done_cv);
         fibril_mutex_unlock(&d->done_mtx);
@@ -599 +626 @@
 
 /**
- * Issue a transfer on the bus, wait for result.
+ * Issue a transfer on the bus, wait for the result.
  *
  * @param device Device for which to send the batch
@@ -613 +640 @@
     const char *name)
 {
-        sync_data_t sd = { .done = 0 };
+        sync_data_t sd = { .done = false };
         fibril_mutex_initialize(&sd.done_mtx);
         fibril_condvar_initialize(&sd.done_cv);
 
         const int ret = bus_device_send_batch(device, target, direction,
-            data, size, setup_data,
-            sync_transfer_complete, &sd, name);
+            data, size, setup_data, sync_transfer_complete, &sd, name);
         if (ret != EOK)
                 return ret;
 
+        /*
+         * Note: There are requests that are completed synchronously. It is not
+         *       therefore possible to just lock the mutex before and wait.
+         */
         fibril_mutex_lock(&sd.done_mtx);
-        while (!sd.done) {
+        while (!sd.done)
                 fibril_condvar_wait(&sd.done_cv, &sd.done_mtx);
-        }
         fibril_mutex_unlock(&sd.done_mtx);