| Message ID | 20210113194510.GA1290698@rowland.harvard.edu |
|---|---|
| State | New |
| Headers | show |
| Series | USB: gadget: dummy-hcd: Fix errors in port-reset handling | expand |
Alan Stern <stern@rowland.harvard.edu> writes: > Commit c318840fb2a4 ("USB: Gadget: dummy-hcd: Fix shift-out-of-bounds > bug") messed up the way dummy-hcd handles requests to turn on the > RESET port feature (I didn't notice that the original switch case > ended with a fallthrough). The call to set_link_state() was > inadvertently removed, as was the code to set the USB_PORT_STAT_RESET > flag when the speed is USB2. > > In addition, the original code never checked whether the port was > connected before handling the port-reset request. There was a check > for the port being powered, but it was removed by that commit! In > practice this doesn't matter much because the kernel doesn't try to > reset disconnected ports, but it's still bad form. > > This patch fixes these problems by changing the fallthrough to break, > adding back in the missing set_link_state() call, setting the > port-reset status flag, adding a port-is-connected test, and removing > a redundant assignment statement. > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > Fixes: c318840fb2a4 ("USB: Gadget: dummy-hcd: Fix shift-out-of-bounds bug") > CC: <stable@vger.kernel.org> Acked-by: Felipe Balbi <balbi@kernel.org> -- balbi
Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c =================================================================== --- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c +++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c @@ -2266,17 +2266,20 @@ static int dummy_hub_control( } fallthrough; case USB_PORT_FEAT_RESET: + if (!(dum_hcd->port_status & USB_PORT_STAT_CONNECTION)) + break; /* if it's already enabled, disable */ if (hcd->speed == HCD_USB3) { - dum_hcd->port_status = 0; dum_hcd->port_status = (USB_SS_PORT_STAT_POWER | USB_PORT_STAT_CONNECTION | USB_PORT_STAT_RESET); - } else + } else { dum_hcd->port_status &= ~(USB_PORT_STAT_ENABLE | USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED); + dum_hcd->port_status |= USB_PORT_STAT_RESET; + } /* * We want to reset device status. All but the * Self powered feature @@ -2288,7 +2291,8 @@ static int dummy_hub_control( * interval? Is it still 50msec as for HS? */ dum_hcd->re_timeout = jiffies + msecs_to_jiffies(50); - fallthrough; + set_link_state(dum_hcd); + break; case USB_PORT_FEAT_C_CONNECTION: case USB_PORT_FEAT_C_RESET: case USB_PORT_FEAT_C_ENABLE:
Commit c318840fb2a4 ("USB: Gadget: dummy-hcd: Fix shift-out-of-bounds bug") messed up the way dummy-hcd handles requests to turn on the RESET port feature (I didn't notice that the original switch case ended with a fallthrough). The call to set_link_state() was inadvertently removed, as was the code to set the USB_PORT_STAT_RESET flag when the speed is USB2. In addition, the original code never checked whether the port was connected before handling the port-reset request. There was a check for the port being powered, but it was removed by that commit! In practice this doesn't matter much because the kernel doesn't try to reset disconnected ports, but it's still bad form. This patch fixes these problems by changing the fallthrough to break, adding back in the missing set_link_state() call, setting the port-reset status flag, adding a port-is-connected test, and removing a redundant assignment statement. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Fixes: c318840fb2a4 ("USB: Gadget: dummy-hcd: Fix shift-out-of-bounds bug") CC: <stable@vger.kernel.org> --- [as1952] drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)