core: Add validation to timeval parameters
authorChris Dickens <christopher.a.dickens@gmail.com>
Thu, 27 Aug 2020 18:05:11 +0000 (11:05 -0700)
committerChris Dickens <christopher.a.dickens@gmail.com>
Sun, 13 Sep 2020 07:09:19 +0000 (00:09 -0700)
Prior to this change, the timeval structures provided by users did not
go through any type of validation, therefore an invalid timeval would
result in potentially unclear or confusing errors when used later on.
Add checks to the core API functions that accept timevals and return
LIBUSB_ERROR_INVALID_PARAM if the timeval is not valid.

While at it, add some macro definitions to avoid magic numbers.

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
libusb/io.c
libusb/libusbi.h
libusb/os/threads_posix.c
libusb/version_nano.h

index b625620..d565f89 100644 (file)
@@ -1261,9 +1261,9 @@ static int calculate_timeout(struct usbi_transfer *itransfer)
 
        itransfer->timeout.tv_sec += timeout / 1000U;
        itransfer->timeout.tv_nsec += (timeout % 1000U) * 1000000L;
-       if (itransfer->timeout.tv_nsec >= 1000000000L) {
+       if (itransfer->timeout.tv_nsec >= NSEC_PER_SEC) {
                ++itransfer->timeout.tv_sec;
-               itransfer->timeout.tv_nsec -= 1000000000L;
+               itransfer->timeout.tv_nsec -= NSEC_PER_SEC;
        }
 
        return 0;
@@ -2002,6 +2002,7 @@ void API_EXPORTED libusb_unlock_event_waiters(libusb_context *ctx)
  * indicates unlimited timeout.
  * \returns 0 after a transfer completes or another thread stops event handling
  * \returns 1 if the timeout expired
+ * \returns LIBUSB_ERROR_INVALID_PARAM if timeval is invalid
  * \ref libusb_mtasync
  */
 int API_EXPORTED libusb_wait_for_event(libusb_context *ctx, struct timeval *tv)
@@ -2009,11 +2010,14 @@ int API_EXPORTED libusb_wait_for_event(libusb_context *ctx, struct timeval *tv)
        int r;
 
        ctx = usbi_get_context(ctx);
-       if (tv == NULL) {
+       if (!tv) {
                usbi_cond_wait(&ctx->event_waiters_cond, &ctx->event_waiters_lock);
                return 0;
        }
 
+       if (!TIMEVAL_IS_VALID(tv))
+               return LIBUSB_ERROR_INVALID_PARAM;
+
        r = usbi_cond_timedwait(&ctx->event_waiters_cond,
                &ctx->event_waiters_lock, tv);
        if (r < 0)
@@ -2328,7 +2332,9 @@ static int get_next_timeout(libusb_context *ctx, struct timeval *tv,
  * \param tv the maximum time to block waiting for events, or an all zero
  * timeval struct for non-blocking mode
  * \param completed pointer to completion integer to check, or NULL
- * \returns 0 on success, or a LIBUSB_ERROR code on failure
+ * \returns 0 on success
+ * \returns LIBUSB_ERROR_INVALID_PARAM if timeval is invalid
+ * \returns another LIBUSB_ERROR code on other failure
  * \ref libusb_mtasync
  */
 int API_EXPORTED libusb_handle_events_timeout_completed(libusb_context *ctx,
@@ -2337,6 +2343,9 @@ int API_EXPORTED libusb_handle_events_timeout_completed(libusb_context *ctx,
        int r;
        struct timeval poll_timeout;
 
+       if (!TIMEVAL_IS_VALID(tv))
+               return LIBUSB_ERROR_INVALID_PARAM;
+
        ctx = usbi_get_context(ctx);
        r = get_next_timeout(ctx, tv, &poll_timeout);
        if (r) {
@@ -2465,7 +2474,9 @@ int API_EXPORTED libusb_handle_events_completed(libusb_context *ctx,
  * \param ctx the context to operate on, or NULL for the default context
  * \param tv the maximum time to block waiting for events, or zero for
  * non-blocking mode
- * \returns 0 on success, or a LIBUSB_ERROR code on failure
+ * \returns 0 on success
+ * \returns LIBUSB_ERROR_INVALID_PARAM if timeval is invalid
+ * \returns another LIBUSB_ERROR code on other failure
  * \ref libusb_mtasync
  */
 int API_EXPORTED libusb_handle_events_locked(libusb_context *ctx,
@@ -2474,6 +2485,9 @@ int API_EXPORTED libusb_handle_events_locked(libusb_context *ctx,
        int r;
        struct timeval poll_timeout;
 
+       if (!TIMEVAL_IS_VALID(tv))
+               return LIBUSB_ERROR_INVALID_PARAM;
+
        ctx = usbi_get_context(ctx);
        r = get_next_timeout(ctx, tv, &poll_timeout);
        if (r) {
index 94d10d7..25b7e9a 100644 (file)
@@ -228,6 +228,13 @@ static inline void *usbi_reallocf(void *ptr, size_t size)
        return ret;
 }
 
+#define USEC_PER_SEC   1000000L
+#define NSEC_PER_SEC   1000000000L
+
+#define TIMEVAL_IS_VALID(tv)                                           \
+       ((tv)->tv_sec >= 0 &&                                           \
+        (tv)->tv_usec >= 0 && (tv)->tv_usec < USEC_PER_SEC)
+
 #define TIMESPEC_IS_SET(ts)    ((ts)->tv_sec || (ts)->tv_nsec)
 #define TIMESPEC_CLEAR(ts)     (ts)->tv_sec = (ts)->tv_nsec = 0
 #define TIMESPEC_CMP(a, b, CMP)                                                \
@@ -240,7 +247,7 @@ static inline void *usbi_reallocf(void *ptr, size_t size)
                (result)->tv_nsec = (a)->tv_nsec - (b)->tv_nsec;        \
                if ((result)->tv_nsec < 0L) {                           \
                        --(result)->tv_sec;                             \
-                       (result)->tv_nsec += 1000000000L;               \
+                       (result)->tv_nsec += NSEC_PER_SEC;              \
                }                                                       \
        } while (0)
 
index d79e8a6..b4199e6 100644 (file)
@@ -50,9 +50,9 @@ int usbi_cond_timedwait(pthread_cond_t *cond,
                return r;
 
        timeout.tv_sec += tv->tv_sec;
-       timeout.tv_nsec += tv->tv_usec * 1000;
-       while (timeout.tv_nsec >= 1000000000L) {
-               timeout.tv_nsec -= 1000000000L;
+       timeout.tv_nsec += tv->tv_usec * 1000L;
+       if (timeout.tv_nsec >= NSEC_PER_SEC) {
+               timeout.tv_nsec -= NSEC_PER_SEC;
                timeout.tv_sec++;
        }
 
index 3c7f971..83c3072 100644 (file)
@@ -1 +1 @@
-#define LIBUSB_NANO 11556
+#define LIBUSB_NANO 11557