[bug report] dma-mapping: add benchmark support for streaming DMA APIs

From: Dan Carpenter <dan.carpenter@oracle.com>
To: song.bao.hua@hisilicon.com
Cc: iommu@lists.linux-foundation.org
Subject: [bug report] dma-mapping: add benchmark support for streaming DMA APIs
Date: Wed, 9 Dec 2020 10:00:23 +0300	[thread overview]
Message-ID: <X9B2B4zgzFwpqVcM@mwanda> (raw)

Hello Barry Song,

The patch 65789daa8087: "dma-mapping: add benchmark support for
streaming DMA APIs" from Nov 16, 2020, leads to the following static
checker warning:

	kernel/dma/map_benchmark.c:241 map_benchmark_ioctl()
	error: undefined (user controlled) shift '1 << (map->bparam.dma_bits)'

kernel/dma/map_benchmark.c
   191  static long map_benchmark_ioctl(struct file *file, unsigned int cmd,
   192                  unsigned long arg)
   193  {
   194          struct map_benchmark_data *map = file->private_data;
   195          void __user *argp = (void __user *)arg;
   196          u64 old_dma_mask;
   197  
   198          int ret;
   199  
   200          if (copy_from_user(&map->bparam, argp, sizeof(map->bparam)))
                                   ^^^^^^^^^^^^^
Comes from the user

   201                  return -EFAULT;
   202  
   203          switch (cmd) {
   204          case DMA_MAP_BENCHMARK:
   205                  if (map->bparam.threads == 0 ||
   206                      map->bparam.threads > DMA_MAP_MAX_THREADS) {
   207                          pr_err("invalid thread number\n");
   208                          return -EINVAL;
   209                  }
   210  
   211                  if (map->bparam.seconds == 0 ||
   212                      map->bparam.seconds > DMA_MAP_MAX_SECONDS) {
   213                          pr_err("invalid duration seconds\n");
   214                          return -EINVAL;
   215                  }
   216  
   217                  if (map->bparam.node != NUMA_NO_NODE &&
   218                      !node_possible(map->bparam.node)) {
   219                          pr_err("invalid numa node\n");
   220                          return -EINVAL;
   221                  }
   222  
   223                  switch (map->bparam.dma_dir) {
   224                  case DMA_MAP_BIDIRECTIONAL:
   225                          map->dir = DMA_BIDIRECTIONAL;
   226                          break;
   227                  case DMA_MAP_FROM_DEVICE:
   228                          map->dir = DMA_FROM_DEVICE;
   229                          break;
   230                  case DMA_MAP_TO_DEVICE:
   231                          map->dir = DMA_TO_DEVICE;
   232                          break;
   233                  default:
   234                          pr_err("invalid DMA direction\n");
   235                          return -EINVAL;
   236                  }
   237  
   238                  old_dma_mask = dma_get_mask(map->dev);
   239  
   240                  ret = dma_set_mask(map->dev,
   241                                     DMA_BIT_MASK(map->bparam.dma_bits));
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
If this is more than 31 then the behavior is undefined (but in real life
it will shift wrap).

   242                  if (ret) {
   243                          pr_err("failed to set dma_mask on device %s\n",
   244                                  dev_name(map->dev));
   245                          return -EINVAL;
   246                  }
   247  
   248                  ret = do_map_benchmark(map);
   249  
   250                  /*
   251                   * restore the original dma_mask as many devices' dma_mask are
   252                   * set by architectures, acpi, busses. When we bind them back
   253                   * to their original drivers, those drivers shouldn't see
   254                   * dma_mask changed by benchmark
   255                   */
   256                  dma_set_mask(map->dev, old_dma_mask);
   257                  break;
   258          default:
   259                  return -EINVAL;
   260          }
   261  
   262          if (copy_to_user(argp, &map->bparam, sizeof(map->bparam)))
   263                  return -EFAULT;
   264  
   265          return ret;
   266  }

regards,
dan carpenter
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu