git: 6be8028f4441 - stable/13 - iommu_gas: Eliminate a possible case of use-after-free
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 08 Aug 2022 03:22:07 UTC
The branch stable/13 has been updated by dougm:
URL: https://cgit.FreeBSD.org/src/commit/?id=6be8028f44416597a8b3ae3d6423e318d5a1d34e
commit 6be8028f44416597a8b3ae3d6423e318d5a1d34e
Author: Alan Cox <alc@FreeBSD.org>
AuthorDate: 2022-07-22 17:00:26 +0000
Commit: Doug Moore <dougm@FreeBSD.org>
CommitDate: 2022-08-08 03:18:29 +0000
iommu_gas: Eliminate a possible case of use-after-free
Eliminate a possible case of use-after-free in an error handling path
after a mapping failure. Specifically, eliminate IOMMU_MAP_ENTRY_QI_NF
and instead perform the IOTLB invalidation synchronously. Otherwise,
when iommu_domain_unload_entry() is called and told not to free the
IOMMU map entry, the caller could free the entry before dmar_qi_task()
is finished with it.
Reviewed by: kib
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D35878
(cherry picked from commit 8bc3673847453ca51237b5c85fe57f3f02e17a4b)
---
sys/arm64/iommu/iommu.c | 3 ++-
sys/dev/iommu/iommu.h | 3 ++-
sys/dev/iommu/iommu_gas.c | 6 ++++--
sys/dev/iommu/iommu_gas.h | 1 -
sys/x86/iommu/intel_ctx.c | 28 +++++++++++++++++++---------
sys/x86/iommu/intel_dmar.h | 2 ++
sys/x86/iommu/intel_qi.c | 14 ++++++++++++--
7 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/sys/arm64/iommu/iommu.c b/sys/arm64/iommu/iommu.c
index 0fad03c7cd8e..0a7503976036 100644
--- a/sys/arm64/iommu/iommu.c
+++ b/sys/arm64/iommu/iommu.c
@@ -379,7 +379,8 @@ iommu_find(device_t dev, bool verbose)
}
void
-iommu_domain_unload_entry(struct iommu_map_entry *entry, bool free)
+iommu_domain_unload_entry(struct iommu_map_entry *entry, bool free,
+ bool cansleep __unused)
{
dprintf("%s\n", __func__);
diff --git a/sys/dev/iommu/iommu.h b/sys/dev/iommu/iommu.h
index 62b5659b6e83..65fefe3ada7b 100644
--- a/sys/dev/iommu/iommu.h
+++ b/sys/dev/iommu/iommu.h
@@ -151,7 +151,8 @@ void iommu_free_ctx_locked(struct iommu_unit *iommu, struct iommu_ctx *ctx);
struct iommu_ctx *iommu_get_ctx(struct iommu_unit *, device_t dev,
uint16_t rid, bool id_mapped, bool rmrr_init);
struct iommu_unit *iommu_find(device_t dev, bool verbose);
-void iommu_domain_unload_entry(struct iommu_map_entry *entry, bool free);
+void iommu_domain_unload_entry(struct iommu_map_entry *entry, bool free,
+ bool cansleep);
void iommu_domain_unload(struct iommu_domain *domain,
struct iommu_map_entries_tailq *entries, bool cansleep);
diff --git a/sys/dev/iommu/iommu_gas.c b/sys/dev/iommu/iommu_gas.c
index f94ab3756c7b..abc7a336117c 100644
--- a/sys/dev/iommu/iommu_gas.c
+++ b/sys/dev/iommu/iommu_gas.c
@@ -660,7 +660,8 @@ iommu_gas_map(struct iommu_domain *domain,
entry->end - entry->start, ma, eflags,
((flags & IOMMU_MF_CANWAIT) != 0 ? IOMMU_PGF_WAITOK : 0));
if (error == ENOMEM) {
- iommu_domain_unload_entry(entry, true);
+ iommu_domain_unload_entry(entry, true,
+ (flags & IOMMU_MF_CANWAIT) != 0);
return (error);
}
KASSERT(error == 0,
@@ -698,7 +699,8 @@ iommu_gas_map_region(struct iommu_domain *domain, struct iommu_map_entry *entry,
entry->end - entry->start, ma + OFF_TO_IDX(start - entry->start),
eflags, ((flags & IOMMU_MF_CANWAIT) != 0 ? IOMMU_PGF_WAITOK : 0));
if (error == ENOMEM) {
- iommu_domain_unload_entry(entry, false);
+ iommu_domain_unload_entry(entry, false,
+ (flags & IOMMU_MF_CANWAIT) != 0);
return (error);
}
KASSERT(error == 0,
diff --git a/sys/dev/iommu/iommu_gas.h b/sys/dev/iommu/iommu_gas.h
index c32a098538b0..a9d0df5f272f 100644
--- a/sys/dev/iommu/iommu_gas.h
+++ b/sys/dev/iommu/iommu_gas.h
@@ -50,7 +50,6 @@
#define IOMMU_MAP_ENTRY_MAP 0x0004 /* Busdma created, linked by
dmamap_link */
#define IOMMU_MAP_ENTRY_UNMAPPED 0x0010 /* No backing pages */
-#define IOMMU_MAP_ENTRY_QI_NF 0x0020 /* qi task, do not free entry */
#define IOMMU_MAP_ENTRY_READ 0x1000 /* Read permitted */
#define IOMMU_MAP_ENTRY_WRITE 0x2000 /* Write permitted */
#define IOMMU_MAP_ENTRY_SNOOP 0x4000 /* Snoop */
diff --git a/sys/x86/iommu/intel_ctx.c b/sys/x86/iommu/intel_ctx.c
index 4e94936f27d4..1bb923f33d86 100644
--- a/sys/x86/iommu/intel_ctx.c
+++ b/sys/x86/iommu/intel_ctx.c
@@ -864,25 +864,35 @@ dmar_domain_free_entry(struct iommu_map_entry *entry, bool free)
}
void
-iommu_domain_unload_entry(struct iommu_map_entry *entry, bool free)
+iommu_domain_unload_entry(struct iommu_map_entry *entry, bool free,
+ bool cansleep)
{
struct dmar_domain *domain;
struct dmar_unit *unit;
domain = IODOM2DOM(entry->domain);
unit = DOM2DMAR(domain);
+
+ /*
+ * If "free" is false, then the IOTLB invalidation must be performed
+ * synchronously. Otherwise, the caller might free the entry before
+ * dmar_qi_task() is finished processing it.
+ */
if (unit->qi_enabled) {
DMAR_LOCK(unit);
- dmar_qi_invalidate_locked(IODOM2DOM(entry->domain),
- entry->start, entry->end - entry->start, &entry->gseq,
- true);
- if (!free)
- entry->flags |= IOMMU_MAP_ENTRY_QI_NF;
- TAILQ_INSERT_TAIL(&unit->tlb_flush_entries, entry, dmamap_link);
+ if (free) {
+ dmar_qi_invalidate_locked(domain, entry->start,
+ entry->end - entry->start, &entry->gseq, true);
+ TAILQ_INSERT_TAIL(&unit->tlb_flush_entries, entry,
+ dmamap_link);
+ } else {
+ dmar_qi_invalidate_sync_locked(domain, entry->start,
+ entry->end - entry->start, cansleep);
+ }
DMAR_UNLOCK(unit);
} else {
- domain_flush_iotlb_sync(IODOM2DOM(entry->domain),
- entry->start, entry->end - entry->start);
+ domain_flush_iotlb_sync(domain, entry->start, entry->end -
+ entry->start);
dmar_domain_free_entry(entry, free);
}
}
diff --git a/sys/x86/iommu/intel_dmar.h b/sys/x86/iommu/intel_dmar.h
index e49f96bf93e1..21e626257a0d 100644
--- a/sys/x86/iommu/intel_dmar.h
+++ b/sys/x86/iommu/intel_dmar.h
@@ -250,6 +250,8 @@ int dmar_init_qi(struct dmar_unit *unit);
void dmar_fini_qi(struct dmar_unit *unit);
void dmar_qi_invalidate_locked(struct dmar_domain *domain, iommu_gaddr_t start,
iommu_gaddr_t size, struct iommu_qi_genseq *psec, bool emit_wait);
+void dmar_qi_invalidate_sync_locked(struct dmar_domain *domain,
+ iommu_gaddr_t start, iommu_gaddr_t size, bool cansleep);
void dmar_qi_invalidate_ctx_glob_locked(struct dmar_unit *unit);
void dmar_qi_invalidate_iotlb_glob_locked(struct dmar_unit *unit);
void dmar_qi_invalidate_iec_glob(struct dmar_unit *unit);
diff --git a/sys/x86/iommu/intel_qi.c b/sys/x86/iommu/intel_qi.c
index 894e3d537ac7..1400be3852e9 100644
--- a/sys/x86/iommu/intel_qi.c
+++ b/sys/x86/iommu/intel_qi.c
@@ -242,6 +242,17 @@ dmar_qi_invalidate_locked(struct dmar_domain *domain, iommu_gaddr_t base,
dmar_qi_advance_tail(unit);
}
+void
+dmar_qi_invalidate_sync_locked(struct dmar_domain *domain, iommu_gaddr_t base,
+ iommu_gaddr_t size, bool cansleep)
+{
+ struct iommu_qi_genseq gseq;
+
+ DMAR_ASSERT_LOCKED(domain->dmar);
+ dmar_qi_invalidate_locked(domain, base, size, &gseq, true);
+ dmar_qi_wait_for_seq(domain->dmar, &gseq, !cansleep);
+}
+
void
dmar_qi_invalidate_ctx_glob_locked(struct dmar_unit *unit)
{
@@ -352,8 +363,7 @@ dmar_qi_task(void *arg, int pending __unused)
break;
TAILQ_REMOVE(&unit->tlb_flush_entries, entry, dmamap_link);
DMAR_UNLOCK(unit);
- dmar_domain_free_entry(entry, (entry->flags &
- IOMMU_MAP_ENTRY_QI_NF) == 0);
+ dmar_domain_free_entry(entry, true);
DMAR_LOCK(unit);
}
ics = dmar_read4(unit, DMAR_ICS_REG);