d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

It was possible to have two threads concurrently aborting the same

Browse Source 
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
This commit is contained in:
iedowse 2005-03-19 19:08:46 +00:00
parent 18c31229e8
commit cc1d054d9c
6 changed files with 92 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
sys/dev/usb/ehci.c
View File

@ -1179,6 +1179,7 @@ ehci_allocx(struct usbd_bus *bus)
memset(xfer, 0, sizeof(struct ehci_xfer));
usb_init_task(&EXFER(xfer)->abort_task, ehci_timeout_task,
xfer);
EXFER(xfer)->ehci_xfer_flags = 0;
#ifdef DIAGNOSTIC
EXFER(xfer)->isdone = 1;
xfer->busy_free = XFER_BUSY;
@ -2518,10 +2519,29 @@ ehci_abort_xfer(usbd_xfer_handle xfer, usbd_status status)
if (xfer->device->bus->intr_context || !curproc)
panic("ehci_abort_xfer: not in process context");
/*
* If an abort is already in progress then just wait for it to
* complete and return.
*/
if (exfer->ehci_xfer_flags & EHCI_XFER_ABORTING) {
DPRINTFN(2, ("ehci_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, ("ehci_abort_xfer: waiting for abort to finish\n"));
exfer->ehci_xfer_flags |= EHCI_XFER_ABORTWAIT;
while (exfer->ehci_xfer_flags & EHCI_XFER_ABORTING)
tsleep(&exfer->ehci_xfer_flags, PZERO, "ehciaw", 0);
return;
}
/*
* Step 1: Make interrupt routine and timeouts ignore xfer.
*/
s = splusb();
exfer->ehci_xfer_flags |= EHCI_XFER_ABORTING;
xfer->status = status; /* make software ignore it */
usb_uncallout(xfer->timeout_handle, ehci_timeout, xfer);
usb_rem_task(epipe->pipe.device, &exfer->abort_task);
@ -2639,6 +2659,12 @@ ehci_abort_xfer(usbd_xfer_handle xfer, usbd_status status)
#ifdef DIAGNOSTIC
exfer->isdone = 1;
#endif
/* Do the wakeup first to avoid touching the xfer after the callback. */
exfer->ehci_xfer_flags &= ~EHCI_XFER_ABORTING;
if (exfer->ehci_xfer_flags & EHCI_XFER_ABORTWAIT) {
exfer->ehci_xfer_flags &= ~EHCI_XFER_ABORTWAIT;
wakeup(&exfer->ehci_xfer_flags);
}
usb_transfer_complete(xfer);
/* printf("%s: %d TDs aborted\n", __func__, count); */

4
sys/dev/usb/ehcivar.h
View File

@ -65,10 +65,14 @@ struct ehci_xfer {
LIST_ENTRY(ehci_xfer) inext; /* list of active xfers */
ehci_soft_qtd_t *sqtdstart;
ehci_soft_qtd_t *sqtdend;
u_int32_t ehci_xfer_flags;
#ifdef DIAGNOSTIC
int isdone;
#endif
};
#define EHCI_XFER_ABORTING 0x0001 /* xfer is aborting. */
#define EHCI_XFER_ABORTWAIT 0x0002 /* abort completion is being awaited. */
#define EXFER(xfer) ((struct ehci_xfer *)(xfer))
/*

28
sys/dev/usb/ohci.c
View File

@ -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);

2
sys/dev/usb/ohcivar.h
View File

@ -159,6 +159,8 @@ struct ohci_xfer {
u_int32_t ohci_xfer_flags;
};
#define OHCI_ISOC_DIRTY 0x01
#define OHCI_XFER_ABORTING 0x02 /* xfer is aborting. */
#define OHCI_XFER_ABORTWAIT 0x04 /* abort completion is being awaited. */
#define OXFER(xfer) ((struct ohci_xfer *)(xfer))

29
sys/dev/usb/uhci.c
View File

@ -646,6 +646,7 @@ uhci_allocx(struct usbd_bus *bus)
UXFER(xfer)->iinfo.sc = sc;
usb_init_task(&UXFER(xfer)->abort_task, uhci_timeout_task,
xfer);
UXFER(xfer)->uhci_xfer_flags = 0;
#ifdef DIAGNOSTIC
UXFER(xfer)->iinfo.isdone = 1;
xfer->busy_free = XFER_BUSY;
@ -1933,7 +1934,8 @@ uhci_device_bulk_abort(usbd_xfer_handle xfer)
void
uhci_abort_xfer(usbd_xfer_handle xfer, usbd_status status)
{
uhci_intr_info_t *ii = &UXFER(xfer)->iinfo;
struct uhci_xfer *uxfer = UXFER(xfer);
uhci_intr_info_t *ii = &uxfer->iinfo;
struct uhci_pipe *upipe = (struct uhci_pipe *)xfer->pipe;
uhci_softc_t *sc = (uhci_softc_t *)upipe->pipe.device->bus;
uhci_soft_td_t *std;
@ -1955,10 +1957,29 @@ uhci_abort_xfer(usbd_xfer_handle xfer, usbd_status status)
if (xfer->device->bus->intr_context || !curproc)
panic("uhci_abort_xfer: not in process context");
/*
* If an abort is already in progress then just wait for it to
* complete and return.
*/
if (uxfer->uhci_xfer_flags & UHCI_XFER_ABORTING) {
DPRINTFN(2, ("uhci_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, ("uhci_abort_xfer: waiting for abort to finish\n"));
uxfer->uhci_xfer_flags |= UHCI_XFER_ABORTWAIT;
while (uxfer->uhci_xfer_flags & UHCI_XFER_ABORTING)
tsleep(&uxfer->uhci_xfer_flags, PZERO, "uhciaw", 0);
return;
}
/*
* Step 1: Make interrupt routine and hardware ignore xfer.
*/
s = splusb();
uxfer->uhci_xfer_flags |= UHCI_XFER_ABORTING;
xfer->status = status; /* make software ignore it */
usb_uncallout(xfer->timeout_handle, uhci_timeout, ii);
usb_rem_task(xfer->pipe->device, &UXFER(xfer)->abort_task);
@ -1992,6 +2013,12 @@ uhci_abort_xfer(usbd_xfer_handle xfer, usbd_status status)
#ifdef DIAGNOSTIC
ii->isdone = 1;
#endif
/* Do the wakeup first to avoid touching the xfer after the callback. */
uxfer->uhci_xfer_flags &= ~UHCI_XFER_ABORTING;
if (uxfer->uhci_xfer_flags & UHCI_XFER_ABORTWAIT) {
uxfer->uhci_xfer_flags &= ~UHCI_XFER_ABORTWAIT;
wakeup(&uxfer->uhci_xfer_flags);
}
usb_transfer_complete(xfer);
splx(s);
}

4
sys/dev/usb/uhcivar.h
View File

@ -85,8 +85,12 @@ struct uhci_xfer {
uhci_intr_info_t iinfo;
struct usb_task abort_task;
int curframe;
u_int32_t uhci_xfer_flags;
};
#define UHCI_XFER_ABORTING 0x0001 /* xfer is aborting. */
#define UHCI_XFER_ABORTWAIT 0x0002 /* abort completion is being awaited. */
#define UXFER(xfer) ((struct uhci_xfer *)(xfer))
/*