Re: [bug report] iommu/amd: Use is_attach_deferred call-back

[Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

Hi Dan,

On 08/24/17 at 02:04pm, Dan Carpenter wrote:
> Hello Baoquan He,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch df3f7a6e8e85: "iommu/amd: Use is_attach_deferred call-back" 
> from Aug 9, 2017, leads to the following Smatch complaint:
> 
> drivers/iommu/amd_iommu.c:2265 get_domain()
> 	 error: we previously assumed 'domain' could be null (see line 2259)
> 
> drivers/iommu/amd_iommu.c
>   2258		domain = get_dev_data(dev)->domain;
>   2259		if (domain == NULL && get_dev_data(dev)->defer_attach) {
>                     ^^^^^^^^^^^^^^
> The patch adds a new check for NULL.
> 
>   2260			get_dev_data(dev)->defer_attach = false;
>   2261			io_domain = iommu_get_domain_for_dev(dev);
>   2262			domain = to_pdomain(io_domain);
>   2263			attach_device(dev, domain);
>   2264		}
>   2265		if (!dma_ops_domain(domain))

Thanks for pointing it out, it's truly a code bug. We should check if
'domain' is NULL when pass it to dma_ops_domain() to dereference.

I would like to fix it with below code change, and will post a patch
soon.

-       if (!dma_ops_domain(domain))
+       if (domain && !dma_ops_domain(domain))
                return ERR_PTR(-EBUSY);

Thanks
Baoquan

>                                     ^^^^^^
> Existing unchecked dereference inside the function.
> 
>   2266			return ERR_PTR(-EBUSY);
>   2267	
> 
> regards,
> dan carpenter