Re: [PATCH v1 3/4] iommu: Unpublish the old attach handle before the group replace callback

[Baolu Lu]

On 9/30/2026 5:51 AM, Nicolin Chen wrote:
> 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@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Nicolin Chen<nicolinc@xxxxxxxxxx>
> ---
>   drivers/iommu/iommu.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)

This doesn't fix the same-domain replace issue, which can still result
in a use-after-free bug. I agree that this needs a separate lifetime
mechanism as stated in the cover letter. But is it possible to prevent
the same-domain replace case from going through this interface?

For this patch alone,

Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>