Re: [PATCH] usb: chipidea: host: Improve port index sanitizing

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Nov 29, 2024 at 07:33:18PM +0800, Xu Yang wrote:
> Coverity complains "Illegal address computation (OVERRUN)" on status_reg.
> This will follow "846cbf98cbef USB: EHCI: Improve port index sanitizing" to
> improve port index sanitizing.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/chipidea/host.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 0cce19208370..442d79e32a65 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -256,8 +256,14 @@ static int ci_ehci_hub_control(
>  	struct device *dev = hcd->self.controller;
>  	struct ci_hdrc *ci = dev_get_drvdata(dev);
>  
> -	port_index = wIndex & 0xff;
> -	port_index -= (port_index > 0);
> +	/*
> +	 * Avoid out-of-bounds values while calculating the port index
> +	 * from wIndex. The compiler doesn't like pointers to invalid
> +	 * addresses, even if they are never used.

The compiler does not care so what does care?  Why is this needed if it
is never accessed?  This comment is odd to me.

thanks,

greg k-h


> +	 */
> +	port_index = (wIndex - 1) & 0xff;
> +	if (port_index >= HCS_N_PORTS_MAX)
> +		port_index = 0;
>  	status_reg = &ehci->regs->port_status[port_index];

So this is used?  But what controls wIndex here and how can it be too
big?

thanks,

greg k-h