| Message ID | 20171120183339.15120-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted, archived |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | [U-Boot,v2,1/1] dm: usb: ehci: avoid possible NULL dereference | expand |
On 11/20/2017 07:33 PM, Heinrich Schuchardt wrote: > Currently we check in ehci_shutdown() if ctrl is NULL after > dereferencing it. > > Before this we have already dereferenced ctrl, ctrl->hccr, > and ctrl->hcor in ehci_get_portsc_register(), ehci_submit_root(), > and hci_common_init(). > > A better approach is to already check ctrl, ctrl->hccr, and ctrl->hcor > during the initialization in ehci_register() and usb_lowlevel_init() > and signal an error here via the return code. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > v2 > move the check to the initialization functions Applied, thanks. > --- > drivers/usb/host/ehci-hcd.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c > index be3e842dcc..9437f7ecea 100644 > --- a/drivers/usb/host/ehci-hcd.c > +++ b/drivers/usb/host/ehci-hcd.c > @@ -210,9 +210,6 @@ static int ehci_shutdown(struct ehci_ctrl *ctrl) > uint32_t cmd, reg; > int max_ports = HCS_N_PORTS(ehci_readl(&ctrl->hccr->cr_hcsparams)); > > - if (!ctrl || !ctrl->hcor) > - return -EINVAL; > - > cmd = ehci_readl(&ctrl->hcor->or_usbcmd); > /* If not run, directly return */ > if (!(cmd & CMD_RUN)) > @@ -1112,6 +1109,8 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) > rc = ehci_hcd_init(index, init, &ctrl->hccr, &ctrl->hcor); > if (rc) > return rc; > + if (!ctrl->hccr || !ctrl->hcor) > + return -1; > if (init == USB_INIT_DEVICE) > goto done; > > @@ -1613,10 +1612,13 @@ int ehci_register(struct udevice *dev, struct ehci_hccr *hccr, > { > struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > struct ehci_ctrl *ctrl = dev_get_priv(dev); > - int ret; > + int ret = -1; > > debug("%s: dev='%s', ctrl=%p, hccr=%p, hcor=%p, init=%d\n", __func__, > dev->name, ctrl, hccr, hcor, init); > + > + if (!ctrl || !hccr || !hcor) > + goto err; > > priv->desc_before_addr = true; > >
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index be3e842dcc..9437f7ecea 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -210,9 +210,6 @@ static int ehci_shutdown(struct ehci_ctrl *ctrl) uint32_t cmd, reg; int max_ports = HCS_N_PORTS(ehci_readl(&ctrl->hccr->cr_hcsparams)); - if (!ctrl || !ctrl->hcor) - return -EINVAL; - cmd = ehci_readl(&ctrl->hcor->or_usbcmd); /* If not run, directly return */ if (!(cmd & CMD_RUN)) @@ -1112,6 +1109,8 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) rc = ehci_hcd_init(index, init, &ctrl->hccr, &ctrl->hcor); if (rc) return rc; + if (!ctrl->hccr || !ctrl->hcor) + return -1; if (init == USB_INIT_DEVICE) goto done; @@ -1613,10 +1612,13 @@ int ehci_register(struct udevice *dev, struct ehci_hccr *hccr, { struct usb_bus_priv *priv = dev_get_uclass_priv(dev); struct ehci_ctrl *ctrl = dev_get_priv(dev); - int ret; + int ret = -1; debug("%s: dev='%s', ctrl=%p, hccr=%p, hcor=%p, init=%d\n", __func__, dev->name, ctrl, hccr, hcor, init); + + if (!ctrl || !hccr || !hcor) + goto err; priv->desc_before_addr = true;
Currently we check in ehci_shutdown() if ctrl is NULL after dereferencing it. Before this we have already dereferenced ctrl, ctrl->hccr, and ctrl->hcor in ehci_get_portsc_register(), ehci_submit_root(), and hci_common_init(). A better approach is to already check ctrl, ctrl->hccr, and ctrl->hcor during the initialization in ehci_register() and usb_lowlevel_init() and signal an error here via the return code. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- v2 move the check to the initialization functions --- drivers/usb/host/ehci-hcd.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)