[U-Boot] [PATCH v2 1/1] dm: usb: ehci: avoid possible NULL dereference

[Marek Vasut]

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


-- 
Best regards,
Marek Vasut