Re: [PATCH v3 6/9] s390/vfio-ap: fix potential use of uninitialized apm_filtered bitmap

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential subsystem-wide hang due to an unbounded infinite loop in `apq_reset_check()` while holding global mutexes.
--

--- Patch [6]: [PATCH v3 6/9] s390/vfio-ap: fix potential use of uninitialized apm_filtered bitmap ---
commit 54d6a10b5f4c5f98506b757af68f95ada615865b
Author: Anthony Krowiak <[email protected]>

s390/vfio-ap: fix potential use of uninitialized apm_filtered bitmap

This commit fixes a bug where the apm_filtered bitmap was used uninitialized
if a hot plug configuration change only added control domains. It zeroes the
bitmap using bitmap_zero() at the start of the function to prevent using
stack garbage.

Cc: [email protected]

> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index b6d0c988cc810..cdc9bf5c5e53d 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -2840,6 +2840,8 @@ static void vfio_ap_mdev_hot_plug_cfg(struct ap_matrix_mdev *matrix_mdev)
>  	DECLARE_BITMAP(apm_filtered, AP_DEVICES);
>  	bool filter_domains, filter_adapters, filter_cdoms, do_hotplug = false;
>  
> +	bitmap_zero(apm_filtered, AP_DEVICES);
> +

[Severity: High]
This isn't a bug introduced by this patch, but can the subsystem hang if the
hardware continuously returns -EBUSY?

Looking at apq_reset_check(), there is a loop that continues indefinitely
without a timeout bounds:

drivers/s390/crypto/vfio_ap_ops.c:apq_reset_check() {
	while (true) {
		msleep(AP_RESET_INTERVAL);
		elapsed += AP_RESET_INTERVAL;
		status = ap_tapq(q->apqn, NULL);
		ret = apq_status_check(q->apqn, &status);
		if (ret == -EIO)
			return;
		if (ret == -EBUSY) {
			pr_notice_ratelimited(WAIT_MSG, elapsed,...);
}

If a user process triggers a reset, it queues a reset and subsequently waits
for it using flush_work() while holding the global matrix_dev->mdevs_lock
and matrix_dev->guests_lock:

drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_reset_queues() {
	hash_for_each(matrix_mdev->qtable.queues, loop_cursor, q, mdev_qnode) {
		flush_work(&q->reset_work);
}

Could this create a permanent kernel thread hang that persistently holds
global locks if the hardware is unresponsive or repeatedly returns
AP_RESPONSE_BUSY?

>  	filter_adapters = bitmap_intersects(matrix_mdev->matrix.apm,
>  					    matrix_mdev->apm_add, AP_DEVICES);
>  	filter_domains = bitmap_intersects(matrix_mdev->matrix.aqm,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6