usb/78986: crash when printing to usb connected printer

[Ian Dowse]

The following reply was made to PR usb/78986; it has been noted by GNATS.

From: Ian Dowse <iedowse at maths.tcd.ie>
To: Marc van Kempen <marc at bowtie.nl>
Cc: FreeBSD-gnats-submit at FreeBSD.org
Subject: Re: usb/78986: crash when printing to usb connected printer 
Date: Sat, 19 Mar 2005 15:40:34 +0000

 In message <200503181455.j2IEtfQx073610 at e16014.upc-e.chello.nl>, Marc van Kempe
 n writes:
 >Every once in a while, printing to my Samsung ML-1510 laserprinter connected
 >through usb will completely lock up the computer.
 >	
 >>How-To-Repeat:
 >Print several times until it locks up. The problem is that there doesn't 
 >seem to be a pattern to it.
 
 Hi, could you try the following patch. There appears to be a race
 condition when aborting a USB transfer while the transfer is currently
 being timed out. This fixes only the UHCI case - there are probably
 similar problems with the other host controllers.
 
 Ian
 
 Index: uhci.c
 ===================================================================
 RCS file: /dump/FreeBSD-CVS/src/sys/dev/usb/uhci.c,v
 retrieving revision 1.154.2.4
 diff -u -r1.154.2.4 uhci.c
 --- uhci.c	30 Jan 2005 01:00:10 -0000	1.154.2.4
 +++ uhci.c	19 Mar 2005 15:29:50 -0000
 @@ -646,6 +646,7 @@
  		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 @@
  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;
 @@ -1956,9 +1958,28 @@
  		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 @@
  #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);
  }
 Index: uhcivar.h
 ===================================================================
 RCS file: /dump/FreeBSD-CVS/src/sys/dev/usb/uhcivar.h,v
 retrieving revision 1.37.2.1
 diff -u -r1.37.2.1 uhcivar.h
 --- uhcivar.h	30 Jan 2005 01:00:10 -0000	1.37.2.1
 +++ uhcivar.h	19 Mar 2005 15:29:50 -0000
 @@ -85,8 +85,12 @@
  	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))
  
  /*