Finally plug the public xfer leak #1 in our USB stack.

Every call to usbd_abort_pipe() on an interrupt pipe would simply
reset the intrxfer pointer, which would prevent usbd_close_pipe()
to free it.  Since we abort pipes in a lot of situations: when a
device is detached, when a USB-to-serial adapter is closed, when
an error occurs, when the machine is suspended, etc, this would
result in hundreds of leaked xfers in most of my machines.

xhci(4) is not affected, but you can't enable it right now since
the stack is not ready :)

While here put a KASSERT() to make sure drivers are only calling
the interrupt abort method for intrxfer, if that's not the case,
please let met know.

diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c
index ef8e564..e6e5dbd 100644 (file)
--- a/sys/dev/usb/ehci.c
+++ b/sys/dev/usb/ehci.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ehci.c,v 1.148 2014/04/29 12:45:29 mpi Exp $ */
+/*     $OpenBSD: ehci.c,v 1.149 2014/04/29 14:11:23 mpi Exp $ */
 /*     $NetBSD: ehci.c,v 1.66 2004/06/30 03:11:56 mycroft Exp $        */
 
 /*
@@ -3448,11 +3448,8 @@ ehci_device_intr_start(struct usbd_xfer *xfer)
 void
 ehci_device_intr_abort(struct usbd_xfer *xfer)
 {
-       DPRINTFN(1, ("ehci_device_intr_abort: xfer=%p\n", xfer));
-       if (xfer->pipe->intrxfer == xfer) {
-               DPRINTFN(1, ("ehci_device_intr_abort: remove\n"));
-               xfer->pipe->intrxfer = NULL;
-       }
+       KASSERT(xfer->pipe->intrxfer == xfer);
+
        /*
         * XXX - abort_xfer uses ehci_sync_hc, which syncs via the advance
         *       async doorbell. That's dependant on the async list, wheras

diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c
index dc30c41..8571ee6 100644 (file)
--- a/sys/dev/usb/ohci.c
+++ b/sys/dev/usb/ohci.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ohci.c,v 1.125 2014/04/27 14:48:10 mpi Exp $ */
+/*     $OpenBSD: ohci.c,v 1.126 2014/04/29 14:11:23 mpi Exp $ */
 /*     $NetBSD: ohci.c,v 1.139 2003/02/22 05:24:16 tsutsui Exp $       */
 /*     $FreeBSD: src/sys/dev/usb/ohci.c,v 1.22 1999/11/17 22:33:40 n_hibma Exp $       */
 
@@ -2973,14 +2973,11 @@ ohci_device_intr_start(struct usbd_xfer *xfer)
        return (USBD_IN_PROGRESS);
 }
 
-/* Abort a device control request. */
 void
 ohci_device_intr_abort(struct usbd_xfer *xfer)
 {
-       if (xfer->pipe->intrxfer == xfer) {
-               DPRINTF(("ohci_device_intr_abort: remove\n"));
-               xfer->pipe->intrxfer = NULL;
-       }
+       KASSERT(xfer->pipe->intrxfer == xfer);
+
        ohci_abort_xfer(xfer, USBD_CANCELLED);
 }
 

diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c
index cc3e930..ebbcaf9 100644 (file)
--- a/sys/dev/usb/uhci.c
+++ b/sys/dev/usb/uhci.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uhci.c,v 1.111 2014/04/27 14:48:10 mpi Exp $  */
+/*     $OpenBSD: uhci.c,v 1.112 2014/04/29 14:11:23 mpi Exp $  */
 /*     $NetBSD: uhci.c,v 1.172 2003/02/23 04:19:26 simonb Exp $        */
 /*     $FreeBSD: src/sys/dev/usb/uhci.c,v 1.33 1999/11/17 22:33:41 n_hibma Exp $       */
 
@@ -2011,15 +2011,11 @@ uhci_device_ctrl_close(struct usbd_pipe *pipe)
 {
 }
 
-/* Abort a device interrupt request. */
 void
 uhci_device_intr_abort(struct usbd_xfer *xfer)
 {
-       DPRINTFN(1,("uhci_device_intr_abort: xfer=%p\n", xfer));
-       if (xfer->pipe->intrxfer == xfer) {
-               DPRINTFN(1,("uhci_device_intr_abort: remove\n"));
-               xfer->pipe->intrxfer = NULL;
-       }
+       KASSERT(xfer->pipe->intrxfer == xfer);
+
        uhci_abort_xfer(xfer, USBD_CANCELLED);
 }