[src/trunk]: src/sys/dev/usb usbdi(9): Assert no concurrent aborts on a singl...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/1c8285880501
branches:  trunk
changeset: 363462:1c8285880501
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun Mar 13 11:28:42 2022 +0000

description:
usbdi(9): Assert no concurrent aborts on a single pipe.

It is a driver bug to try to abort a pipe at the same time in two
different threads.

HCI drivers may release the bus lock to sleep in upm_abort while
waiting for the hardware to acknowledge an abort, so it won't try to,
e.g., scribble over a DMA buffer in the xfer that we've recycled
after usbd_abort_pipe returns.

If this happens, a concurrent usbd_abort_pipe might try to apply
upm_abort to the same xfer, which HCI drivers are not prepared for
and may wreak havoc.

To avoid this, allow only one usbd_abort_pipe in flight at any given
time.

diffstat:

 sys/dev/usb/usb_subr.c |   7 +++++--
 sys/dev/usb/usbdi.c    |  20 ++++++++++++++++++--
 sys/dev/usb/usbdivar.h |   4 +++-
 3 files changed, 26 insertions(+), 5 deletions(-)

diffs (101 lines):

diff -r 7fb86d1c863c -r 1c8285880501 sys/dev/usb/usb_subr.c
--- a/sys/dev/usb/usb_subr.c    Sun Mar 13 11:28:33 2022 +0000
+++ b/sys/dev/usb/usb_subr.c    Sun Mar 13 11:28:42 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usb_subr.c,v 1.270 2022/03/03 06:13:35 riastradh Exp $ */
+/*     $NetBSD: usb_subr.c,v 1.271 2022/03/13 11:28:42 riastradh Exp $ */
 /*     $FreeBSD: src/sys/dev/usb/usb_subr.c,v 1.18 1999/11/17 22:33:47 n_hibma Exp $   */
 
 /*
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.270 2022/03/03 06:13:35 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.271 2022/03/13 11:28:42 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -948,6 +948,7 @@
        SIMPLEQ_INIT(&p->up_queue);
        p->up_callingxfer = NULL;
        cv_init(&p->up_callingcv, "usbpipecb");
+       p->up_abortlwp = NULL;
 
        err = dev->ud_bus->ub_methods->ubm_open(p);
        if (err) {
@@ -967,6 +968,8 @@
        err = USBD_NORMAL_COMPLETION;
 
 out:   if (p) {
+               KASSERT(p->up_abortlwp == NULL);
+               KASSERT(p->up_callingxfer == NULL);
                cv_destroy(&p->up_callingcv);
                kmem_free(p, dev->ud_bus->ub_pipesize);
        }
diff -r 7fb86d1c863c -r 1c8285880501 sys/dev/usb/usbdi.c
--- a/sys/dev/usb/usbdi.c       Sun Mar 13 11:28:33 2022 +0000
+++ b/sys/dev/usb/usbdi.c       Sun Mar 13 11:28:42 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdi.c,v 1.234 2022/03/13 11:28:33 riastradh Exp $    */
+/*     $NetBSD: usbdi.c,v 1.235 2022/03/13 11:28:42 riastradh Exp $    */
 
 /*
  * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.234 2022/03/13 11:28:33 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.235 2022/03/13 11:28:42 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1020,6 +1020,16 @@
        ASSERT_SLEEPABLE();
        KASSERT(mutex_owned(pipe->up_dev->ud_bus->ub_lock));
 
+       /*
+        * Allow only one thread at a time to abort the pipe, so we
+        * don't get confused if upm_abort drops the lock in the middle
+        * of the abort to wait for hardware completion softints to
+        * stop using the xfer before returning.
+        */
+       KASSERTMSG(pipe->up_abortlwp == NULL, "pipe->up_abortlwp=%p",
+           pipe->up_abortlwp);
+       pipe->up_abortlwp = curlwp;
+
 #ifdef USB_DEBUG
        if (usbdebug > 5)
                usbd_dump_queue(pipe);
@@ -1051,6 +1061,12 @@
                        /* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
                }
        }
+
+       KASSERT(mutex_owned(pipe->up_dev->ud_bus->ub_lock));
+       KASSERTMSG(pipe->up_abortlwp == NULL, "pipe->up_abortlwp=%p",
+           pipe->up_abortlwp);
+       pipe->up_abortlwp = NULL;
+
        SDT_PROBE1(usb, device, pipe, abort__done,  pipe);
 }
 
diff -r 7fb86d1c863c -r 1c8285880501 sys/dev/usb/usbdivar.h
--- a/sys/dev/usb/usbdivar.h    Sun Mar 13 11:28:33 2022 +0000
+++ b/sys/dev/usb/usbdivar.h    Sun Mar 13 11:28:42 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdivar.h,v 1.135 2022/03/09 22:17:41 riastradh Exp $ */
+/*     $NetBSD: usbdivar.h,v 1.136 2022/03/13 11:28:42 riastradh Exp $ */
 
 /*
  * Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -257,6 +257,8 @@
        struct usbd_xfer       *up_callingxfer; /* currently in callback */
        kcondvar_t              up_callingcv;
 
+       struct lwp             *up_abortlwp;    /* lwp currently aborting */
+
        /* Filled by HC driver. */
        const struct usbd_pipe_methods
                               *up_methods;