On 2021-07-19 10:12, John Garry wrote:
On 19/07/2021 08:58, Dan Carpenter wrote:
Hi John,

url: https://github.com/0day-ci/linux/commits/John-Garry/iommu-Allow-IOVA-rcache-range-be-configured/20210714-184328 base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-randconfig-m031-20210714 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>

smatch warnings:
drivers/iommu/dma-iommu.c:384 iommu_dma_init_domain() warn: variable dereferenced before check 'dev' (see line 374)


thanks for the notice

vim +/dev +384 drivers/iommu/dma-iommu.c

06d60728ff5c01 Christoph Hellwig     2019-05-20  332  static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, ac6d704679d343 Jean-Philippe Brucker 2021-06-18  333 dma_addr_t limit, struct device *dev)
0db2e5d18f76a6 Robin Murphy          2015-10-01  334  {
fdbe574eb69312 Robin Murphy          2017-01-19  335      struct iommu_dma_cookie *cookie = domain->iova_cookie; c61a4633a56aaa Shaokun Zhang         2019-01-24  336      unsigned long order, base_pfn; 6b0c54e7f27159 Yunsheng Lin          2019-08-24  337      struct iova_domain *iovad; de4ba360c3e4ed John Garry            2021-07-14  338      size_t max_opt_dma_size; de4ba360c3e4ed John Garry            2021-07-14  339      unsigned long iova_len = 0;
0db2e5d18f76a6 Robin Murphy          2015-10-01  340
fdbe574eb69312 Robin Murphy          2017-01-19  341      if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE) fdbe574eb69312 Robin Murphy          2017-01-19  342          return -EINVAL;
0db2e5d18f76a6 Robin Murphy          2015-10-01  343
6b0c54e7f27159 Yunsheng Lin          2019-08-24  344      iovad = &cookie->iovad;
6b0c54e7f27159 Yunsheng Lin          2019-08-24  345
0db2e5d18f76a6 Robin Murphy          2015-10-01  346      /* Use the smallest supported page size for IOVA granularity */ d16e0faab911cc Robin Murphy          2016-04-07  347      order = __ffs(domain->pgsize_bitmap); 0db2e5d18f76a6 Robin Murphy          2015-10-01  348      base_pfn = max_t(unsigned long, 1, base >> order);
0db2e5d18f76a6 Robin Murphy          2015-10-01  349
0db2e5d18f76a6 Robin Murphy          2015-10-01  350      /* Check the domain allows at least some access to the device... */ 0db2e5d18f76a6 Robin Murphy          2015-10-01  351      if (domain->geometry.force_aperture) { 0db2e5d18f76a6 Robin Murphy          2015-10-01  352          if (base > domain->geometry.aperture_end || ac6d704679d343 Jean-Philippe Brucker 2021-06-18  353 limit < domain->geometry.aperture_start) { 0db2e5d18f76a6 Robin Murphy          2015-10-01  354 pr_warn("specified DMA range outside IOMMU capability\n"); 0db2e5d18f76a6 Robin Murphy          2015-10-01  355 return -EFAULT;
0db2e5d18f76a6 Robin Murphy          2015-10-01  356          }
0db2e5d18f76a6 Robin Murphy          2015-10-01  357          /* ...then finally give it a kicking to make sure it fits */ 0db2e5d18f76a6 Robin Murphy          2015-10-01  358          base_pfn = max_t(unsigned long, base_pfn, 0db2e5d18f76a6 Robin Murphy          2015-10-01  359 domain->geometry.aperture_start >> order);
0db2e5d18f76a6 Robin Murphy          2015-10-01  360      }
0db2e5d18f76a6 Robin Murphy          2015-10-01  361
f51d7bb79c1124 Robin Murphy          2017-01-16  362      /* start_pfn is always nonzero for an already-initialised domain */ 0db2e5d18f76a6 Robin Murphy          2015-10-01  363      if (iovad->start_pfn) { 0db2e5d18f76a6 Robin Murphy          2015-10-01  364          if (1UL << order != iovad->granule || f51d7bb79c1124 Robin Murphy          2017-01-16  365 base_pfn != iovad->start_pfn) { 0db2e5d18f76a6 Robin Murphy          2015-10-01  366 pr_warn("Incompatible range for DMA domain\n"); 0db2e5d18f76a6 Robin Murphy          2015-10-01  367 return -EFAULT;
0db2e5d18f76a6 Robin Murphy          2015-10-01  368          }
7c1b058c8b5a31 Robin Murphy          2017-03-16  369
7c1b058c8b5a31 Robin Murphy          2017-03-16  370          return 0;
0db2e5d18f76a6 Robin Murphy          2015-10-01  371      }
7c1b058c8b5a31 Robin Murphy          2017-03-16  372
de4ba360c3e4ed John Garry            2021-07-14  373
de4ba360c3e4ed John Garry            2021-07-14 @374 max_opt_dma_size = iommu_group_get_max_opt_dma_size(dev->iommu_group); ^^^^^^^^^^^^^^^^
New unchecked dereference

de4ba360c3e4ed John Garry            2021-07-14  375      if (max_opt_dma_size) { de4ba360c3e4ed John Garry            2021-07-14  376          unsigned long shift = __ffs(1UL << order);
de4ba360c3e4ed John Garry            2021-07-14  377
de4ba360c3e4ed John Garry            2021-07-14  378          iova_len = max_opt_dma_size >> shift; de4ba360c3e4ed John Garry            2021-07-14  379          iova_len = roundup_pow_of_two(iova_len);
de4ba360c3e4ed John Garry            2021-07-14  380      }
de4ba360c3e4ed John Garry            2021-07-14  381
de4ba360c3e4ed John Garry            2021-07-14  382 init_iova_domain(iovad, 1UL << order, base_pfn, iova_len);
2da274cdf998a1 Zhen Lei              2018-09-20  383
82c3cefb9f1652 Lu Baolu              2021-02-25 @384      if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


a250c23f15c21c Robin Murphy          2021-04-01  385 domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) { b34e9b0de3c411 Tom Murphy            2020-09-10  386          if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all, 2a2b8eaa5b2566 Tom Murphy            2020-11-24 387                        iommu_dma_entry_dtor)) b34e9b0de3c411 Tom Murphy            2020-09-10  388 pr_warn("iova flush queue initialization failed\n");
b34e9b0de3c411 Tom Murphy            2020-09-10  389          else
2da274cdf998a1 Zhen Lei              2018-09-20  390 cookie->fq_domain = domain;
2da274cdf998a1 Zhen Lei              2018-09-20  391      }
2da274cdf998a1 Zhen Lei              2018-09-20  392
7c1b058c8b5a31 Robin Murphy          2017-03-16  393      if (!dev)
                                                             ^^^^
Old code has checks for NULL


I doubt that in practice we need this check.

Function iommu_dma_init_domain() is only called by iommu_setup_dma_ops(). Furthermore, iommu_setup_dma_ops() calls iommu_get_domain_for_dev(dev), which cannot safely handle dev == NULL for when we call iommu_dma_init_domain() there. As such, the dev == NULL checks in iommu_dma_init_domain() are effectively redundant.

Indeed, I have a patch for that in the stack I'm preparing:

https://gitlab.arm.com/linux-arm/linux-rm/-/commit/9b6cf2a214107c153ee278b1664f688888d7328f

Robin.

0db2e5d18f76a6 Robin Murphy          2015-10-01  394          return 0;
7c1b058c8b5a31 Robin Murphy          2017-03-16  395
7c1b058c8b5a31 Robin Murphy          2017-03-16  396      return iova_reserve_iommu_regions(dev, domain);
0db2e5d18f76a6 Robin Murphy          2015-10-01  397  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

.


_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to