The branch main has been updated by kib:
URL:
https://cgit.FreeBSD.org/src/commit/?id=c745a6818bcbf33cf7f59641c925d19b3f98cea8
commit c745a6818bcbf33cf7f59641c925d19b3f98cea8
Author: Konstantin Belousov <k...@freebsd.org>
AuthorDate: 2025-09-05 23:06:18 +0000
Commit: Konstantin Belousov <k...@freebsd.org>
CommitDate: 2025-09-09 02:51:08 +0000
iommu_get_requester(): make it more resilient against arbitrary dev arg
If passed the parent of a device instead of the device, the loop might
end up with the host bridge in the pci local variable. If the passed
device is not from the pci hierarchy, any of the calculated parents
might be NULL.
Change the interface to allow the caller to receive error.
Instead of asserting, just issue a message and return ENXIO,
allowing the caller to select appropriate action.
PR: 289318
Reviewed by: jah
Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks
Differential revision: https://reviews.freebsd.org/D52406
---
sys/dev/iommu/busdma_iommu.c | 54 ++++++++++++++++++++++++++++++++-----------
sys/dev/iommu/iommu.h | 2 +-
sys/kern/subr_bus.c | 13 +++++++++--
sys/x86/iommu/amd_intrmap.c | 14 ++++++++---
sys/x86/iommu/intel_intrmap.c | 8 +++++--
5 files changed, 70 insertions(+), 21 deletions(-)
diff --git a/sys/dev/iommu/busdma_iommu.c b/sys/dev/iommu/busdma_iommu.c
index 6856b0551dde..668ccf056463 100644
--- a/sys/dev/iommu/busdma_iommu.c
+++ b/sys/dev/iommu/busdma_iommu.c
@@ -114,8 +114,8 @@ iommu_bus_dma_is_dev_disabled(int domain, int bus, int
slot, int func)
* domain, and must collectively be assigned to use either IOMMU or
* bounce mapping.
*/
-device_t
-iommu_get_requester(device_t dev, uint16_t *rid)
+int
+iommu_get_requester(device_t dev, device_t *requesterp, uint16_t *rid)
{
devclass_t pci_class;
device_t l, pci, pcib, pcip, pcibp, requester;
@@ -129,7 +129,8 @@ iommu_get_requester(device_t dev, uint16_t *rid)
pci = device_get_parent(dev);
if (pci == NULL || device_get_devclass(pci) != pci_class) {
*rid = 0; /* XXXKIB: Could be ACPI HID */
- return (requester);
+ *requesterp = NULL;
+ return (ENOTTY);
}
*rid = pci_get_rid(dev);
@@ -141,16 +142,39 @@ iommu_get_requester(device_t dev, uint16_t *rid)
*/
for (;;) {
pci = device_get_parent(l);
- KASSERT(pci != NULL, ("iommu_get_requester(%s): NULL parent "
- "for %s", device_get_name(dev), device_get_name(l)));
- KASSERT(device_get_devclass(pci) == pci_class,
- ("iommu_get_requester(%s): non-pci parent %s for %s",
- device_get_name(dev), device_get_name(pci),
- device_get_name(l)));
+ if (pci == NULL) {
+ if (bootverbose) {
+ printf(
+ "iommu_get_requester(%s): NULL parent for %s\n",
+ device_get_name(dev), device_get_name(l));
+ }
+ *rid = 0;
+ *requesterp = NULL;
+ return (ENXIO);
+ }
+ if (device_get_devclass(pci) != pci_class) {
+ if (bootverbose) {
+ printf(
+ "iommu_get_requester(%s): non-pci parent %s for %s\n",
+ device_get_name(dev), device_get_name(pci),
+ device_get_name(l));
+ }
+ *rid = 0;
+ *requesterp = NULL;
+ return (ENXIO);
+ }
pcib = device_get_parent(pci);
- KASSERT(pcib != NULL, ("iommu_get_requester(%s): NULL bridge "
- "for %s", device_get_name(dev), device_get_name(pci)));
+ if (pcib == NULL) {
+ if (bootverbose) {
+ printf(
+ "iommu_get_requester(%s): NULL bridge for %s\n",
+ device_get_name(dev), device_get_name(pci));
+ }
+ *rid = 0;
+ *requesterp = NULL;
+ return (ENXIO);
+ }
/*
* The parent of our "bridge" isn't another PCI bus,
@@ -229,7 +253,8 @@ iommu_get_requester(device_t dev, uint16_t *rid)
}
}
}
- return (requester);
+ *requesterp = requester;
+ return (0);
}
struct iommu_ctx *
@@ -237,10 +262,13 @@ iommu_instantiate_ctx(struct iommu_unit *unit, device_t
dev, bool rmrr)
{
device_t requester;
struct iommu_ctx *ctx;
+ int error;
bool disabled;
uint16_t rid;
- requester = iommu_get_requester(dev, &rid);
+ error = iommu_get_requester(dev, &requester, &rid);
+ if (error != 0)
+ return (NULL);
/*
* If the user requested the IOMMU disabled for the device, we
diff --git a/sys/dev/iommu/iommu.h b/sys/dev/iommu/iommu.h
index b1858f0df9f7..55044042c5d2 100644
--- a/sys/dev/iommu/iommu.h
+++ b/sys/dev/iommu/iommu.h
@@ -170,7 +170,7 @@ void iommu_domain_unload(struct iommu_domain *domain,
void iommu_unit_pre_instantiate_ctx(struct iommu_unit *iommu);
struct iommu_ctx *iommu_instantiate_ctx(struct iommu_unit *iommu,
device_t dev, bool rmrr);
-device_t iommu_get_requester(device_t dev, uint16_t *rid);
+int iommu_get_requester(device_t dev, device_t *requester, uint16_t *rid);
int iommu_init_busdma(struct iommu_unit *unit);
void iommu_fini_busdma(struct iommu_unit *unit);
diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 62a3da964c37..bf5bda7e058d 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -280,6 +280,9 @@ device_sysctl_handler(SYSCTL_HANDLER_ARGS)
struct sbuf sb;
device_t dev = (device_t)arg1;
device_t iommu;
+#ifdef IOMMU
+ device_t requester;
+#endif
int error;
uint16_t rid;
const char *c;
@@ -314,9 +317,15 @@ device_sysctl_handler(SYSCTL_HANDLER_ARGS)
}
rid = 0;
#ifdef IOMMU
- iommu_get_requester(dev, &rid);
+ error = iommu_get_requester(dev, &requester, &rid);
+ /*
+ * Do not return requester error from sysctl, iommu
+ * unit might be assigned by other means.
+ */
+#else
+ error = ENXIO;
#endif
- if (rid != 0)
+ if (error == 0)
sbuf_printf(&sb, "%srid=%#x", c, rid);
break;
default:
diff --git a/sys/x86/iommu/amd_intrmap.c b/sys/x86/iommu/amd_intrmap.c
index a4c1a7836268..f8900fe0561f 100644
--- a/sys/x86/iommu/amd_intrmap.c
+++ b/sys/x86/iommu/amd_intrmap.c
@@ -112,6 +112,8 @@ amdiommu_map_msi_intr(device_t src, u_int cpu, u_int vector,
{
struct amdiommu_ctx *ctx;
struct amdiommu_unit *unit;
+ device_t requester;
+ int error __diagused;
uint16_t rid;
bool is_iommu;
@@ -180,7 +182,8 @@ amdiommu_map_msi_intr(device_t src, u_int cpu, u_int vector,
*addr |= ((uint64_t)cpu & 0xffffff00) << 32;
}
- iommu_get_requester(src, &rid);
+ error = iommu_get_requester(src, &requester, &rid);
+ MPASS(error == 0);
AMDIOMMU_LOCK(unit);
amdiommu_qi_invalidate_ir_locked(unit, rid);
AMDIOMMU_UNLOCK(unit);
@@ -220,6 +223,7 @@ static struct amdiommu_ctx *
amdiommu_ir_find(device_t src, uint16_t *ridp, bool *is_iommu)
{
devclass_t src_class;
+ device_t requester;
struct amdiommu_unit *unit;
struct amdiommu_ctx *ctx;
uint32_t edte;
@@ -251,7 +255,8 @@ amdiommu_ir_find(device_t src, uint16_t *ridp, bool
*is_iommu)
error = amdiommu_find_unit(src, &unit, &rid, &dte, &edte,
bootverbose);
if (error == 0) {
- iommu_get_requester(src, &rid);
+ error = iommu_get_requester(src, &requester, &rid);
+ MPASS(error == 0);
ctx = amdiommu_get_ctx_for_dev(unit, src,
rid, 0, false /* XXXKIB */, false, dte, edte);
}
@@ -266,6 +271,8 @@ amdiommu_ir_free_irte(struct amdiommu_ctx *ctx, device_t
src,
u_int cookie)
{
struct amdiommu_unit *unit;
+ device_t requester;
+ int error __diagused;
uint16_t rid;
MPASS(ctx != NULL);
@@ -291,7 +298,8 @@ amdiommu_ir_free_irte(struct amdiommu_ctx *ctx, device_t
src,
atomic_thread_fence_rel();
bzero(irte, sizeof(*irte));
}
- iommu_get_requester(src, &rid);
+ error = iommu_get_requester(src, &requester, &rid);
+ MPASS(error == 0);
AMDIOMMU_LOCK(unit);
amdiommu_qi_invalidate_ir_locked(unit, rid);
AMDIOMMU_UNLOCK(unit);
diff --git a/sys/x86/iommu/intel_intrmap.c b/sys/x86/iommu/intel_intrmap.c
index 06e41523624b..f12a0c9bae9b 100644
--- a/sys/x86/iommu/intel_intrmap.c
+++ b/sys/x86/iommu/intel_intrmap.c
@@ -234,6 +234,8 @@ dmar_ir_find(device_t src, uint16_t *rid, int *is_dmar)
{
devclass_t src_class;
struct dmar_unit *unit;
+ device_t requester;
+ int error __diagused;
/*
* We need to determine if the interrupt source generates FSB
@@ -253,8 +255,10 @@ dmar_ir_find(device_t src, uint16_t *rid, int *is_dmar)
unit = dmar_find_hpet(src, rid);
} else {
unit = dmar_find(src, bootverbose);
- if (unit != NULL && rid != NULL)
- iommu_get_requester(src, rid);
+ if (unit != NULL && rid != NULL) {
+ error = iommu_get_requester(src, &requester, rid);
+ MPASS(error == 0);
+ }
}
return (unit);
}
