| Message ID | 20240131090023.12331-2-pstanner@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Regather scattered PCI-Code | expand |
On Wed, Jan 31, 2024 at 10:00:20AM +0100, Philipp Stanner wrote: > The #ifdef for the ioport-ranges accidentally also guards iounmap(), > potentially compiling an empty function. This would cause the mapping to > be leaked. > > Move the guard so that iounmap() will always be part of the function. I tweaked the subject and commit log to be more explicit about what the bug is. Let me know if I got it wrong: pci_iounmap(): Fix MMIO mapping leak The #ifdef ARCH_HAS_GENERIC_IOPORT_MAP accidentally also guards iounmap(), which means MMIO mappings are leaked. Move the guard so we call iounmap() for MMIO mappings. Bjorn
On Wed, 2024-01-31 at 15:09 -0600, Bjorn Helgaas wrote: > On Wed, Jan 31, 2024 at 10:00:20AM +0100, Philipp Stanner wrote: > > The #ifdef for the ioport-ranges accidentally also guards > > iounmap(), > > potentially compiling an empty function. This would cause the > > mapping to > > be leaked. > > > > Move the guard so that iounmap() will always be part of the > > function. > > I tweaked the subject and commit log to be more explicit about what > the bug is. Let me know if I got it wrong: Mostly correct IMO > > pci_iounmap(): Fix MMIO mapping leak > > The #ifdef ARCH_HAS_GENERIC_IOPORT_MAP accidentally also guards > iounmap(), > which means MMIO mappings are leaked. nit: I wasn't entirely sure when they are actually leaked, just that they _could_ be leaked. To know for sure we'd need to search who sets ARCH_WANTS_GENERIC_PCI_IOUNMAP without setting ARCH_HAS_GENERIC_IOPORT_MAP. I think your formulation should be fine, though, since it's definitely a bug. P. > > Move the guard so we call iounmap() for MMIO mappings. > > Bjorn >
diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c index ce39ce9f3526..2829ddb0e316 100644 --- a/lib/pci_iomap.c +++ b/lib/pci_iomap.c @@ -170,8 +170,8 @@ void pci_iounmap(struct pci_dev *dev, void __iomem *p) if (addr >= start && addr < start + IO_SPACE_LIMIT) return; - iounmap(p); #endif + iounmap(p); } EXPORT_SYMBOL(pci_iounmap);
The #ifdef for the ioport-ranges accidentally also guards iounmap(), potentially compiling an empty function. This would cause the mapping to be leaked. Move the guard so that iounmap() will always be part of the function. CC: <stable@vger.kernel.org> # v5.15+ Fixes: 316e8d79a095 ("pci_iounmap'2: Electric Boogaloo: try to make sense of it all") Reported-by: Danilo Krummrich <dakr@redhat.com> Suggested-by: Arnd Bergmann <arnd@kernel.org> Signed-off-by: Philipp Stanner <pstanner@redhat.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de> --- lib/pci_iomap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)