Re: [PATCH v4 6/6] dma-iommu: Pass iova len for IOVA domain init

[Robin Murphy <robin.murphy@arm.com>]

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 <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> 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/kbuild-all@lists.01.org
>>
>> .
>>
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu