| Message ID | 20241129113318.296073-1-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb: chipidea: host: Improve port index sanitizing | expand |
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
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. + */ + port_index = (wIndex - 1) & 0xff; + if (port_index >= HCS_N_PORTS_MAX) + port_index = 0; status_reg = &ehci->regs->port_status[port_index]; spin_lock_irqsave(&ehci->lock, flags);
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(-)