Re: [PATCH v8 1/6] s390/vfio-ap: Fix leaks of pinned NIB and registered GISC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/vfio-ap: wild DMA write due to unconditional NIB unpin in unmap_iova
--

commit f138c70af93258555b6e1c1c1233b5d5dc8b82da
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

s390/vfio-ap: Fix leaks of pinned NIB and registered GISC

This commit addresses resource leaks of pinned NIB pages and registered
GISCs across several paths. It introduces intentional leaks for cases where
hardware might still be able to write to the NIB, while freeing resources
when queues become unavailable.

> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
[ ... ]
> @@ -1857,8 +2006,28 @@ static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova, u64 length)
>  	int loop_cursor;
>  
>  	hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
> -		if (q->saved_iova >= iova && q->saved_iova < iova + length)
> +		if (q->saved_iova >= iova && q->saved_iova < iova + length) {
>  			vfio_ap_irq_disable(q);
> +			/*
> +			 * If IRQ disable failed or IR=0 could not be confirmed,
> +			 * vfio_ap_irq_disable() intentionally leaks the NIB to
> +			 * prevent a wild DMA write. But vfio core requires the
> +			 * page to be unpinned before dma_unmap returns, or it
> +			 * will BUG_ON after 10 re-notification rounds.
> +			 *
> +			 * Fall back to a bounded queue reset. The ZAPQ zeroizes
> +			 * the NIB pointer in hardware, eliminating the DMA risk
> +			 * that justified the leak. Once the worker finishes (or
> +			 * times out with a reset confirmed in-progress), the
> +			 * hardware no longer holds a reference to saved_iova and
> +			 * it is safe to unpin unconditionally.
> +			 */
> +			if (q->saved_iova) {
> +				vfio_ap_mdev_reset_queue(q);
> +				flush_work(&q->reset_work);
> +				vfio_ap_free_aqic_resources(q);

[Severity: High]
Does this unconditional call to vfio_ap_free_aqic_resources() risk allowing
a wild DMA write into host memory?

If vfio_ap_mdev_reset_queue() encounters a hardware fault and TAPQ returns
an invalid response code, it intentionally leaves the NIB pinned. The
comment in vfio_ap_mdev_reset_queue() notes:

"An invalid response code indicates a hardware or firmware bug. Since we
cannot determine whether the queue can still DMA-write to the NIB, the AQIC
resources are intentionally leaked lest the NIB page is reallocated to a new
owner."

However, calling vfio_ap_free_aqic_resources() in unmap_iova() forces the
NIB page to be unpinned regardless of the reset result. If the ZAPQ failed
to zeroize the NIB, unpinning it here overrides the intentional safety leak
and could expose the host to memory corruption.

> +			}
> +		}
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925124551.665448-1-akrowiak@linux.ibm.com?part=1