From dd7df2b75118c920cb21b4442983168e8b2f14ac Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Wed, 14 Aug 2019 14:00:41 -0700 Subject: 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 --- libusb/io.c | 32 +++++++++++++++++++++++++++++++- libusb/libusbi.h | 5 +++++ libusb/version_nano.h | 2 +- 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 * Copyright © 2001 Johannes Erdfelt + * Copyright © 2019 Nathan Hjelm + * 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 * Copyright © 2001 Johannes Erdfelt + * Copyright © 2019 Nathan Hjelm + * 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 -- cgit v1.2.1