在 2023年7月1日星期六 CST 下午11:51:43,Zhang Shurong 写道: > 在 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); Hi there. I apologize for reaching out once more. I'm feeling a bit puzzled about what my next step should be. I'm unsure whether I should rewrite this patch or attempt to address the issue at the caller level.