Re: [PATCH v2] USB: ehci_brcm_hub_control: filter out invalid port index

[Alan Stern <stern@rowland.harvard.edu>]

On Thu, Nov 11, 2021 at 08:10:37PM +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. Remove cases greater than 0xf, because such cases
> are meaningless. It is reasonable to return -EPIPE, the caller
> rh_call_control will check the return value and handle it properly.
> The callee ehci_hub_control also doesn't need to deal with situations
> greater than 0xf. The size of the array port_status determines that
> such situations should not be handled.
> 
> Reported-by: TCS Robot <tcs_robot@tencent.com>
> Signed-off-by: Haimin Zhang <tcs.kernel@gmail.com>
> ---
>  drivers/usb/host/ehci-brcm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-brcm.c b/drivers/usb/host/ehci-brcm.c
> index d3626bfa966b..07e6df336a08 100644
> --- a/drivers/usb/host/ehci-brcm.c
> +++ b/drivers/usb/host/ehci-brcm.c
> @@ -63,6 +63,9 @@ static int ehci_brcm_hub_control(
>  	unsigned long flags;
>  	int retval, irq_disabled = 0;
>  
> +	if (!wIndex || wIndex > ports)
> +		return -EPIPE;

This is just like your original version of the patch.  It won't work 
right if wIndex is equal to 0x0101, for instance.

Alan Stern

> +
>  	status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1];
>  
>  	/*
> @@ -70,7 +73,6 @@ static int ehci_brcm_hub_control(
>  	 * of RESUME
>  	 */
>  	if ((typeReq == GetPortStatus) &&
> -	    (wIndex && wIndex <= ports) &&
>  	    ehci->reset_done[wIndex-1] &&
>  	    time_after_eq(jiffies, ehci->reset_done[wIndex-1]) &&
>  	    (ehci_readl(ehci, status_reg) & PORT_RESUME)) {
> -- 
> 2.30.1 (Apple Git-130)
>