Re: [PATCH 6/6] dmaengine: idxd: Do not release interrupt handles on terminate_all

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dma: idxd: sleep in atomic context in device_terminate_all()
- [High] dmaengine: idxd: Hardware UAF and IOMMU faults due to flushing active WQ without disable
- [Medium] dmaengine: idxd: missing check for INVALID_INT_HANDLE in release
--

commit dc7ea0d446f4b8c4e4f29fa4595acabcca6b297f
Author: Vinicius Costa Gomes <vinicius.gomes@intel.com>

dmaengine: idxd: Do not release interrupt handles on terminate_all

This commit splits the interrupt handle teardown from the descriptor
flushing in idxd_wq_flush_descs(). This prevents the interrupt handle
from being incorrectly released during dmaengine terminate_all, which
was causing subsequent runs to time out.

> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
> index 131138483b877..06299f40f5312 100644
> --- a/drivers/dma/idxd/device.c
> +++ b/drivers/dma/idxd/device.c
> @@ -1349,6 +1349,16 @@ void idxd_wq_free_irq(struct idxd_wq *wq)
>  }
>  
>  void idxd_wq_flush_descs(struct idxd_wq *wq)
> +{
> +	guard(mutex)(&wq->wq_lock);
> +
> +	if (wq->state != IDXD_WQ_ENABLED || wq->type != IDXD_WQT_KERNEL)
> +		return;
> +
> +	idxd_flush_pending_descs(&wq->ie);
> +}

[Severity: High]
Does this code introduce a sleep-in-atomic bug when called via the
dmaengine termination path?

The dmaengine API allows dmaengine_terminate_async() to be called from
atomic context (such as an interrupt handler or while holding a spinlock).

This routes to idxd_dma_terminate_all(), which then calls
idxd_wq_flush_descs().

Because idxd_wq_flush_descs() unconditionally acquires wq->wq_lock using
a sleepable mutex, terminating an asynchronous transfer from atomic
context could trigger a scheduling-while-atomic panic.

[Severity: High]
Can this flush implementation cause a use-after-free and IOMMU faults?

By calling idxd_flush_pending_descs() without first disabling or halting
the hardware workqueue, the software descriptors are completed, freed,
and client memory is unmapped while the hardware may still be actively
processing them.

Since the hardware continues to execute DMA operations into buffers that
have already been unmapped, can this result in memory corruption and
IOMMU faults?

> +
> +void idxd_wq_release_int_handle(struct idxd_wq *wq)
>  {
>  	struct idxd_irq_entry *ie = &wq->ie;
>  	struct idxd_device *idxd = wq->idxd;
> @@ -1358,7 +1368,6 @@ void idxd_wq_flush_descs(struct idxd_wq *wq)
>  	if (wq->state != IDXD_WQ_ENABLED || wq->type != IDXD_WQT_KERNEL)
>  		return;
>  
> -	idxd_flush_pending_descs(ie);
>  	if (idxd->request_int_handles)
>  		idxd_device_release_int_handle(idxd, ie->int_handle, IDXD_IRQ_MSIX);

[Severity: Medium]
Is it possible for ie->int_handle to be INVALID_INT_HANDLE here?

If a previous operation leaves the handle as INVALID_INT_HANDLE (-1) but
the workqueue remains in the IDXD_WQ_ENABLED state, this code will
unconditionally issue a release command to the hardware using -1 (0xffff).

Should this function include a check for ie->int_handle ==
INVALID_INT_HANDLE before calling idxd_device_release_int_handle(),
similar to how it is handled in idxd_wq_free_irq()?

>  	idxd_device_clear_perm_entry(idxd, ie);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-wt-idxd-fixes-v1-0-30a35a445077@intel.com?part=6