core: Improve robustness of event handling

Prior to this commit, it was possible on Linux to poll a file descriptor
that was not valid for libusb. This could happen given the following
sequence of events:

  1) Application registers hotplug callback
  2) Application calls into libusb to handle events
  3) USB device is unplugged, triggering hotplug event
  4) Event handling thread calls registered hotplug callback
  5) Hotplug callback calls libusb_close(), which closes the fd
  6) Event handling code retries the poll() call before returning

At this point, the file descriptor that is being passed to poll() may be
invalid or worse may not belong to libusb at all.

This commit fixes this by removing the code that restarted the poll() call.
It was originally added to allow the processing of multiple hotplug messages
when these messages were delivered across a pipe, but with a linked list now
holding the hotplug messages this is no longer a concern. If any hotplug
messages are present on the list, they are all processed in one pass.

Closes #238

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

diff --git a/libusb/io.c b/libusb/io.c
index 0bfb73d..6eaf72c 100644 (file)
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -2077,7 +2077,6 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
        struct pollfd *fds = NULL;
        int i = -1;
        int timeout_ms;
-       int special_event;
 
        /* prevent attempts to recursively handle events (e.g. calling into
         * libusb_handle_events() from within a hotplug or transfer callback) */
@@ -2146,32 +2145,29 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
        if (tv->tv_usec % 1000)
                timeout_ms++;
 
-redo_poll:
        usbi_dbg("poll() %d fds with timeout in %dms", nfds, timeout_ms);
        r = usbi_poll(fds, nfds, timeout_ms);
        usbi_dbg("poll() returned %d", r);
        if (r == 0) {
                r = handle_timeouts(ctx);
                goto done;
-       }
-       else if (r == -1 && errno == EINTR) {
+       } else if (r == -1 && errno == EINTR) {
                r = LIBUSB_ERROR_INTERRUPTED;
                goto done;
-       }
-       else if (r < 0) {
+       } else if (r < 0) {
                usbi_err(ctx, "poll failed %d err=%d", r, errno);
                r = LIBUSB_ERROR_IO;
                goto done;
        }
 
-       special_event = 0;
-
        /* fds[0] is always the event pipe */
        if (fds[0].revents) {
-               libusb_hotplug_message *message = NULL;
+               struct list_head hotplug_msgs;
                struct usbi_transfer *itransfer;
                int ret = 0;
 
+               list_init(&hotplug_msgs);
+
                usbi_dbg("caught a fish on the event pipe");
 
                /* take the the event data lock while processing events */
@@ -2193,9 +2189,7 @@ redo_poll:
                /* check for any pending hotplug messages */
                if (!list_empty(&ctx->hotplug_msgs)) {
                        usbi_dbg("hotplug message received");
-                       special_event = 1;
-                       message = list_first_entry(&ctx->hotplug_msgs, libusb_hotplug_message, list);
-                       list_del(&message->list);
+                       list_cut(&hotplug_msgs, &ctx->hotplug_msgs);
                }
 
                /* complete any pending transfers */
@@ -2215,14 +2209,18 @@ redo_poll:
 
                usbi_mutex_unlock(&ctx->event_data_lock);
 
-               /* process the hotplug message, if any */
-               if (message) {
+               /* process the hotplug messages, if any */
+               while (!list_empty(&hotplug_msgs)) {
+                       libusb_hotplug_message *message =
+                               list_first_entry(&hotplug_msgs, libusb_hotplug_message, list);
+
                        usbi_hotplug_match(ctx, message->device, message->event);
 
                        /* the device left, dereference the device */
                        if (LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT == message->event)
                                libusb_unref_device(message->device);
 
+                       list_del(&message->list);
                        free(message);
                }
 
@@ -2233,7 +2231,7 @@ redo_poll:
                }
 
                if (0 == --r)
-                       goto handled;
+                       goto done;
        }
 
 #ifdef USBI_TIMERFD_AVAILABLE
@@ -2242,7 +2240,6 @@ redo_poll:
                /* timerfd indicates that a timeout has expired */
                int ret;
                usbi_dbg("timerfd triggered");
-               special_event = 1;
 
                ret = handle_timerfd_trigger(ctx);
                if (ret < 0) {
@@ -2252,7 +2249,7 @@ redo_poll:
                }
 
                if (0 == --r)
-                       goto handled;
+                       goto done;
        }
 #endif
 
@@ -2260,12 +2257,6 @@ redo_poll:
        if (r)
                usbi_err(ctx, "backend handle_events failed with error %d", r);
 
-handled:
-       if (r == 0 && special_event) {
-               timeout_ms = 0;
-               goto redo_poll;
-       }
-
 done:
        usbi_end_event_handling(ctx);
        return r;

diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 752e398..026ed0f 100644 (file)
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -139,6 +139,19 @@ static inline void list_del(struct list_head *entry)
        entry->next = entry->prev = NULL;
 }
 
+static inline void list_cut(struct list_head *list, struct list_head *head)
+{
+       if (list_empty(head))
+               return;
+
+       list->next = head->next;
+       list->next->prev = list;
+       list->prev = head->prev;
+       list->prev->next = list;
+
+       list_init(head);
+}
+
 static inline void *usbi_reallocf(void *ptr, size_t size)
 {
        void *ret = realloc(ptr, size);

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 900c220..e9b13cf 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11190
+#define LIBUSB_NANO 11191