On Sat, 2025-03-08 at 15:07 -0600, Bjorn Helgaas wrote:
> On Sat, Mar 08, 2025 at 01:23:28PM +0300, Dan Carpenter wrote:
> > Hello Philipp Stanner,
> >
> > Commit ba10e5011d05 ("PCI: Check BAR index for validity") from Mar
> > 4,
> > 2025 (linux-next), leads to the following Smatch static checker
> > warning:
> >
> > drivers/pci/devres.c:632
> > pcim_remove_bar_from_legacy_table()
> > error: buffer overflow 'legacy_iomap_table' 6 <= 15
>
> Thanks, I dropped this patch for now.
>
> > drivers/pci/devres.c
> > 621 static void pcim_remove_bar_from_legacy_table(struct
> > pci_dev *pdev, int bar)
> > 622 {
> > 623 void __iomem **legacy_iomap_table;
> > 624
> > 625 if (!pci_bar_index_is_valid(bar))
> >
> > This line used to check PCI_STD_NUM_BARS (6) but now it's checking
> > PCI_NUM_RESOURCES (15).
What is even going on here. Why are thos different values? Does a PCI
device now have at most 6, or 15 BARs?
Or is a BAR different from a "resource"?
And why would it be 15? I haven't read the standard, but I would
suspect it should be 16.
And which of those two here should be used?
https://elixir.bootlin.com/linux/v6.14-rc4/source/include/linux/pci.h#L133
The comment doesn't say *which one* is "preserved for backwards
compatibility".
So many questions…
But granted, the check is wrong for the devres resource array, and I
suppose it should be made the same size as pci_dev.resource.
> >
> > 626 return;
> > 627
> > 628 legacy_iomap_table = (void __iomem
> > **)pcim_iomap_table(pdev);
> > 629 if (!legacy_iomap_table)
> > 630 return;
> > 631
> > --> 632 legacy_iomap_table[bar] = NULL;
> > ^^^^^^^^^^^^^^^^^^^^^^^
> > Leading to a buffer overflow.
Leading to a *potential* buffer overflow.
Anyways, thanks for reporting.
P.
> >
> > 633 }
> >
> > regards,
> > dan carpenter
