Re: [PATCH v3] usb: chipidea: host: fix port index underflow and UBSAN complains

[Peter Chen <peter.chen@kernel.org>]

On 21-06-18 16:28:58, Li Jun wrote:
> If wIndex is 0 (and it often is), these calculations underflow and
> UBSAN complains, here resolve this by not decrementing the index when
> it is equal to 0, this copies the solution from commit 85e3990bea49
> ("USB: EHCI: avoid undefined pointer arithmetic and placate UBSAN")
> 
> Reported-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> Signed-off-by: Li Jun <jun.li@nxp.com>

Applied, thanks.

Peter
> ---
> Change for v3:
> -  Correct the bug's reporter's user name format.
> 
> Change for v2:
> -  Add wIndex range check to ensure a correct port index value.
> 
>  drivers/usb/chipidea/host.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index e86d13c04bdb..bdc3885c0d49 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -240,15 +240,18 @@ static int ci_ehci_hub_control(
>  )
>  {
>  	struct ehci_hcd	*ehci = hcd_to_ehci(hcd);
> +	unsigned int	ports = HCS_N_PORTS(ehci->hcs_params);
>  	u32 __iomem	*status_reg;
> -	u32		temp;
> +	u32		temp, port_index;
>  	unsigned long	flags;
>  	int		retval = 0;
>  	bool		done = false;
>  	struct device *dev = hcd->self.controller;
>  	struct ci_hdrc *ci = dev_get_drvdata(dev);
>  
> -	status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1];
> +	port_index = wIndex & 0xff;
> +	port_index -= (port_index > 0);
> +	status_reg = &ehci->regs->port_status[port_index];
>  
>  	spin_lock_irqsave(&ehci->lock, flags);
>  
> @@ -260,6 +263,11 @@ static int ci_ehci_hub_control(
>  	}
>  
>  	if (typeReq == SetPortFeature && wValue == USB_PORT_FEAT_SUSPEND) {
> +		if (!wIndex || wIndex > ports) {
> +			retval = -EPIPE;
> +			goto done;
> +		}
> +
>  		temp = ehci_readl(ehci, status_reg);
>  		if ((temp & PORT_PE) == 0 || (temp & PORT_RESET) != 0) {
>  			retval = -EPIPE;
> @@ -288,7 +296,7 @@ static int ci_ehci_hub_control(
>  			ehci_writel(ehci, temp, status_reg);
>  		}
>  
> -		set_bit((wIndex & 0xff) - 1, &ehci->suspended_ports);
> +		set_bit(port_index, &ehci->suspended_ports);
>  		goto done;
>  	}
>  
> -- 
> 2.25.1
> 

-- 

Thanks,
Peter Chen