* [PATCH v2 1/5] iommu: Unpublish the attach handle before the group detach callback
2026-11-02 7:04 [PATCH v2 0/5] iommu: Fix attach handle publication ordering Nicolin Chen
@ 2026-11-02 7:04 ` Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 2/5] iommu: Unpublish the attach handle before the PASID " Nicolin Chen
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2026-11-02 7:04 UTC (permalink / raw)
To: joro, Lu Baolu, Kevin Tian, Yi Liu
Cc: Will Deacon, Robin Murphy, Jean-Philippe Brucker,
Jason Gunthorpe, iommu, linux-kernel
The group->pasid_array entry is erased only after the detach callback has
returned, so the outgoing attach handle stays published for the whole of
that call. iommu_attach_handle_get() reads it under the xa_lock alone, not
under the group mutex, so a fault raised in the meantime still finds that
handle and the domain being detached, both of which the caller frees the
moment that detach returns.
Commit 5e9f822c9c68 ("iommu: Swap the order of setting group->pasid_array
and calling attach op of iommu drivers") has fixed the attach path, while
the detach path never got the symmetric change.
Erase the entry ahead of the callback instead. A missing entry reads back
as NULL from xa_load(), so that iommu_attach_handle_get() returns -ENOENT
and the fault gets rejected rather than delivered against a domain that is
on its way out.
Fixes: 8519e689834a ("iommu: Extend domain attach group with handle support")
Cc: stable@vger.kernel.org
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b486b8bbd1fcd..0f9d49a5c3fa2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3951,8 +3951,12 @@ void iommu_detach_group_handle(struct iommu_domain *domain,
struct iommu_group *group)
{
mutex_lock(&group->mutex);
- __iommu_group_set_core_domain(group);
+ /*
+ * Unpublish the handle first, so it would not resolve to the detaching
+ * domain once the driver is detaching it.
+ */
xa_erase(&group->pasid_array, IOMMU_NO_PASID);
+ __iommu_group_set_core_domain(group);
mutex_unlock(&group->mutex);
}
EXPORT_SYMBOL_NS_GPL(iommu_detach_group_handle, "IOMMUFD_INTERNAL");
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 2/5] iommu: Unpublish the attach handle before the PASID detach callback
2026-11-02 7:04 [PATCH v2 0/5] iommu: Fix attach handle publication ordering Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 1/5] iommu: Unpublish the attach handle before the group detach callback Nicolin Chen
@ 2026-11-02 7:04 ` Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 3/5] iommu: Unpublish the old attach handle before the group replace callback Nicolin Chen
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2026-11-02 7:04 UTC (permalink / raw)
To: joro, Lu Baolu, Kevin Tian, Yi Liu
Cc: Will Deacon, Robin Murphy, Jean-Philippe Brucker,
Jason Gunthorpe, iommu, linux-kernel
iommu_detach_device_pasid() erases the group->pasid_array entry only once
__iommu_remove_group_pasid() has returned, so the detaching handle stays
visible to iommu_attach_handle_get() for the whole of that callback, and
the caller frees it as soon as the detach returns.
Erase the entry ahead of the callback, for the same reason the group path
does: a fault raised in the meantime must not resolve to a domain that is
on its way out.
Fixes: 16603704559c ("iommu: Add attach/detach_dev_pasid iommu interfaces")
Cc: stable@vger.kernel.org
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0f9d49a5c3fa2..aa50a8f6ef6f2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3818,8 +3818,12 @@ void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev,
struct iommu_group *group = dev->iommu_group;
mutex_lock(&group->mutex);
- __iommu_remove_group_pasid(group, pasid, domain);
+ /*
+ * Unpublish the handle first, so it would not resolve to the detaching
+ * domain once the driver is detaching it.
+ */
xa_erase(&group->pasid_array, pasid);
+ __iommu_remove_group_pasid(group, pasid, domain);
mutex_unlock(&group->mutex);
}
EXPORT_SYMBOL_GPL(iommu_detach_device_pasid);
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 3/5] iommu: Unpublish the old attach handle before the group replace callback
2026-11-02 7:04 [PATCH v2 0/5] iommu: Fix attach handle publication ordering Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 1/5] iommu: Unpublish the attach handle before the group detach callback Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 2/5] iommu: Unpublish the attach handle before the PASID " Nicolin Chen
@ 2026-11-02 7:04 ` Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 4/5] iommu: Unpublish the old attach handle before the PASID " Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 5/5] iommu: Reject a same-domain replace in the attach handle APIs Nicolin Chen
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2026-11-02 7:04 UTC (permalink / raw)
To: joro, Lu Baolu, Kevin Tian, Yi Liu
Cc: Will Deacon, Robin Murphy, Jean-Philippe Brucker,
Jason Gunthorpe, iommu, linux-kernel
iommu_replace_group_handle() looks like it hides the detaching handle ahead
of the driver callback. But in fact, xa_reserve() calls xa_cmpxchg(), which
stores only when the current entry matches @old=NULL. For a replace, there
must be an entry sitting there by definition (i.e. @old cannot be NULL), so
the compare fails, XA_ZERO_ENTRY is never written, and the call decays into
a plain read.
Therefore, the old handle stays published during __iommu_group_set_domain()
and only gets replaced after the call. This creates a window, during which
an asynchronous fault path might hit UAF:
core, holding group->mutex fault path, no group->mutex
========================== ===========================
xa_reserve() ,-- old handle stays published
no-op, the slot is occupied |
__iommu_group_set_domain() |
driver attach/detach ops |
iopf_queue_flush_dev() |
| iommu_attach_handle_get()
| reads old_handle
xa_store(new entry) `-- window closes
mutex_unlock()
kfree(old_handle) [caller]
UAF: old_handle->domain->iopf_handler()
Replace the xa_reserve() with xa_store(XA_ZERO_ENTRY) to unpublish the old
handle.
Fixes: 8519e689834a ("iommu: Extend domain attach group with handle support")
Cc: stable@vger.kernel.org
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/iommu.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index aa50a8f6ef6f2..0a199c1c3141e 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3985,7 +3985,7 @@ int iommu_replace_group_handle(struct iommu_group *group,
struct iommu_domain *new_domain,
struct iommu_attach_handle *handle)
{
- void *curr, *entry;
+ void *curr, *entry, *old;
int ret;
if (!new_domain || !handle)
@@ -3993,22 +3993,25 @@ int iommu_replace_group_handle(struct iommu_group *group,
mutex_lock(&group->mutex);
entry = iommu_make_pasid_array_entry(new_domain, handle);
- ret = xa_reserve(&group->pasid_array, IOMMU_NO_PASID, GFP_KERNEL);
- if (ret)
+ /*
+ * iommu_attach_handle_get() runs without the group mutex, so unpublish
+ * the old handle before the driver callback: a fault must not resolve
+ * to either domain while the switch is in progress. This reserves the
+ * slot too, so the store below cannot fail.
+ */
+ old = xa_store(&group->pasid_array, IOMMU_NO_PASID, XA_ZERO_ENTRY,
+ GFP_KERNEL);
+ if (xa_is_err(old)) {
+ ret = xa_err(old);
goto err_unlock;
+ }
ret = __iommu_group_set_domain(group, new_domain);
if (ret)
- goto err_release;
+ entry = old; /* Restore the old handle */
curr = xa_store(&group->pasid_array, IOMMU_NO_PASID, entry, GFP_KERNEL);
WARN_ON(xa_is_err(curr));
-
- mutex_unlock(&group->mutex);
-
- return 0;
-err_release:
- xa_release(&group->pasid_array, IOMMU_NO_PASID);
err_unlock:
mutex_unlock(&group->mutex);
return ret;
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 4/5] iommu: Unpublish the old attach handle before the PASID replace callback
2026-11-02 7:04 [PATCH v2 0/5] iommu: Fix attach handle publication ordering Nicolin Chen
` (2 preceding siblings ...)
2026-11-02 7:04 ` [PATCH v2 3/5] iommu: Unpublish the old attach handle before the group replace callback Nicolin Chen
@ 2026-11-02 7:04 ` Nicolin Chen
2026-11-02 7:04 ` [PATCH v2 5/5] iommu: Reject a same-domain replace in the attach handle APIs Nicolin Chen
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2026-11-02 7:04 UTC (permalink / raw)
To: joro, Lu Baolu, Kevin Tian, Yi Liu
Cc: Will Deacon, Robin Murphy, Jean-Philippe Brucker,
Jason Gunthorpe, iommu, linux-kernel
Similar to iommu_replace_group_handle(), iommu_replace_device_pasid() has
the same inert reserve as the group path and the UAF window:
core, holding group->mutex fault path, no group->mutex
========================== ===========================
xa_cmpxchg() ,-- old handle stays published
no-op, the slot is occupied |
__iommu_set_group_pasid() |
driver attach/detach ops |
iopf_queue_flush_dev() |
| iommu_attach_handle_get()
| reads old_handle
xa_store(new entry) `-- window closes
mutex_unlock()
kfree(old_handle) [caller]
UAF: old_handle->domain->iopf_handler()
Store XA_ZERO_ENTRY outright, just as the group path now does.
Fixes: 8a9e1e773f60 ("iommu: Introduce a replace API for device pasid")
Cc: stable@vger.kernel.org
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/iommu.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0a199c1c3141e..3fea48d3538ad 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3751,8 +3751,13 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
}
entry = iommu_make_pasid_array_entry(domain, handle);
- curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
- XA_ZERO_ENTRY, GFP_KERNEL);
+ /*
+ * iommu_attach_handle_get() runs without the group mutex, so unpublish
+ * the old handle before the driver callback: a fault must not resolve
+ * to either domain while the switch is in progress. This reserves the
+ * slot too, so the store below cannot fail.
+ */
+ curr = xa_store(&group->pasid_array, pasid, XA_ZERO_ENTRY, GFP_KERNEL);
if (xa_is_err(curr)) {
ret = xa_err(curr);
goto out_unlock;
@@ -3776,7 +3781,7 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
if (curr == entry) {
WARN_ON(1);
ret = -EINVAL;
- goto out_unlock;
+ goto out_store;
}
curr_domain = pasid_array_entry_to_domain(curr);
@@ -3786,16 +3791,16 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
ret = __iommu_set_group_pasid(domain, group,
pasid, curr_domain);
if (ret)
- goto out_unlock;
+ entry = curr; /* restore the old handle */
}
+out_store:
/*
- * The above xa_cmpxchg() reserved the memory, and the
- * group->mutex is held, this cannot fail.
+ * The xa_store() above reserved the memory, and the group->mutex
+ * is held, this cannot fail.
*/
WARN_ON(xa_is_err(xa_store(&group->pasid_array,
pasid, entry, GFP_KERNEL)));
-
out_unlock:
mutex_unlock(&group->mutex);
return ret;
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 5/5] iommu: Reject a same-domain replace in the attach handle APIs
2026-11-02 7:04 [PATCH v2 0/5] iommu: Fix attach handle publication ordering Nicolin Chen
` (3 preceding siblings ...)
2026-11-02 7:04 ` [PATCH v2 4/5] iommu: Unpublish the old attach handle before the PASID " Nicolin Chen
@ 2026-11-02 7:04 ` Nicolin Chen
4 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2026-11-02 7:04 UTC (permalink / raw)
To: joro, Lu Baolu, Kevin Tian, Yi Liu
Cc: Will Deacon, Robin Murphy, Jean-Philippe Brucker,
Jason Gunthorpe, iommu, linux-kernel
Both iommu_replace_group_handle() and iommu_replace_device_pasid() skip the
driver callback when the new domain is the one currently attached. In that
case, nothing flushes the pending faults that may still be holding the old
handle. Then, the caller frees the old handle once the replace returns, so
such a fault would hit a use-after-free.
No caller does this today, since iommufd_device_do_replace() returns early
when the new hwpt is the old one. However, it is only a caller convention,
which neither API enforces.
Reject a same-domain replace in both APIs with a WARN_ON, and document it
in their kernel-docs.
Suggested-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/iommu.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3fea48d3538ad..a9bc4ca5dbb4e 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3716,6 +3716,10 @@ EXPORT_SYMBOL_GPL(iommu_attach_device_pasid);
* Caller should always provide a new handle to avoid race with the paths
* that have lockless reference to handle if it intends to pass a valid handle.
*
+ * The @domain must not be the domain currently attached to the @pasid. Such a
+ * same-domain replace would skip the driver callback flushing pending faults.
+ * Then, those faults could use the old handle after the caller frees it.
+ *
* Return 0 on success, or an error.
*/
int iommu_replace_device_pasid(struct iommu_domain *domain,
@@ -3785,15 +3789,17 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
}
curr_domain = pasid_array_entry_to_domain(curr);
- ret = 0;
-
- if (curr_domain != domain) {
- ret = __iommu_set_group_pasid(domain, group,
- pasid, curr_domain);
- if (ret)
- entry = curr; /* restore the old handle */
+ /* Same as iommu_replace_group_handle(), reject a same-domain replace */
+ if (WARN_ON(curr_domain == domain)) {
+ entry = curr; /* restore the old handle */
+ ret = -EINVAL;
+ goto out_store;
}
+ ret = __iommu_set_group_pasid(domain, group, pasid, curr_domain);
+ if (ret)
+ entry = curr; /* restore the old handle */
+
out_store:
/*
* The xa_store() above reserved the memory, and the group->mutex
@@ -3985,6 +3991,10 @@ EXPORT_SYMBOL_NS_GPL(iommu_detach_group_handle, "IOMMUFD_INTERNAL");
*
* Caller should always provide a new handle to avoid race with the paths
* that have lockless reference to handle.
+ *
+ * The @new_domain must not be the domain currently attached to the group. Such
+ * a same-domain replace would skip the driver callback flushing pending faults.
+ * Then, those faults could use the old handle after the caller frees it.
*/
int iommu_replace_group_handle(struct iommu_group *group,
struct iommu_domain *new_domain,
@@ -3997,6 +4007,16 @@ int iommu_replace_group_handle(struct iommu_group *group,
return -EINVAL;
mutex_lock(&group->mutex);
+ /*
+ * A same-domain replace would skip the driver callback flushing pending
+ * faults. Then, those faults could use the old handle after the caller
+ * frees it.
+ */
+ if (WARN_ON(group->domain == new_domain)) {
+ ret = -EINVAL;
+ goto err_unlock;
+ }
+
entry = iommu_make_pasid_array_entry(new_domain, handle);
/*
* iommu_attach_handle_get() runs without the group mutex, so unpublish
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread