Changeset a6afb4c in mainline


Ignore:
Timestamp:
2018-01-23T14:02:35Z (6 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

Location:
uspace/lib
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • uspace/lib/drv/include/usbhc_iface.h

    re7e1fd3 ra6afb4c  
    6060 * Negative values could be used to indicate error.
    6161 */
    62 typedef int16_t usb_endpoint_t;
     62typedef uint16_t usb_endpoint_t;
    6363
    6464/** USB address type.
    6565 * Negative values could be used to indicate error.
    6666 */
    67 typedef int16_t usb_address_t;
     67typedef uint16_t usb_address_t;
     68
     69/**
     70 * USB Stream ID type.
     71 */
     72typedef uint16_t usb_stream_t;
    6873
    6974/** USB transfer type. */
    … …  
    7277        USB_TRANSFER_ISOCHRONOUS = 1,
    7378        USB_TRANSFER_BULK = 2,
    74         USB_TRANSFER_INTERRUPT = 3
     79        USB_TRANSFER_INTERRUPT = 3,
    7580} usb_transfer_type_t;
     81
     82#define USB_TRANSFER_COUNT  (USB_TRANSFER_INTERRUPT + 1)
    7683
    7784/** USB data transfer direction. */
    … …  
    7986        USB_DIRECTION_IN,
    8087        USB_DIRECTION_OUT,
    81         USB_DIRECTION_BOTH
     88        USB_DIRECTION_BOTH,
    8289} usb_direction_t;
     90
     91#define USB_DIRECTION_COUNT  (USB_DIRECTION_BOTH + 1)
    8392
    8493/** USB complete address type.
    … …  
    8998                usb_address_t address;
    9099                usb_endpoint_t endpoint;
    91                 uint32_t stream;
     100                usb_stream_t stream;
    92101        } __attribute__((packed));
    93102        uint64_t packed;

  • uspace/lib/usb/include/usb/usb.h

    re7e1fd3 ra6afb4c  
    6363}
    6464
     65static inline bool usb_speed_is_valid(const usb_speed_t s)
     66{
     67        return (s >= USB_SPEED_LOW) && (s < USB_SPEED_MAX);
     68}
     69
    6570const char *usb_str_speed(usb_speed_t);
    6671
    … …  
    97102static inline bool usb_address_is_valid(usb_address_t a)
    98103{
    99         return (a >= USB_ADDRESS_DEFAULT) && (a <= USB11_ADDRESS_MAX);
     104        return a <= USB11_ADDRESS_MAX;
    100105}
    101106
    … …  
    105110/** Maximum endpoint number in USB */
    106111#define USB_ENDPOINT_MAX 16
     112
     113/** There might be two directions for every endpoint number (except 0) */
     114#define USB_ENDPOINT_COUNT (2 * USB_ENDPOINT_MAX)
    107115
    108116/** Check USB endpoint for allowed values.
    … …  
    115123static inline bool usb_endpoint_is_valid(usb_endpoint_t ep)
    116124{
    117         return (ep >= USB_ENDPOINT_DEFAULT_CONTROL) &&
    118             (ep < USB_ENDPOINT_MAX);
     125        return ep < USB_ENDPOINT_MAX;
    119126}
    120127
    121 /** Check USB target for allowed values (address and endpoint).
     128/**
     129 * Check USB target for allowed values (address, endpoint, stream).
    122130 *
    123131 * @param target.
    124132 * @return True, if values are wihtin limits, false otherwise.
    125133 */
    126 static inline bool usb_target_is_valid(usb_target_t target)
     134static inline bool usb_target_is_valid(usb_target_t *target)
    127135{
    128         return usb_address_is_valid(target.address) &&
    129             usb_endpoint_is_valid(target.endpoint);
     136        return usb_address_is_valid(target->address) &&
     137            usb_endpoint_is_valid(target->endpoint);
     138
     139        // A 16-bit Stream ID is always valid.
    130140}
    131141
    … …  
    136146 * @return Whether @p a and @p b points to the same pipe on the same device.
    137147 */
    138 static inline int usb_target_same(usb_target_t a, usb_target_t b)
     148static inline bool usb_target_same(usb_target_t a, usb_target_t b)
    139149{
    140150        return (a.address == b.address)
    … …  
    142152}
    143153
    144 /** General handle type.
    145  * Used by various USB functions as opaque handle.
    146  */
    147 typedef sysarg_t usb_handle_t;
     154static inline bool usb_transfer_type_is_valid(usb_transfer_type_t type)
     155{
     156        return (type >= 0) && (type < USB_TRANSFER_COUNT);
     157}
     158
     159static inline bool usb_direction_is_valid(usb_direction_t dir)
     160{
     161        return (dir >= 0) && (dir < USB_DIRECTION_COUNT);
     162}
    148163
    149164/** USB packet identifier. */

  • uspace/lib/usbhost/include/usb/host/bus.h

    re7e1fd3 ra6afb4c  
    8484        usb_speed_t speed;
    8585        usb_address_t address;
    86         endpoint_t *endpoints [2 * USB_ENDPOINT_MAX];
     86        endpoint_t *endpoints [USB_ENDPOINT_COUNT];
    8787
    8888        /* Managing bus */

  • uspace/lib/usbhost/src/bus.c

    re7e1fd3 ra6afb4c  
    4242#include <errno.h>
    4343#include <mem.h>
     44#include <macros.h>
    4445#include <stdio.h>
    4546#include <str_error.h>
    … …  
    345346 * For different arguments, the result is stable but not defined.
    346347 */
    347 static int bus_endpoint_index(usb_endpoint_t ep, usb_direction_t dir)
     348static size_t bus_endpoint_index(usb_endpoint_t ep, usb_direction_t dir)
    348349{
    349350        return 2 * ep + (dir == USB_DIRECTION_OUT);
    … …  
    384385        endpoint_add_ref(ep);
    385386
     387        const size_t idx = bus_endpoint_index(ep->endpoint, ep->direction);
     388        if (idx >= ARRAY_SIZE(device->endpoints)) {
     389                usb_log_warning("Invalid endpoint description (ep no %u out of "
     390                    "bounds)", ep->endpoint);
     391                goto drop;
     392        }
     393
    386394        if (ep->max_transfer_size == 0) {
    387395                usb_log_warning("Invalid endpoint description (mps %zu, "
    388396                        "%u packets)", ep->max_packet_size, ep->packets_per_uframe);
    389                 /* Bus reference */
    390                 endpoint_del_ref(ep);
    391                 return EINVAL;
     397                goto drop;
    392398        }
    393399
    … …  
    397403            usb_str_direction(ep->direction),
    398404            ep->max_transfer_size);
    399 
    400         const int idx = bus_endpoint_index(ep->endpoint, ep->direction);
    401405
    402406        fibril_mutex_lock(&device->guard);
    … …  
    423427
    424428        return EOK;
     429drop:
     430        /* Bus reference */
     431        endpoint_del_ref(ep);
     432        return EINVAL;
    425433}
    426434
    … …  
    428436 * Search for an endpoint. Returns a reference.
    429437 */
    430 endpoint_t *bus_find_endpoint(device_t *device, usb_endpoint_t endpoint, usb_direction_t dir)
     438endpoint_t *bus_find_endpoint(device_t *device, usb_endpoint_t endpoint,
     439    usb_direction_t dir)
    431440{
    432441        assert(device);
    433442
    434         const int idx = bus_endpoint_index(endpoint, dir);
    435         const int ctrl_idx = bus_endpoint_index(endpoint, USB_DIRECTION_BOTH);
     443        const size_t idx = bus_endpoint_index(endpoint, dir);
     444        const size_t ctrl_idx = bus_endpoint_index(endpoint, USB_DIRECTION_BOTH);
     445
     446        endpoint_t *ep = NULL;
    436447
    437448        fibril_mutex_lock(&device->guard);
    438         endpoint_t *ep = device->endpoints[idx];
     449        if (idx < ARRAY_SIZE(device->endpoints))
     450                ep = device->endpoints[idx];
    439451        /*
    440452         * If the endpoint was not found, it's still possible it is a control
    441453         * endpoint having direction BOTH.
    442454         */
    443         if (!ep) {
     455        if (!ep && ctrl_idx < ARRAY_SIZE(device->endpoints)) {
    444456                ep = device->endpoints[ctrl_idx];
    445457                if (ep && ep->transfer_type != USB_TRANSFER_CONTROL)
    … …  
    478490            ep->max_transfer_size);
    479491
    480         const int idx = bus_endpoint_index(ep->endpoint, ep->direction);
     492        const size_t idx = bus_endpoint_index(ep->endpoint, ep->direction);
     493
     494        if (idx >= ARRAY_SIZE(device->endpoints))
     495                return EINVAL;
    481496
    482497        fibril_mutex_lock(&device->guard);
    … …  
    495510
    496511/**
    497  * Reserve the default address on the bus. Also, report the speed of the device
    498  * that is listening on the default address.
    499  *
    500  * The speed is then used for devices enumerated while the address is reserved.
     512 * Reserve the default address on the bus for the specified device (hub).
    501513 */
    502514int bus_reserve_default_address(bus_t *bus, device_t *dev)
    … …  
    564576        assert(ep->device == device);
    565577
    566         const int err = endpoint_send_batch(ep, target, direction, data, size, setup_data,
    567             on_complete, arg, name);
     578        /*
     579         * This method is already callable from HC only, so we can force these
     580         * conditions harder.
     581         * Invalid values from devices shall be caught on DDF interface already.
     582         */
     583        assert(usb_target_is_valid(&target));
     584        assert(usb_direction_is_valid(direction));
     585        assert(direction != USB_DIRECTION_BOTH);
     586        assert(size == 0 || data != NULL);
     587        assert(arg == NULL || on_complete != NULL);
     588        assert(name);
     589
     590        const int err = endpoint_send_batch(ep, target, direction,
     591            data, size, setup_data, on_complete, arg, name);
    568592
    569593        /* Temporary reference */
    … …  
    573597}
    574598
     599/**
     600 * A structure to pass data from the completion callback to the caller.
     601 */
    575602typedef struct {
    576603        fibril_mutex_t done_mtx;
    577604        fibril_condvar_t done_cv;
    578         unsigned done;
     605        bool done;
    579606
    580607        size_t transferred_size;
    … …  
    592619        d->error = error;
    593620        fibril_mutex_lock(&d->done_mtx);
    594         d->done = 1;
     621        d->done = true;
    595622        fibril_condvar_broadcast(&d->done_cv);
    596623        fibril_mutex_unlock(&d->done_mtx);
    … …  
    599626
    600627/**
    601  * Issue a transfer on the bus, wait for result.
     628 * Issue a transfer on the bus, wait for the result.
    602629 *
    603630 * @param device Device for which to send the batch
    … …  
    613640    const char *name)
    614641{
    615         sync_data_t sd = { .done = 0 };
     642        sync_data_t sd = { .done = false };
    616643        fibril_mutex_initialize(&sd.done_mtx);
    617644        fibril_condvar_initialize(&sd.done_cv);
    618645
    619646        const int ret = bus_device_send_batch(device, target, direction,
    620             data, size, setup_data,
    621             sync_transfer_complete, &sd, name);
     647            data, size, setup_data, sync_transfer_complete, &sd, name);
    622648        if (ret != EOK)
    623649                return ret;
    624650
     651        /*
     652         * Note: There are requests that are completed synchronously. It is not
     653         *       therefore possible to just lock the mutex before and wait.
     654         */
    625655        fibril_mutex_lock(&sd.done_mtx);
    626         while (!sd.done) {
     656        while (!sd.done)
    627657                fibril_condvar_wait(&sd.done_cv, &sd.done_mtx);
    628         }
    629658        fibril_mutex_unlock(&sd.done_mtx);
    630659

  • uspace/lib/usbhost/src/ddf_helpers.c

    re7e1fd3 ra6afb4c  
    154154        int err;
    155155
     156        if (!usb_speed_is_valid(speed))
     157                return EINVAL;
     158
    156159        usb_log_debug("Hub %d reported a new %s speed device on port: %u",
    157160            hub->address, usb_str_speed(speed), port);
    … …  
    212215
    213216        if (!victim) {
    214                 usb_log_warning("Hub '%s' tried to remove non-existant"
     217                usb_log_warning("Hub '%s' tried to remove non-existent"
    215218                    " device.", ddf_fun_get_name(fun));
    216219                return ENOENT;
    … …  
    271274        target.address = dev->address;
    272275
     276        if (!usb_target_is_valid(&target))
     277                return EINVAL;
     278
     279        if (size > 0 && data == NULL)
     280                return EBADMEM;
     281
     282        if (!callback && arg)
     283                return EBADMEM;
     284
    273285        return bus_device_send_batch(dev, target, USB_DIRECTION_IN,
    274286            data, size, setup_data,
    … …  
    295307
    296308        target.address = dev->address;
     309
     310        if (!usb_target_is_valid(&target))
     311                return EINVAL;
     312
     313        if (size > 0 && data == NULL)
     314                return EBADMEM;
     315
     316        if (!callback && arg)
     317                return EBADMEM;
    297318
    298319        return bus_device_send_batch(dev, target, USB_DIRECTION_OUT,
Note: See TracChangeset for help on using the changeset viewer.