在 2023年6月26日星期一 CST 下午1:52:02，您写道：
> On Mon, Jun 26, 2023 at 07:48:05AM +0200, Jan Beulich wrote:
> > On 25.06.2023 18:42, Zhang Shurong wrote:
> > > --- a/drivers/usb/host/xen-hcd.c
> > > +++ b/drivers/usb/host/xen-hcd.c
> > > @@ -456,6 +456,8 @@ static int xenhcd_hub_control(struct usb_hcd *hcd,
> > > __u16 typeReq, __u16 wValue,> >
> > > info->ports[wIndex - 1].c_connection =
false;
> > > fallthrough;
> > >
> > > default:
> > > + if (wValue >= 32)
> > > + goto error;
> > >
> > > info->ports[wIndex - 1].status &= ~(1
<< wValue);
> >
> > Even 31 is out of bounds (as in: UB) as long as it's 1 here rather
> > than 1u.
>
> Why isn't the caller fixed so this type of value could never be passed
> to the hub_control callback?
>
> thanks,
>
> greg k-h
Although I'm not knowledgeable about the USB subsystem, I've observed that not
all driver code that implements hub_control callback performs a shift
operation on wValue, and not all shift operations among them cause problems.
Therefore, I've decided to fix this issue within each driver itself.
For example, in r8a66597_hub_control, it will first check whether wValue is
valid (always < 31) before the shift operation. In case of an invalid number,
the code would execute the error branch instead of the shift operation.
switch (wValue) {
case USB_PORT_FEAT_ENABLE:
rh->port &= ~USB_PORT_STAT_POWER;
break;
case USB_PORT_FEAT_SUSPEND:
break;
case USB_PORT_FEAT_POWER:
r8a66597_port_power(r8a66597, port, 0);
break;
case USB_PORT_FEAT_C_ENABLE:
case USB_PORT_FEAT_C_SUSPEND:
case USB_PORT_FEAT_C_CONNECTION:
case USB_PORT_FEAT_C_OVER_CURRENT:
case USB_PORT_FEAT_C_RESET:
break;
default:
goto error;
}
rh->port &= ~(1 << wValue);
