| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231201121622.16343-5-pstanner@redhat.com>
Date: Fri, 1 Dec 2023 13:16:22 +0100
From: Philipp Stanner <pstanner@...hat.com>
To: Bjorn Helgaas <bhelgaas@...gle.com>, Arnd Bergmann <arnd@...db.de>,
Andrew Morton <akpm@...ux-foundation.org>,
Dan Williams <dan.j.williams@...el.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Jakub Kicinski <kuba@...nel.org>,
Dave Jiang <dave.jiang@...el.com>,
Uladzislau Koshchanka <koshchanka@...il.com>,
NeilBrown <neilb@...e.de>,
Niklas Schnelle <schnelle@...ux.ibm.com>,
John Sanpe <sanpeqf@...il.com>,
Kent Overstreet <kent.overstreet@...il.com>,
Philipp Stanner <pstanner@...hat.com>,
"Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
Kees Cook <keescook@...omium.org>,
David Gow <davidgow@...gle.com>,
Yury Norov <yury.norov@...il.com>,
"wuqiang.matt" <wuqiang.matt@...edance.com>,
Jason Baron <jbaron@...mai.com>,
Kefeng Wang <wangkefeng.wang@...wei.com>,
Ben Dooks <ben.dooks@...ethink.co.uk>, dakr@...hat.com
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-arch@...r.kernel.org, Arnd Bergmann <arnd@...nel.org>
Subject: [PATCH v2 4/4] lib, pci: unify generic pci_iounmap()
The implementation of pci_iounmap() is currently scattered over two
files, drivers/pci/iounmap.c and lib/iomap.c. Additionally,
architectures can define their own version.
Besides one unified version being desirable in the first place, the old
version in drivers/pci/iounmap.c contained a bug and could leak memory
mappings. The bug was that #ifdef ARCH_HAS_GENERIC_IOPORT_MAP should not
have guarded iounmap(p); in addition to the preceding code.
To have only one version, it's necessary to create a helper function,
iomem_is_ioport(), that tells pci_iounmap() whether the passed address
points to an ioport or normal memory.
iomem_is_ioport() can be provided through three different ways:
1. The architecture itself provides it.
2. As a default version in include/asm-generic/io.h for those
architectures that don't use CONFIG_GENERIC_IOMAP, but also don't
provide their own version of iomem_is_ioport().
3. As a default version in lib/iomap.c for those architectures that
define and use CONFIG_GENERIC_IOMAP (currently, only x86 really
uses the functions in lib/iomap.c)
Create a unified version of pci_iounmap() in drivers/pci/iomap.c.
Provide the function iomem_is_ioport() in include/asm-generic/io.h and
lib/iomap.c.
Remove the CONFIG_GENERIC_IOMAP guard around
ARCH_WANTS_GENERIC_PCI_IOUNMAP so that configs that set
CONFIG_GENERIC_PCI_IOMAP without CONFIG_GENERIC_IOMAP still get the
function.
Fixes: 316e8d79a095 ("pci_iounmap'2: Electric Boogaloo: try to make sense of it all")
Suggested-by: Arnd Bergmann <arnd@...nel.org>
Signed-off-by: Philipp Stanner <pstanner@...hat.com>
---
drivers/pci/iomap.c | 40 +++++++++++-----------------------------
include/asm-generic/io.h | 37 ++++++++++++++++++++++++++++++++++---
lib/iomap.c | 16 +++++++++-------
3 files changed, 54 insertions(+), 39 deletions(-)
diff --git a/drivers/pci/iomap.c b/drivers/pci/iomap.c
index 0a9d503ba533..cb7f86371b7d 100644
--- a/drivers/pci/iomap.c
+++ b/drivers/pci/iomap.c
@@ -135,42 +135,24 @@ void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
EXPORT_SYMBOL_GPL(pci_iomap_wc);
/*
- * pci_iounmap() somewhat illogically comes from lib/iomap.c for the
- * CONFIG_GENERIC_IOMAP case, because that's the code that knows about
- * the different IOMAP ranges.
+ * Generic version of pci_iounmap() that is used if the architecture does not
+ * provide its own version.
*
- * But if the architecture does not use the generic iomap code, and if
- * it has _not_ defined it's own private pci_iounmap function, we define
- * it here.
- *
- * NOTE! This default implementation assumes that if the architecture
- * support ioport mapping (HAS_IOPORT_MAP), the ioport mapping will
- * be fixed to the range [ PCI_IOBASE, PCI_IOBASE+IO_SPACE_LIMIT [,
- * and does not need unmapping with 'ioport_unmap()'.
- *
- * If you have different rules for your architecture, you need to
- * implement your own pci_iounmap() that knows the rules for where
- * and how IO vs MEM get mapped.
- *
- * This code is odd, and the ARCH_HAS/ARCH_WANTS #define logic comes
- * from legacy <asm-generic/io.h> header file behavior. In particular,
- * it would seem to make sense to do the iounmap(p) for the non-IO-space
- * case here regardless, but that's not what the old header file code
- * did. Probably incorrectly, but this is meant to be bug-for-bug
- * compatible.
+ * If you have special rules for your architecture, you need to implement your
+ * own pci_iounmap() in ARCH/asm/io.h that knows the rules for where and how IO
+ * vs MEM get mapped.
*/
#if defined(ARCH_WANTS_GENERIC_PCI_IOUNMAP)
-void pci_iounmap(struct pci_dev *dev, void __iomem *p)
+void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
{
-#ifdef ARCH_HAS_GENERIC_IOPORT_MAP
- uintptr_t start = (uintptr_t) PCI_IOBASE;
- uintptr_t addr = (uintptr_t) p;
-
- if (addr >= start && addr < start + IO_SPACE_LIMIT)
+#ifdef CONFIG_HAS_IOPORT
+ if (iomem_is_ioport(addr)) {
+ ioport_unmap(addr);
return;
- iounmap(p);
+ }
#endif
+ iounmap(addr);
}
EXPORT_SYMBOL(pci_iounmap);
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index bac63e874c7b..4177d6b97e0b 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -1129,11 +1129,42 @@ extern void ioport_unmap(void __iomem *p);
#endif /* CONFIG_GENERIC_IOMAP */
#endif /* CONFIG_HAS_IOPORT_MAP */
-#ifndef CONFIG_GENERIC_IOMAP
#ifndef pci_iounmap
+#define pci_iounmap pci_iounmap
#define ARCH_WANTS_GENERIC_PCI_IOUNMAP
-#endif
-#endif
+#endif /* pci_iounmap */
+
+
+/*
+ * This function is a helper only needed for the generic pci_iounmap().
+ * It's provided here if the architecture does not select GENERIC_IOMAP and does
+ * not provide its own version.
+ */
+#ifdef CONFIG_HAS_IOPORT
+#ifndef iomem_is_ioport /* i.e., if the architecture hasn't defined its own. */
+#define iomem_is_ioport iomem_is_ioport
+
+#ifndef CONFIG_GENERIC_IOMAP
+static inline bool iomem_is_ioport(void __iomem *addr)
+{
+ unsigned long port = (unsigned long __force)addr;
+
+ // TODO: do we have to take IO_SPACE_LIMIT and PCI_IOBASE into account
+ // similar as in ioport_map() ?
+
+ if (port > MMIO_UPPER_LIMIT)
+ return false;
+
+ return true;
+}
+#else /* CONFIG_GENERIC_IOMAP. Version from lib/iomap.c will be used. */
+bool iomem_is_ioport(void __iomem *addr);
+#define ARCH_WANTS_GENERIC_IOMEM_IS_IOPORT
+#endif /* CONFIG_GENERIC_IOMAP */
+
+#endif /* iomem_is_ioport */
+#endif /* CONFIG_HAS_IOPORT */
+
#ifndef xlate_dev_mem_ptr
#define xlate_dev_mem_ptr xlate_dev_mem_ptr
diff --git a/lib/iomap.c b/lib/iomap.c
index 4f8b31baa575..adaf6246f892 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -418,12 +418,14 @@ EXPORT_SYMBOL(ioport_map);
EXPORT_SYMBOL(ioport_unmap);
#endif /* CONFIG_HAS_IOPORT_MAP */
-#ifdef CONFIG_PCI
-/* Hide the details if this is a MMIO or PIO address space and just do what
- * you expect in the correct way. */
-void pci_iounmap(struct pci_dev *dev, void __iomem * addr)
+#if defined(ARCH_WANTS_GENERIC_IOMEM_IS_IOPORT)
+bool iomem_is_ioport(void __iomem *addr)
{
- IO_COND(addr, /* nothing */, iounmap(addr));
+ unsigned long port = (unsigned long __force)addr;
+
+ if (port > PIO_OFFSET && port < PIO_RESERVED)
+ return true;
+
+ return false;
}
-EXPORT_SYMBOL(pci_iounmap);
-#endif /* CONFIG_PCI */
+#endif /* ARCH_WANTS_GENERIC_IOMEM_IS_IOPORT */
--
2.43.0
Powered by blists - more mailing lists