iomap: implement pcim_iounmap_regions()
Subsystems (2)
Hey Tejun,
These are very old.
The patch ec04b075843d: "iomap: implement pcim_iounmap_regions()" from Mar 9, 2007, leads to the following static checker warning:
lib/devres.c:425 pcim_iounmap_regions() error: buffer overflow 'iomap' 6 <= 16
lib/devres.c
405 /**
406 * pcim_iounmap_regions - Unmap and release PCI BARs 407 * @pdev: PCI device to map IO resources for 408 * @mask: Mask of BARs to unmap and release 409 * 410 * Unmap and release regions specified by @mask. 411 */
412 void pcim_iounmap_regions(struct pci_dev *pdev, int mask) 413 {
414 void __iomem * const *iomap;
415 int i;
416
417 iomap = pcim_iomap_table(pdev);
418 if (!iomap)
419 return;
420
421 for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
422 if (!(mask & (1 << i)))
423 continue;
Probably maks is always valid is what helps us but basically the static checker is suggesting that instead of DEVICE_COUNT_RESOURCE we should be using PCI_ROM_RESOURCE or PCIM_IOMAP_MAX. It could be that this was deliberate though...
424 425 pcim_iounmap(pdev, iomap[i]); 426 pci_release_region(pdev, i); 427 } 428 }
regards,
dan carpenter
Hello,
Quoted 8 lines · Dan Carpenter
421 for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) { 422 if (!(mask & (1 << i))) 423 continue;Probably maks is always valid is what helps us but basically the static checker is suggesting that instead of
DEVICE_COUNT_RESOURCEwe should be usingPCI_ROM_RESOURCEorPCIM_IOMAP_MAX. It could be that this was deliberate though...
Looks like a silly mistake on my part. Care to submit a patch?
Thanks.
Signature · 1 line
tejun
PATCH 1/1 devres: fix a for loop bounds check
The iomap[] array has PCIM_IOMAP_MAX (6) elements and not DEVICE_COUNT_RESOURCE (16). This bug was found using a static checker. It may be that the "if (!(mask & (1 << i)))" check means we never actually go past the end of the array in real life.
lib/devres.c +1 -1
@@ -418,7 +418,7 @@ void pcim_iounmap_regions(struct pci_dev *pdev, int mask)if (!iomap)return ; -for (i =0 ; i <DEVICE_COUNT_RESOURCE ; i++) { +for (i =0 ; i <PCIM_IOMAP_MAX ; i++) {if (!(mask & (1 << i)))continue ;
Re: [patch] devres: fix a for loop bounds check
Quoted 7 lines · Dan Carpenter
The iomap[] array has
PCIM_IOMAP_MAX(6) elements and notDEVICE_COUNT_RESOURCE(16). This bug was found using a static checker. It may be that the "if (!(mask & (1 << i)))" check means we never actually go past the end of the array in real life.Fixes ec04b075843d ('iomap: implement pcim_iounmap_regions()')Signed-off-by Dan Carpenter <[email protected]>
Thanks.
Signature · 1 line
tejun
Quoted 7 lines · Tejun Heo
Hello,
Quoted 4 lines · Dan Carpenter
421 for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) { 422 if (!(mask & (1 << i))) 423 continue;
Quoted 7 lines
Quoted 4 lines
Probably maks is always valid is what helps us but basically the static checker is suggesting that instead of
DEVICE_COUNT_RESOURCEwe should be usingPCI_ROM_RESOURCEorPCIM_IOMAP_MAX. It could be that this was deliberate though...Looks like a silly mistake on my part. Care to submit a patch?
Sure, I will do that.
regards,
dan carpenter