Re: usbd_do_request_flags_pipe diagnostic panic

[Nick Hudson <skrll%netbsd.org@localhost>]

On 06/06/15 05:39, takahiro hayashi wrote:
> Hello,
>
> On nick-nhusb branch kernel panics in usbd_transfer() when the
> zero-length request gets stalled.
>
> This happens when the uhidev driver issues usbd_set_idle to my
> USB keyboard, one of its uhidevs returns stall for SET_IDLE request.
>
> While usbd_do_request_flags_pipe() processes the xfer for SET_IDLE,
> it tries to read endpoint's status and clear stall condition
> if the endpoint is stalled.
> It reuses the xfer to store GET_STATUS request, but ux_buf of xfer
> is not allocated, then KASSERT at line 298 in usbd_transfer() fails.
>
> Should usbd_do_request*() allocate ux_buf even if ux_length is 0?
> Should I file PR this prob?

How about this diff?

Nick

Index: dev/usb/usbdi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.162.2.26
diff -u -p -r1.162.2.26 usbdi.c
--- dev/usb/usbdi.c	30 Mar 2015 12:09:30 -0000	1.162.2.26
+++ dev/usb/usbdi.c	7 Jun 2015 10:49:55 -0000
@@ -1062,32 +1062,34 @@ usbd_do_request_flags_pipe(struct usbd_d
 		 * any halt condition.
 		 */
 		usb_device_request_t treq;
-		usb_status_t status;
-		uint16_t s;
 		usbd_status nerr;
 
-		treq.bmRequestType = UT_READ_ENDPOINT;
-		treq.bRequest = UR_GET_STATUS;
-		USETW(treq.wValue, 0);
-		USETW(treq.wIndex, 0);
-		USETW(treq.wLength, sizeof(usb_status_t));
-		usbd_setup_default_xfer(xfer, dev, 0, USBD_DEFAULT_TIMEOUT,
-					   &treq, &status,sizeof(usb_status_t),
-					   0, 0);
-		nerr = usbd_sync_transfer(xfer);
-		if (nerr)
-			goto bad;
-		s = UGETW(status.wStatus);
-		USBHIST_LOG(usbdebug, "status = 0x%04x", s, 0, 0, 0);
-		if (!(s & UES_HALT))
-			goto bad;
+		void *buf = usbd_alloc_buffer(xfer, sizeof(usb_status_t));
+		if (buf) {
+			usb_status_t status;
+			treq.bmRequestType = UT_READ_ENDPOINT;
+			treq.bRequest = UR_GET_STATUS;
+			USETW(treq.wValue, 0);
+			USETW(treq.wIndex, 0);
+			USETW(treq.wLength, sizeof(usb_status_t));
+			usbd_setup_default_xfer(xfer, dev, 0,
+			    USBD_DEFAULT_TIMEOUT, &treq, &status,
+			    sizeof(usb_status_t), 0, 0);
+			nerr = usbd_sync_transfer(xfer);
+			if (nerr)
+				goto bad;
+			uint16_t s = UGETW(status.wStatus);
+			USBHIST_LOG(usbdebug, "status = 0x%04x", s, 0, 0, 0);
+			if (!(s & UES_HALT))
+				goto bad;
+		}
 		treq.bmRequestType = UT_WRITE_ENDPOINT;
 		treq.bRequest = UR_CLEAR_FEATURE;
 		USETW(treq.wValue, UF_ENDPOINT_HALT);
 		USETW(treq.wIndex, 0);
 		USETW(treq.wLength, 0);
 		usbd_setup_default_xfer(xfer, dev, 0, USBD_DEFAULT_TIMEOUT,
-					   &treq, &status, 0, 0, 0);
+		    &treq, NULL, 0, 0, 0);
 		nerr = usbd_sync_transfer(xfer);
 		if (nerr)
 			goto bad;