> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Wednesday, December 9, 2020 8:00 PM
> To: Song Bao Hua (Barry Song) <[email protected]>
> Cc: [email protected]
> Subject: [bug report] dma-mapping: add benchmark support for streaming DMA
> APIs
>
> 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).
Guess it should be less than 64?
For 64, it would be ~0ULL, otherwise, it will be 1ULL<<n-1
In test app,
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=7679325702
I have some code like:
+ /* suppose the mininum DMA zone is 1MB in the world */
+ if (bits < 20 || bits > 64) {
+ fprintf(stderr, "invalid dma mask bit, must be in 20-64\n");
+ exit(1);
+ }
Maybe I should do the same thing in kernel as well.
>
> 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
Thanks
Barry
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
