Hello Michael Zoran,
The patch 4807f2c0e684: "staging: vc04_services: add
vchiq_pagelist_info structure" from Nov 7, 2016, leads to the
following static checker warning:
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:440 create_pagelist()
warn: overflowed symbol reused: 'count'
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
398 static struct vchiq_pagelist_info *
399 create_pagelist(char __user *buf, size_t count, unsigned short type)
^^^^^^^^^^^^
This comes from the user from the ioctl. It starts as an int but gets
cast to unsigned long.
400 {
401 PAGELIST_T *pagelist;
402 struct vchiq_pagelist_info *pagelistinfo;
403 struct page **pages;
404 u32 *addrs;
405 unsigned int num_pages, offset, i, k;
406 int actual_pages;
407 size_t pagelist_size;
408 struct scatterlist *scatterlist, *sg;
409 int dma_buffers;
410 dma_addr_t dma_addr;
411
412 offset = ((unsigned int)(unsigned long)buf & (PAGE_SIZE - 1));
413 num_pages = DIV_ROUND_UP(count + offset, PAGE_SIZE);
^^^^^^^^^^^^^^^^^^^^^^^^^
Which means that this can definitely overflow. I can happen in two
ways. First the "count + offset" can overflow. Then the DIV_ROUND_UP
starts by adding PAGE_SIZE to the result and that addition can overflow.
The problem with handling integer overflow issues from a static analysis
perspective is that many integer overflows are not harmful. Often we
maybe we get a smaller num_pages from what we had intended but it doesn't
cause a problem because we never reference "count" again. This warning
message is supposed to solve that problem by warning by only complaining
when we re-use count.
414
415 pagelist_size = sizeof(PAGELIST_T) +
416 (num_pages * sizeof(u32)) +
417 (num_pages * sizeof(pages[0]) +
418 (num_pages * sizeof(struct scatterlist))) +
419 sizeof(struct vchiq_pagelist_info);
420
421 /* Allocate enough storage to hold the page pointers and the page
422 * list
423 */
424 pagelist = dma_zalloc_coherent(g_dev,
425 pagelist_size,
426 &dma_addr,
427 GFP_KERNEL);
428
429 vchiq_log_trace(vchiq_arm_log_level, "%s - %pK", __func__, pagelist);
430
431 if (!pagelist)
432 return NULL;
433
434 addrs = pagelist->addrs;
435 pages = (struct page **)(addrs + num_pages);
436 scatterlist = (struct scatterlist *)(pages + num_pages);
437 pagelistinfo = (struct vchiq_pagelist_info *)
438 (scatterlist + num_pages);
439
440 pagelist->length = count;
^^^^^^^^^^^^^^^^^^^^^^^^^
So it complains here. But then when I look at it more, it's not clear
to me if it matters that "pagelist->length" is out of sync with
"num_pages"...
Anyway, that's why I'm forwarding this static checker output to you
instead of trying to fix it myself.
441 pagelist->type = type;
442 pagelist->offset = offset;
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
