* re: iomap: implement pcim_iounmap_regions()
@ 2015-09-16 16:00 Dan Carpenter
2015-09-16 17:10 ` Tejun Heo
2015-09-16 18:14 ` iomap: implement pcim_iounmap_regions() Dan Carpenter
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2015-09-16 16:00 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: iomap: implement pcim_iounmap_regions()
2015-09-16 16:00 iomap: implement pcim_iounmap_regions() Dan Carpenter
@ 2015-09-16 17:10 ` Tejun Heo
2015-09-21 16:21 ` [patch] devres: fix a for loop bounds check Dan Carpenter
2015-09-16 18:14 ` iomap: implement pcim_iounmap_regions() Dan Carpenter
1 sibling, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2015-09-16 17:10 UTC (permalink / raw)
To: kernel-janitors
Hello,
On Wed, Sep 16, 2015 at 07:00:24PM +0300, Dan Carpenter wrote:
> 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...
Looks like a silly mistake on my part. Care to submit a patch?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 5+ messages in thread* [patch] devres: fix a for loop bounds check
2015-09-16 17:10 ` Tejun Heo
@ 2015-09-21 16:21 ` Dan Carpenter
2015-09-21 17:40 ` Tejun Heo
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2015-09-21 16:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, htejun
Cc: Catalin Marinas, Cristian Stoica, Dan Williams, Abhilash Kesavan,
linux-kernel, kernel-janitors
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.
Fixes: ec04b075843d ('iomap: implement pcim_iounmap_regions()')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/lib/devres.c b/lib/devres.c
index f13a246..8c85672 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -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;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [patch] devres: fix a for loop bounds check
2015-09-21 16:21 ` [patch] devres: fix a for loop bounds check Dan Carpenter
@ 2015-09-21 17:40 ` Tejun Heo
0 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2015-09-21 17:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, Catalin Marinas, Cristian Stoica,
Dan Williams, Abhilash Kesavan, linux-kernel, kernel-janitors
On Mon, Sep 21, 2015 at 07:21:51PM +0300, Dan Carpenter wrote:
> 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.
>
> Fixes: ec04b075843d ('iomap: implement pcim_iounmap_regions()')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: iomap: implement pcim_iounmap_regions()
2015-09-16 16:00 iomap: implement pcim_iounmap_regions() Dan Carpenter
2015-09-16 17:10 ` Tejun Heo
@ 2015-09-16 18:14 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2015-09-16 18:14 UTC (permalink / raw)
To: kernel-janitors
On Wed, Sep 16, 2015 at 01:10:00PM -0400, Tejun Heo wrote:
> Hello,
>
> On Wed, Sep 16, 2015 at 07:00:24PM +0300, Dan Carpenter wrote:
> > 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...
>
> Looks like a silly mistake on my part. Care to submit a patch?
>
Sure, I will do that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-21 17:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-16 16:00 iomap: implement pcim_iounmap_regions() Dan Carpenter
2015-09-16 17:10 ` Tejun Heo
2015-09-21 16:21 ` [patch] devres: fix a for loop bounds check Dan Carpenter
2015-09-21 17:40 ` Tejun Heo
2015-09-16 18:14 ` iomap: implement pcim_iounmap_regions() Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox