core: protect against changes to the pollfd list during handle_events

It is possible a file descriptor to be removed due to a close and the
same file descriptor value to be added during the same event loop. This
can cause the backend to get a stale revent and incorrectly remove the
file descriptor.

This commit addresses the issue by delaying the actual removal of the
ipollfds entry until just before the backend handle_events is called.
handle_events then goes through the list of closed fds and clears out
the revents associated with the closed file descriptor.

Signed-off-by: Nathan Hjelm <hjelmn@google.com>

3 files changed, 37 insertions, 2 deletions

diff --git a/libusb/io.c b/libusb/io.c
index c3185bb..978b09a 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -3,6 +3,8 @@
  * I/O functions for libusb
  * Copyright © 2007-2009 Daniel Drake <dsd@gentoo.org>
  * Copyright © 2001 Johannes Erdfelt <johannes@erdfelt.com>
+ * Copyright © 2019 Nathan Hjelm <hjelmn@cs.umm.edu>
+ * Copyright © 2019 Google LLC. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -1130,6 +1132,7 @@ int usbi_io_init(struct libusb_context *ctx)
 	usbi_tls_key_create(&ctx->event_handling_key);
 	list_init(&ctx->flying_transfers);
 	list_init(&ctx->ipollfds);
+	list_init(&ctx->removed_ipollfds);
 	list_init(&ctx->hotplug_msgs);
 	list_init(&ctx->completed_transfers);
 
@@ -1178,6 +1181,15 @@ err:
 	return r;
 }
 
+static void cleanup_removed_pollfds(struct libusb_context *ctx)
+{
+	struct usbi_pollfd *ipollfd, *tmp;
+	list_for_each_entry_safe(ipollfd, tmp, &ctx->removed_ipollfds, list, struct usbi_pollfd) {
+		list_del(&ipollfd->list);
+		free(ipollfd);
+	}
+}
+
 void usbi_io_exit(struct libusb_context *ctx)
 {
 	usbi_remove_pollfd(ctx, ctx->event_pipe[0]);
@@ -1196,6 +1208,7 @@ void usbi_io_exit(struct libusb_context *ctx)
 	usbi_mutex_destroy(&ctx->event_data_lock);
 	usbi_tls_key_delete(ctx->event_handling_key);
 	free(ctx->pollfds);
+	cleanup_removed_pollfds(ctx);
 }
 
 static int calculate_timeout(struct usbi_transfer *transfer)
@@ -2120,6 +2133,8 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
 	/* only reallocate the poll fds when the list of poll fds has been modified
 	 * since the last poll, otherwise reuse them to save the additional overhead */
 	usbi_mutex_lock(&ctx->event_data_lock);
+	/* clean up removed poll fds */
+	cleanup_removed_pollfds(ctx);
 	if (ctx->event_flags & USBI_EVENT_POLLFDS_MODIFIED) {
 		usbi_dbg("poll fds modified, reallocating");
 
@@ -2281,6 +2296,20 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
 	}
 #endif
 
+	list_for_each_entry(ipollfd, &ctx->removed_ipollfds, list, struct usbi_pollfd) {
+		for (i = internal_nfds ; i < nfds ; ++i) {
+			if (ipollfd->pollfd.fd == fds[i].fd) {
+				/* pollfd was removed between the creation of the fd
+				 * array and here. remove any triggered revent as
+				 * it is no longer relevant */
+				usbi_dbg("pollfd %d was removed. ignoring raised events",
+					 fds[i].fd);
+				fds[i].revents = 0;
+				break;
+			}
+		}
+	}
+
 	r = usbi_backend.handle_events(ctx, fds + internal_nfds, nfds - internal_nfds, r);
 	if (r)
 		usbi_err(ctx, "backend handle_events failed with error %d", r);
@@ -2712,10 +2741,11 @@ void usbi_remove_pollfd(struct libusb_context *ctx, int fd)
 	}
 
 	list_del(&ipollfd->list);
+	list_add_tail(&ipollfd->list, &ctx->removed_ipollfds);
 	ctx->pollfds_cnt--;
 	usbi_fd_notification(ctx);
 	usbi_mutex_unlock(&ctx->event_data_lock);
-	free(ipollfd);
+
 	if (ctx->fd_removed_cb)
 		ctx->fd_removed_cb(fd, ctx->fd_cb_user_data);
 }
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index f0512b5..0a677bd 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -2,6 +2,8 @@
  * Internal header for libusb
  * Copyright © 2007-2009 Daniel Drake <dsd@gentoo.org>
  * Copyright © 2001 Johannes Erdfelt <johannes@erdfelt.com>
+ * Copyright © 2019 Nathan Hjelm <hjelmn@cs.umm.edu>
+ * Copyright © 2019 Google LLC. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -365,6 +367,9 @@ struct libusb_context {
 	/* list and count of poll fds and an array of poll fd structures that is
 	 * (re)allocated as necessary prior to polling. Protected by event_data_lock. */
 	struct list_head ipollfds;
+        /* list of pollfds that have been removed. keeps track of pollfd changes
+         * between the poll call and */
+        struct list_head removed_ipollfds;
 	struct pollfd *pollfds;
 	POLL_NFDS_TYPE pollfds_cnt;
 
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 1ff7ca7..a71854b 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11390
+#define LIBUSB_NANO 11391