lkml.org 
[lkml]   [2020]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: WARNING in usbhid_raw_request/usb_submit_urb (3)
On Thu, 23 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_raw_request/usb_submit_urb
>
> usb 2-1: Ep 0 disabled: 4

Nasty! This indicates an URB is being submitted while the device is
being reset.

The URB comes from an hidraw ioctl request. I wonder where the reset
comes from? We can try to find out -- but I don't think this will lead
directly to a solution.

In general there is no mutual exclusion between USB I/O and resets.
Unless the reset came through the usbhid driver, I don't see how we can
prevent this from happening again.

I suppose we _could_ change the code so that it would be treated as an
ordinary I/O error, maybe with dev_warn instead of dev_WARN. After
all, failure to find ep 0 can hardly be called a driver bug. Anyway,
let's see what the next test turns up.

Alan Stern

#syz test: https://github.com/google/kasan.git 0fa84af8

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5762,10 +5766,14 @@ static int usb_reset_and_verify_device(s
bos = udev->bos;
udev->bos = NULL;

+ dev_info(&udev->dev, "Device reset\n");
+ dump_stack();
+
for (i = 0; i < SET_CONFIG_TRIES; ++i) {

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;
\
 
 \ /
  Last update: 2020-04-23 23:09    [W:0.135 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site