core: Prevent attempts to recursively hande events

Prior to this commit, it was possible to call certain functions from
within a hotplug or transfer completion callback that would in turn
attempt to handle events (e.g. any of the sync transfer APIs). This
is dangerous because certain events may cause the nested calls to
free memory that is currently in use by the previous calls.

This implementation uses thread-local storage to store a key within
the context that is set to a non-NULL value whenever event handling
is occurring. This allows us to detect when dangerous calls are made
from within event handling context. Such calls will return an error
code of LIBUSB_ERROR_BUSY.

Note that this implementation was chosen because of its portability.
It is supported on all platforms that libusb supports.

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

8 files changed, 101 insertions, 13 deletions

diff --git a/libusb/hotplug.c b/libusb/hotplug.c
index 8dc185f..779cb6b 100644
--- a/libusb/hotplug.c
+++ b/libusb/hotplug.c
@@ -80,8 +80,14 @@
  * are invalid and will remain so even if the device comes back.
  *
  * When handling a LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED event it is considered
- * safe to call any libusb function that takes a libusb_device. On the other hand,
- * when handling a LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT event the only safe function
+ * safe to call any libusb function that takes a libusb_device. It also safe to
+ * open a device and submit asynchronous transfers. However, most other functions
+ * that take a libusb_device_handle are <b>not</b> safe to call. Examples of such
+ * functions are any of the \ref syncio "synchronous API" functions or the blocking
+ * functions that retrieve various \ref desc "USB descriptors". These functions must
+ * be used outside of the context of the hotplug callback.
+ *
+ * When handling a LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT event the only safe function
  * is libusb_get_device_descriptor().
  *
  * The following code provides an example of the usage of the hotplug interface:
diff --git a/libusb/io.c b/libusb/io.c
index 5daf212..01cf5be 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -293,6 +293,12 @@ if (r == 0 && actual_length == sizeof(data)) {
  * success or failure reason, number of bytes of data transferred, etc. See
  * the libusb_transfer structure documentation for more information.
  *
+ * <b>Important Note</b>: The user-specified callback is called from an event
+ * handling context. It is therefore important that no calls are made into
+ * libusb that will attempt to perform any event handling. Examples of such
+ * functions are any listed in the \ref syncio "synchronous API" and any of
+ * the blocking functions that retrieve \ref desc "USB descriptors".
+ *
  * \subsection Deallocation
  *
  * When a transfer has completed (i.e. the callback function has been invoked),
@@ -1123,6 +1129,7 @@ int usbi_io_init(struct libusb_context *ctx)
 	usbi_mutex_init(&ctx->event_waiters_lock, NULL);
 	usbi_cond_init(&ctx->event_waiters_cond, NULL);
 	usbi_mutex_init(&ctx->event_data_lock, NULL);
+	usbi_tls_key_create(&ctx->event_handling_key, NULL);
 	list_init(&ctx->flying_transfers);
 	list_init(&ctx->ipollfds);
 	list_init(&ctx->hotplug_msgs);
@@ -1169,6 +1176,7 @@ err:
 	usbi_mutex_destroy(&ctx->event_waiters_lock);
 	usbi_cond_destroy(&ctx->event_waiters_cond);
 	usbi_mutex_destroy(&ctx->event_data_lock);
+	usbi_tls_key_delete(ctx->event_handling_key);
 	return r;
 }
 
@@ -1188,6 +1196,7 @@ void usbi_io_exit(struct libusb_context *ctx)
 	usbi_mutex_destroy(&ctx->event_waiters_lock);
 	usbi_cond_destroy(&ctx->event_waiters_cond);
 	usbi_mutex_destroy(&ctx->event_data_lock);
+	usbi_tls_key_delete(ctx->event_handling_key);
 	if (ctx->pollfds)
 		free(ctx->pollfds);
 }
@@ -2038,6 +2047,12 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
 	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) */
+	if (usbi_handling_events(ctx))
+		return LIBUSB_ERROR_BUSY;
+	usbi_start_event_handling(ctx);
+
 	/* there are certain fds that libusb uses internally, currently:
 	 *
 	 *   1) event pipe
@@ -2070,7 +2085,8 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
 		ctx->pollfds = calloc(ctx->pollfds_cnt, sizeof(*ctx->pollfds));
 		if (!ctx->pollfds) {
 			usbi_mutex_unlock(&ctx->event_data_lock);
-			return LIBUSB_ERROR_NO_MEM;
+			r = LIBUSB_ERROR_NO_MEM;
+			goto done;
 		}
 
 		list_for_each_entry(ipollfd, &ctx->ipollfds, list, struct usbi_pollfd) {
@@ -2102,13 +2118,18 @@ 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)
-		return handle_timeouts(ctx);
-	else if (r == -1 && errno == EINTR)
-		return LIBUSB_ERROR_INTERRUPTED;
+	if (r == 0) {
+		r = handle_timeouts(ctx);
+		goto done;
+	}
+	else if (r == -1 && errno == EINTR) {
+		r = LIBUSB_ERROR_INTERRUPTED;
+		goto done;
+	}
 	else if (r < 0) {
 		usbi_err(ctx, "poll failed %d err=%d", r, errno);
-		return LIBUSB_ERROR_IO;
+		r = LIBUSB_ERROR_IO;
+		goto done;
 	}
 
 	special_event = 0;
@@ -2171,7 +2192,7 @@ redo_poll:
 		if (ret) {
 			/* return error code */
 			r = ret;
-			goto handled;
+			goto done;
 		}
 
 		if (0 == --r)
@@ -2190,7 +2211,7 @@ redo_poll:
 		if (ret < 0) {
 			/* return error code */
 			r = ret;
-			goto handled;
+			goto done;
 		}
 
 		if (0 == --r)
@@ -2208,6 +2229,8 @@ handled:
 		goto redo_poll;
 	}
 
+done:
+	usbi_end_event_handling(ctx);
 	return r;
 }
 
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 822612e..f1afd99 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -279,6 +279,10 @@ struct libusb_context {
 	/* used to see if there is an active thread doing event handling */
 	int event_handler_active;
 
+	/* A thread-local storage key to track which thread is performing event
+	 * handling */
+	usbi_tls_key_t event_handling_key;
+
 	/* used to wait for event completion in threads other than the one that is
 	 * event handling */
 	usbi_mutex_t event_waiters_lock;
@@ -315,6 +319,16 @@ struct libusb_context {
 	struct list_head list;
 };
 
+/* Macros for managing event handling state */
+#define usbi_handling_events(ctx) \
+	(usbi_tls_key_get((ctx)->event_handling_key) != NULL)
+
+#define usbi_start_event_handling(ctx) \
+	usbi_tls_key_set((ctx)->event_handling_key, ctx)
+
+#define usbi_end_event_handling(ctx) \
+	usbi_tls_key_set((ctx)->event_handling_key, NULL)
+
 /* Update the following macro if new event sources are added */
 #define usbi_pending_events(ctx) \
 	((ctx)->device_close || (ctx)->pollfds_modified \
diff --git a/libusb/os/threads_posix.h b/libusb/os/threads_posix.h
index d7a5d21..3caeca4 100644
--- a/libusb/os/threads_posix.h
+++ b/libusb/os/threads_posix.h
@@ -43,6 +43,12 @@
 #define usbi_cond_destroy		pthread_cond_destroy
 #define usbi_cond_signal		pthread_cond_signal
 
+#define usbi_tls_key_t			pthread_key_t
+#define usbi_tls_key_create		pthread_key_create
+#define usbi_tls_key_get		pthread_getspecific
+#define usbi_tls_key_set		pthread_setspecific
+#define usbi_tls_key_delete		pthread_key_delete
+
 extern int usbi_mutex_init_recursive(pthread_mutex_t *mutex, pthread_mutexattr_t *attr);
 
 int usbi_get_tid(void);
diff --git a/libusb/os/threads_windows.c b/libusb/os/threads_windows.c
index 700bad6..e3fd910 100644
--- a/libusb/os/threads_windows.c
+++ b/libusb/os/threads_windows.c
@@ -209,6 +209,28 @@ int usbi_cond_timedwait(usbi_cond_t *cond,
 	return usbi_cond_intwait(cond, mutex, millis);
 }
 
+int usbi_tls_key_create(usbi_tls_key_t *key, void (*destructor)(void *)) {
+	UNUSED(destructor);
+	if(!key) return ((errno=EINVAL));
+	*key = TlsAlloc();
+	if (*key == TLS_OUT_OF_INDEXES) return ((errno=ENOMEM));
+	return 0;
+}
+
+void *usbi_tls_key_get(usbi_tls_key_t key) {
+	return TlsGetValue(key);
+}
+
+int usbi_tls_key_set(usbi_tls_key_t key, const void *value) {
+	if (!TlsSetValue(key, (LPVOID)value)) return ((errno=EINVAL));
+	return 0;
+}
+
+int usbi_tls_key_delete(usbi_tls_key_t key) {
+	if (!TlsFree(key)) return ((errno=EINVAL));
+	return 0;
+}
+
 int usbi_get_tid(void) {
 	return GetCurrentThreadId();
 }
diff --git a/libusb/os/threads_windows.h b/libusb/os/threads_windows.h
index 2b82925..6a35d9d 100644
--- a/libusb/os/threads_windows.h
+++ b/libusb/os/threads_windows.h
@@ -57,6 +57,7 @@ struct timespec {
 
 #define usbi_mutexattr_t void
 #define usbi_condattr_t  void
+#define usbi_tls_key_t   DWORD
 
 // all Windows mutexes are recursive
 #define usbi_mutex_init_recursive(mutex, attr) usbi_mutex_init((mutex), (attr))
@@ -82,6 +83,11 @@ int usbi_cond_timedwait(usbi_cond_t *cond,
 int usbi_cond_broadcast(usbi_cond_t *cond);
 int usbi_cond_signal(usbi_cond_t *cond);
 
+int   usbi_tls_key_create(usbi_tls_key_t *key, void (*destructor)(void *));
+void *usbi_tls_key_get(usbi_tls_key_t key);
+int   usbi_tls_key_set(usbi_tls_key_t key, const void *value);
+int   usbi_tls_key_delete(usbi_tls_key_t key);
+
 int usbi_get_tid(void);
 
 #endif /* LIBUSB_THREADS_WINDOWS_H */
diff --git a/libusb/sync.c b/libusb/sync.c
index 61a8b9c..0d1acd8 100644
--- a/libusb/sync.c
+++ b/libusb/sync.c
@@ -86,17 +86,22 @@ static void sync_transfer_wait_for_completion(struct libusb_transfer *transfer)
  * \returns LIBUSB_ERROR_PIPE if the control request was not supported by the
  * device
  * \returns LIBUSB_ERROR_NO_DEVICE if the device has been disconnected
+ * \returns LIBUSB_ERROR_BUSY if called from event handling context
  * \returns another LIBUSB_ERROR code on other failures
  */
 int API_EXPORTED libusb_control_transfer(libusb_device_handle *dev_handle,
 	uint8_t bmRequestType, uint8_t bRequest, uint16_t wValue, uint16_t wIndex,
 	unsigned char *data, uint16_t wLength, unsigned int timeout)
 {
-	struct libusb_transfer *transfer = libusb_alloc_transfer(0);
+	struct libusb_transfer *transfer;
 	unsigned char *buffer;
 	int completed = 0;
 	int r;
 
+	if (usbi_handling_events(HANDLE_CTX(dev_handle)))
+		return LIBUSB_ERROR_BUSY;
+
+	transfer = libusb_alloc_transfer(0);
 	if (!transfer)
 		return LIBUSB_ERROR_NO_MEM;
 
@@ -160,10 +165,14 @@ static int do_sync_bulk_transfer(struct libusb_device_handle *dev_handle,
 	unsigned char endpoint, unsigned char *buffer, int length,
 	int *transferred, unsigned int timeout, unsigned char type)
 {
-	struct libusb_transfer *transfer = libusb_alloc_transfer(0);
+	struct libusb_transfer *transfer;
 	int completed = 0;
 	int r;
 
+	if (usbi_handling_events(HANDLE_CTX(dev_handle)))
+		return LIBUSB_ERROR_BUSY;
+
+	transfer = libusb_alloc_transfer(0);
 	if (!transfer)
 		return LIBUSB_ERROR_NO_MEM;
 
@@ -248,6 +257,7 @@ static int do_sync_bulk_transfer(struct libusb_device_handle *dev_handle,
  * \returns LIBUSB_ERROR_OVERFLOW if the device offered more data, see
  * \ref packetoverflow
  * \returns LIBUSB_ERROR_NO_DEVICE if the device has been disconnected
+ * \returns LIBUSB_ERROR_BUSY if called from event handling context
  * \returns another LIBUSB_ERROR code on other failures
  */
 int API_EXPORTED libusb_bulk_transfer(struct libusb_device_handle *dev_handle,
@@ -297,6 +307,7 @@ int API_EXPORTED libusb_bulk_transfer(struct libusb_device_handle *dev_handle,
  * \returns LIBUSB_ERROR_OVERFLOW if the device offered more data, see
  * \ref packetoverflow
  * \returns LIBUSB_ERROR_NO_DEVICE if the device has been disconnected
+ * \returns LIBUSB_ERROR_BUSY if called from event handling context
  * \returns another LIBUSB_ERROR code on other error
  */
 int API_EXPORTED libusb_interrupt_transfer(
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 117d1f6..2453c84 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11012
+#define LIBUSB_NANO 11013