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

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

[Dan Carpenter]

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))
                                    ^^^^^^
Existing unchecked dereference inside the function.

  2266			return ERR_PTR(-EBUSY);
  2267	

regards,
dan carpenter

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

[Baoquan He]

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