On Sat, Nov 13, 2021 at 12:57:14PM +0800, Haimin Zhang wrote: > Due to (wIndex & 0xff) - 1 can get an integer greater than 0xf, this > can cause array index to be out of bounds since the size of array > port_status is 0xf. This change prevents a possible out-of-bounds > pointer computation by forcing the use of a valid port number. I would write 15 instead of 0xf. The size of the array is 15; see the definition of HCS_N_PORTS_MAX. (Yes, I realize the 0xf is equal to 15, but why force people to go out of their way to translate a hex number into decimal?) > > Reported-by: TCS Robot <tcs_robot@xxxxxxxxxxx> > Signed-off-by: Haimin Zhang <tcs.kernel@xxxxxxxxx> > --- You did not include the version change information here. > drivers/usb/host/ehci-brcm.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ehci-brcm.c b/drivers/usb/host/ehci-brcm.c > index d3626bfa966b..2e92918a14dd 100644 > --- a/drivers/usb/host/ehci-brcm.c > +++ b/drivers/usb/host/ehci-brcm.c > @@ -62,8 +62,12 @@ static int ehci_brcm_hub_control( > u32 __iomem *status_reg; > unsigned long flags; > int retval, irq_disabled = 0; > + u32 temp; > > - status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1]; > + temp = (wIndex & 0xff) - 1; > + if (temp >= HCS_N_PORTS_MAX) > + temp = 0; There ought to be a comment explaining why you are doing this (namely, to avoid an "index out of bounds" warning). The reason isn't automatically obvious. Alan Stern > + status_reg = &ehci->regs->port_status[temp]; > > /* > * RESUME is cleared when GetPortStatus() is called 20ms after start > -- > 2.30.1 (Apple Git-130) >
