From 73c7d6521058e64fc17622719374750e41fbc34c Mon Sep 17 00:00:00 2001
From: riastradh <riastradh@NetBSD.org>
Date: Sat, 15 Feb 2020 01:21:56 +0000
Subject: [PATCH] Fix mistakes in previous sloppy change with root intr xfers.

- Make sure ux_status is set to USBD_IN_PROGRESS when started.
  Otherwise, if it is still in flight when we abort the pipe,
  usbd_ar_pipe will skip calling upm_abort.

- Initialize ux_status under the lock; in principle a completion
  interrupt (or a delay) could race with the initialization.

- KASSERT that the xfer is in progress when we're about to complete
  it.

Candidate fix for PR kern/54963 for other HCI drivers than uhci.

ok nick
ok phone

(This is the change that nick evidently MEANT to ok when he ok'd the
previous one!)
---
 sys/arch/mips/adm5120/dev/ahci.c | 10 ++++++---
 sys/dev/ic/sl811hs.c             |  8 +++++--
 sys/dev/usb/ehci.c               |  9 ++++----
 sys/dev/usb/motg.c               | 37 ++++++++++++++++++++++++++++----
 sys/dev/usb/ohci.c               | 10 +++++----
 sys/dev/usb/uhci.c               |  9 ++++----
 sys/dev/usb/vhci.c               |  8 +++++--
 sys/dev/usb/xhci.c               |  6 ++++--
 sys/external/bsd/dwc2/dwc2.c     | 10 +++++----
 9 files changed, 78 insertions(+), 29 deletions(-)

diff --git a/sys/arch/mips/adm5120/dev/ahci.c b/sys/arch/mips/adm5120/dev/ahci.c
index 1c4905f2144a..ae1dcece0598 100644
--- a/sys/arch/mips/adm5120/dev/ahci.c
+++ b/sys/arch/mips/adm5120/dev/ahci.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: ahci.c,v 1.19 2020/02/12 16:02:01 riastradh Exp $	*/
+/*	$NetBSD: ahci.c,v 1.20 2020/02/15 01:21:56 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2007 Ruslan Ermilov and Vsevolod Lobko.
@@ -64,7 +64,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.19 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.20 2020/02/15 01:21:56 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -438,6 +438,7 @@ ahci_poll_hub(void *arg)
 	xfer = sc->sc_intr_xfer;
 	if (xfer == NULL)
 		goto out;
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	/*
 	 * If the intr xfer for which we were scheduled is done, and
@@ -754,11 +755,14 @@ ahci_root_intr_start(struct usbd_xfer *xfer)
 
 	DPRINTF(D_TRACE, ("SLRIstart "));
 
+	mutex_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intr_xfer == NULL);
-
 	sc->sc_interval = MS_TO_TICKS(xfer->ux_pipe->up_endpoint->ue_edesc->bInterval);
 	callout_schedule(&sc->sc_poll_handle, sc->sc_interval);
 	sc->sc_intr_xfer = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
+	mutex_exit(&sc->sc_lock);
+
 	return USBD_IN_PROGRESS;
 }
 
diff --git a/sys/dev/ic/sl811hs.c b/sys/dev/ic/sl811hs.c
index f20337c33c81..bf592f425071 100644
--- a/sys/dev/ic/sl811hs.c
+++ b/sys/dev/ic/sl811hs.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: sl811hs.c,v 1.102 2019/12/27 09:41:50 msaitoh Exp $	*/
+/*	$NetBSD: sl811hs.c,v 1.103 2020/02/15 01:21:56 riastradh Exp $	*/
 
 /*
  * Not (c) 2007 Matthew Orgass
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.102 2019/12/27 09:41:50 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.103 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_slhci.h"
@@ -1030,7 +1030,9 @@ slhci_root_start(struct usbd_xfer *xfer)
 	KASSERT(spipe->ptype == PT_ROOT_INTR);
 
 	mutex_enter(&sc->sc_intr_lock);
+	KASSERT(t->rootintr == NULL);
 	t->rootintr = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
 	mutex_exit(&sc->sc_intr_lock);
 
 	return USBD_IN_PROGRESS;
@@ -2389,6 +2391,8 @@ slhci_callback(struct slhci_softc *sc)
 			if (t->rootintr != NULL) {
 				u_char *p;
 
+				KASSERT(t->rootintr->ux_status ==
+				    USBD_IN_PROGRESS);
 				p = t->rootintr->ux_buf;
 				p[0] = 2;
 				t->rootintr->ux_actlen = 1;
diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c
index e657b0e40d68..ab1cefeb3dc5 100644
--- a/sys/dev/usb/ehci.c
+++ b/sys/dev/usb/ehci.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: ehci.c,v 1.272 2020/02/12 16:02:01 riastradh Exp $ */
+/*	$NetBSD: ehci.c,v 1.273 2020/02/15 01:21:56 riastradh Exp $ */
 
 /*
  * Copyright (c) 2004-2012 The NetBSD Foundation, Inc.
@@ -53,7 +53,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.272 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.273 2020/02/15 01:21:56 riastradh Exp $");
 
 #include "ohci.h"
 #include "uhci.h"
@@ -785,6 +785,7 @@ ehci_pcd(void *addr)
 		/* Just ignore the change. */
 		goto done;
 	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	p = xfer->ux_buf;
 	m = uimin(sc->sc_noport, xfer->ux_length * 8 - 1);
@@ -2724,11 +2725,11 @@ ehci_root_intr_start(struct usbd_xfer *xfer)
 		mutex_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intrxfer == NULL);
 	sc->sc_intrxfer = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
-	xfer->ux_status = USBD_IN_PROGRESS;
-	return xfer->ux_status;
+	return USBD_IN_PROGRESS;
 }
 
 /* Abort a root interrupt request. */
diff --git a/sys/dev/usb/motg.c b/sys/dev/usb/motg.c
index 9c2235c5f952..a95797e16f1d 100644
--- a/sys/dev/usb/motg.c
+++ b/sys/dev/usb/motg.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: motg.c,v 1.26 2020/02/12 16:01:00 riastradh Exp $	*/
+/*	$NetBSD: motg.c,v 1.27 2020/02/15 01:21:56 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2004, 2011, 2012, 2014 The NetBSD Foundation, Inc.
@@ -40,7 +40,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.26 2020/02/12 16:01:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.27 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -991,8 +991,16 @@ motg_root_intr_abort(struct usbd_xfer *xfer)
 	KASSERT(mutex_owned(&sc->sc_lock));
 	KASSERT(xfer->ux_pipe->up_intrxfer == xfer);
 
-	sc->sc_intr_xfer = NULL;
+	/* If xfer has already completed, nothing to do here.  */
+	if (sc->sc_intr_xfer == NULL)
+		return;
 
+	/*
+	 * Otherwise, sc->sc_intr_xfer had better be this transfer.
+	 * Cancel it.
+	 */
+	KASSERT(sc->sc_intr_xfer == xfer);
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	xfer->ux_status = USBD_CANCELLED;
 	usb_transfer_complete(xfer);
 }
@@ -1032,7 +1040,14 @@ motg_root_intr_start(struct usbd_xfer *xfer)
 	if (sc->sc_dying)
 		return USBD_IOERROR;
 
+	if (!polling)
+		mutex_enter(&sc->sc_lock);
+	KASSERT(sc->sc_intr_xfer == NULL);
 	sc->sc_intr_xfer = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
+	if (!polling)
+		mutex_exit(&sc->sc_lock);
+
 	return USBD_IN_PROGRESS;
 }
 
@@ -1045,12 +1060,25 @@ motg_root_intr_close(struct usbd_pipe *pipe)
 
 	KASSERT(mutex_owned(&sc->sc_lock));
 
-	sc->sc_intr_xfer = NULL;
+	/*
+	 * Caller must guarantee the xfer has completed first, by
+	 * closing the pipe only after normal completion or an abort.
+	 */
+	KASSERT(sc->sc_intr_xfer == NULL);
 }
 
 void
 motg_root_intr_done(struct usbd_xfer *xfer)
 {
+	struct motg_softc *sc = MOTG_PIPE2SC(pipe);
+	MOTGHIST_FUNC(); MOTGHIST_CALLED();
+
+	KASSERT(mutex_owned(&sc->sc_lock));
+
+	/* Claim the xfer so it doesn't get completed again.  */
+	KASSERT(sc->sc_intr_xfer == xfer);
+	KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
+	sc->sc_intr_xfer = NULL;
 }
 
 void
@@ -1101,6 +1129,7 @@ motg_hub_change(struct motg_softc *sc)
 
 	if (xfer == NULL)
 		return; /* the interrupt pipe is not open */
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	pipe = xfer->ux_pipe;
 	if (pipe->up_dev == NULL || pipe->up_dev->ud_bus == NULL)
diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c
index bd437d36135c..6cf01ac41439 100644
--- a/sys/dev/usb/ohci.c
+++ b/sys/dev/usb/ohci.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: ohci.c,v 1.294 2020/02/12 16:02:01 riastradh Exp $	*/
+/*	$NetBSD: ohci.c,v 1.295 2020/02/15 01:21:56 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2004, 2005, 2012 The NetBSD Foundation, Inc.
@@ -41,7 +41,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.294 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.295 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1698,6 +1698,8 @@ ohci_rhsc(ohci_softc_t *sc, struct usbd_xfer *xfer)
 		/* Just ignore the change. */
 		return;
 	}
+	KASSERT(xfer == sc->sc_intrxfer);
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	p = xfer->ux_buf;
 	m = uimin(sc->sc_noport, xfer->ux_length * 8 - 1);
@@ -2516,11 +2518,11 @@ ohci_root_intr_start(struct usbd_xfer *xfer)
 		mutex_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intrxfer == NULL);
 	sc->sc_intrxfer = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
-	xfer->ux_status = USBD_IN_PROGRESS;
-	return xfer->ux_status;
+	return USBD_IN_PROGRESS;
 }
 
 /* Abort a root interrupt request. */
diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c
index e3580f0deaca..f8412cbd06e2 100644
--- a/sys/dev/usb/uhci.c
+++ b/sys/dev/usb/uhci.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: uhci.c,v 1.292 2020/02/14 16:47:28 riastradh Exp $	*/
+/*	$NetBSD: uhci.c,v 1.293 2020/02/15 01:21:56 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2004, 2011, 2012 The NetBSD Foundation, Inc.
@@ -42,7 +42,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.292 2020/02/14 16:47:28 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.293 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1013,6 +1013,7 @@ uhci_poll_hub(void *addr)
 	xfer = sc->sc_intr_xfer;
 	if (xfer == NULL)
 		goto out;
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	/*
 	 * If the intr xfer for which we were scheduled is done, and
@@ -3905,12 +3906,12 @@ uhci_root_intr_start(struct usbd_xfer *xfer)
 	sc->sc_ival = mstohz(ival);
 	callout_schedule(&sc->sc_poll_handle, sc->sc_ival);
 	sc->sc_intr_xfer = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
 
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
-	xfer->ux_status = USBD_IN_PROGRESS;
-	return xfer->ux_status;
+	return USBD_IN_PROGRESS;
 }
 
 /* Close the root interrupt pipe. */
diff --git a/sys/dev/usb/vhci.c b/sys/dev/usb/vhci.c
index f1c43919d0c3..ff47837bd3aa 100644
--- a/sys/dev/usb/vhci.c
+++ b/sys/dev/usb/vhci.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: vhci.c,v 1.5 2020/02/12 16:02:01 riastradh Exp $ */
+/*	$NetBSD: vhci.c,v 1.6 2020/02/15 01:21:56 riastradh Exp $ */
 
 /*
  * Copyright (c) 2019 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vhci.c,v 1.5 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vhci.c,v 1.6 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -629,6 +629,7 @@ vhci_root_intr_start(struct usbd_xfer *xfer)
 		mutex_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intrxfer == NULL);
 	sc->sc_intrxfer = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
@@ -654,6 +655,7 @@ vhci_root_intr_abort(struct usbd_xfer *xfer)
 	 * Cancel it.
 	 */
 	KASSERT(sc->sc_intrxfer == xfer);
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	xfer->ux_status = USBD_CANCELLED;
 	usb_transfer_complete(xfer);
 }
@@ -748,6 +750,7 @@ vhci_usb_attach(vhci_fd_t *vfd, struct vhci_ioc_usb_attach *args)
 		ret = ENOBUFS;
 		goto done;
 	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	p = xfer->ux_buf;
 	memset(p, 0, xfer->ux_length);
@@ -823,6 +826,7 @@ vhci_usb_detach(vhci_fd_t *vfd, struct vhci_ioc_usb_detach *args)
 		mutex_exit(&sc->sc_lock);
 		return ENOBUFS;
 	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	mutex_enter(&port->lock);
 
diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index ebd97494630e..5a96b740742c 100644
--- a/sys/dev/usb/xhci.c
+++ b/sys/dev/usb/xhci.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: xhci.c,v 1.117 2020/02/12 16:02:01 riastradh Exp $	*/
+/*	$NetBSD: xhci.c,v 1.118 2020/02/15 01:21:56 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2013 Jonathan A. Kollasch
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.117 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.118 2020/02/15 01:21:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1879,6 +1879,7 @@ xhci_rhpsc(struct xhci_softc * const sc, u_int ctlrport)
 
 	if (xfer == NULL)
 		return;
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 
 	uint8_t *p = xfer->ux_buf;
 	memset(p, 0, xfer->ux_length);
@@ -3717,6 +3718,7 @@ xhci_root_intr_start(struct usbd_xfer *xfer)
 		mutex_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intrxfer[bn] == NULL);
 	sc->sc_intrxfer[bn] = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
diff --git a/sys/external/bsd/dwc2/dwc2.c b/sys/external/bsd/dwc2/dwc2.c
index d305c327a34f..9b4ae330676f 100644
--- a/sys/external/bsd/dwc2/dwc2.c
+++ b/sys/external/bsd/dwc2/dwc2.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: dwc2.c,v 1.68 2020/02/12 16:02:01 riastradh Exp $	*/
+/*	$NetBSD: dwc2.c,v 1.69 2020/02/15 01:21:56 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.68 2020/02/12 16:02:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.69 2020/02/15 01:21:56 riastradh Exp $");
 
 #include "opt_usb.h"
 
@@ -301,6 +301,8 @@ dwc2_rhc(void *addr)
 		return;
 
 	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
+
 	/* set port bit */
 	p = KERNADDR(&xfer->ux_dmabuf, 0);
 
@@ -646,11 +648,11 @@ dwc2_root_intr_start(struct usbd_xfer *xfer)
 		mutex_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intrxfer == NULL);
 	sc->sc_intrxfer = xfer;
+	xfer->ux_status = USBD_IN_PROGRESS;
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
-	xfer->ux_status = USBD_IN_PROGRESS;
-	return xfer->ux_status;
+	return USBD_IN_PROGRESS;
 }
 
 /* Abort a root interrupt request. */