diff mbox series

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

Message ID 1624004938-2399-1-git-send-email-jun.li@nxp.com (mailing list archive)
State Accepted
Commit e5d6a7c6cfae9e714a0e8ff64facd1ac68a784c6
Headers show
Series [v3] usb: chipidea: host: fix port index underflow and UBSAN complains | expand

Commit Message

Jun Li June 18, 2021, 8:28 a.m. UTC
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>
---
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(-)

Comments

Peter Chen June 22, 2021, 1:03 a.m. UTC | #1
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
>
diff mbox series

Patch

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;
 	}