From a3d327674a9efb9b375b4c21fa0c08682642295c Mon Sep 17 00:00:00 2001 From: Ian Dowse Date: Sat, 19 Mar 2005 19:08:46 +0000 Subject: It was possible to have two threads concurrently aborting the same transfer, which lead to panics or page faults. For example if a transfer timed out, another thread could come along and attempt to abort the same transfer while the timeout task was sleeping in the *_abort_xfer() function. Add an "aborting" flag to the private transfer state in each host controller driver and use this to ensure that the abort is only executed once. Also prioritise normal abort requests over timeouts so that the callback is always given a status of USB_CANCELLED even if the timeout-initiated abort began first. The crashes caused by this bug were mainly reported in connection with lpd printing to a USB printer. PR: usb/78208, usb/78986 --- sys/dev/usb/ohci.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'sys/dev/usb/ohci.c') diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c index 47e8b3fa01dd..e0db856e797a 100644 --- a/sys/dev/usb/ohci.c +++ b/sys/dev/usb/ohci.c @@ -998,6 +998,7 @@ ohci_allocx(struct usbd_bus *bus) memset(xfer, 0, sizeof (struct ohci_xfer)); usb_init_task(&OXFER(xfer)->abort_task, ohci_timeout_task, xfer); + OXFER(xfer)->ohci_xfer_flags = 0; #ifdef DIAGNOSTIC xfer->busy_free = XFER_BUSY; #endif @@ -2254,6 +2255,7 @@ ohci_close_pipe(usbd_pipe_handle pipe, ohci_soft_ed_t *head) void ohci_abort_xfer(usbd_xfer_handle xfer, usbd_status status) { + struct ohci_xfer *oxfer = OXFER(xfer); struct ohci_pipe *opipe = (struct ohci_pipe *)xfer->pipe; ohci_softc_t *sc = (ohci_softc_t *)opipe->pipe.device->bus; ohci_soft_ed_t *sed = opipe->sed; @@ -2276,10 +2278,29 @@ ohci_abort_xfer(usbd_xfer_handle xfer, usbd_status status) if (xfer->device->bus->intr_context || !curproc) panic("ohci_abort_xfer: not in process context"); + /* + * If an abort is already in progress then just wait for it to + * complete and return. + */ + if (oxfer->ohci_xfer_flags & OHCI_XFER_ABORTING) { + DPRINTFN(2, ("ohci_abort_xfer: already aborting\n")); + /* No need to wait if we're aborting from a timeout. */ + if (status == USBD_TIMEOUT) + return; + /* Override the status which might be USBD_TIMEOUT. */ + xfer->status = status; + DPRINTFN(2, ("ohci_abort_xfer: waiting for abort to finish\n")); + oxfer->ohci_xfer_flags |= OHCI_XFER_ABORTWAIT; + while (oxfer->ohci_xfer_flags & OHCI_XFER_ABORTING) + tsleep(&oxfer->ohci_xfer_flags, PZERO, "ohciaw", 0); + return; + } + /* * Step 1: Make interrupt routine and hardware ignore xfer. */ s = splusb(); + oxfer->ohci_xfer_flags |= OHCI_XFER_ABORTING; xfer->status = status; /* make software ignore it */ usb_uncallout(xfer->timeout_handle, ohci_timeout, xfer); usb_rem_task(xfer->pipe->device, &OXFER(xfer)->abort_task); @@ -2314,6 +2335,7 @@ ohci_abort_xfer(usbd_xfer_handle xfer, usbd_status status) p = xfer->hcpriv; #ifdef DIAGNOSTIC if (p == NULL) { + oxfer->ohci_xfer_flags &= ~OHCI_XFER_ABORTING; /* XXX */ splx(s); printf("ohci_abort_xfer: hcpriv is NULL\n"); return; @@ -2350,6 +2372,12 @@ ohci_abort_xfer(usbd_xfer_handle xfer, usbd_status status) /* * Step 5: Execute callback. */ + /* Do the wakeup first to avoid touching the xfer after the callback. */ + oxfer->ohci_xfer_flags &= ~OHCI_XFER_ABORTING; + if (oxfer->ohci_xfer_flags & OHCI_XFER_ABORTWAIT) { + oxfer->ohci_xfer_flags &= ~OHCI_XFER_ABORTWAIT; + wakeup(&oxfer->ohci_xfer_flags); + } usb_transfer_complete(xfer); splx(s); -- cgit v1.2.3