aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHans Petter Selasky <hselasky@FreeBSD.org>2021-10-24 11:38:04 +0000
committerHans Petter Selasky <hselasky@FreeBSD.org>2021-11-02 08:43:44 +0000
commitbb9bee1ffbb27f903bfd2c11d681d331bea727ea (patch)
treef23a9c47246ab45e1b723e5f525051c7c2daf3c8
parentc8e557c999384bbb0f4d45915e191d7aff185475 (diff)
downloadsrc-bb9bee1ffbb27f903bfd2c11d681d331bea727ea.tar.gz
src-bb9bee1ffbb27f903bfd2c11d681d331bea727ea.zip
usb(4): Fix for use after free in combination with EVDEV_SUPPORT.
When EVDEV_SUPPORT was introduced, the USB transfers may be running after the main FIFO is closed. In connection to this a race may appear which can lead to use-after-free scenarios. Fix this for all FIFO consumers by initializing and resetting the FIFO queues under the lock used by the client. Then the client driver will see an empty queue in all cases a race may appear. Found by: pho@ Sponsored by: NVIDIA Networking (cherry picked from commit aad0c65d6b37364d8ba92ecb8c85e004398a5194)
-rw-r--r--sys/dev/usb/usb_dev.c47
1 files changed, 34 insertions, 13 deletions
diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c
index 9cd54bf0032c..d1a65bf8561b 100644
--- a/sys/dev/usb/usb_dev.c
+++ b/sys/dev/usb/usb_dev.c
@@ -1951,18 +1951,30 @@ int
usb_fifo_alloc_buffer(struct usb_fifo *f, usb_size_t bufsize,
uint16_t nbuf)
{
+ struct usb_ifqueue temp_q = {};
+ void *queue_data;
+
usb_fifo_free_buffer(f);
- /* allocate an endpoint */
- f->free_q.ifq_maxlen = nbuf;
- f->used_q.ifq_maxlen = nbuf;
+ temp_q.ifq_maxlen = nbuf;
- f->queue_data = usb_alloc_mbufs(
- M_USBDEV, &f->free_q, bufsize, nbuf);
+ queue_data = usb_alloc_mbufs(
+ M_USBDEV, &temp_q, bufsize, nbuf);
- if ((f->queue_data == NULL) && bufsize && nbuf) {
+ if (queue_data == NULL && bufsize != 0 && nbuf != 0)
return (ENOMEM);
- }
+
+ mtx_lock(f->priv_mtx);
+
+ /*
+ * Setup queues and sizes under lock to avoid early use by
+ * concurrent FIFO access:
+ */
+ f->free_q = temp_q;
+ f->used_q.ifq_maxlen = nbuf;
+ f->queue_data = queue_data;
+ mtx_unlock(f->priv_mtx);
+
return (0); /* success */
}
@@ -1975,15 +1987,24 @@ usb_fifo_alloc_buffer(struct usb_fifo *f, usb_size_t bufsize,
void
usb_fifo_free_buffer(struct usb_fifo *f)
{
- if (f->queue_data) {
- /* free old buffer */
- free(f->queue_data, M_USBDEV);
- f->queue_data = NULL;
- }
- /* reset queues */
+ void *queue_data;
+
+ mtx_lock(f->priv_mtx);
+
+ /* Get and clear pointer to free, if any. */
+ queue_data = f->queue_data;
+ f->queue_data = NULL;
+ /*
+ * Reset queues under lock to avoid use of freed buffers by
+ * concurrent FIFO activity:
+ */
memset(&f->free_q, 0, sizeof(f->free_q));
memset(&f->used_q, 0, sizeof(f->used_q));
+ mtx_unlock(f->priv_mtx);
+
+ /* Free old buffer, if any. */
+ free(queue_data, M_USBDEV);
}
void