Re: [PATCH 4/4] s390/vfio-ap: Use AP_DOMAINS for adm_add bitmap size in vfio_ap_mdev_cfg_add()

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

Pre-existing issues:
- [High] Incomplete Fix / Latent Stack Buffer Overflow due to Inconsistent Constant Usage
--

--- Patch [4]: [PATCH 4/4] s390/vfio-ap: Use AP_DOMAINS for adm_add bitmap size in vfio_ap_mdev_cfg_add() ---
commit 35cf2442d556c90b8b97a43b8f409fe51d21bae0
Author: Anthony Krowiak <[email protected]>

s390/vfio-ap: Use AP_DOMAINS for adm_add bitmap size in vfio_ap_mdev_cfg_add()

This commit changes the bitmap size constant from AP_DEVICES to AP_DOMAINS
in vfio_ap_mdev_cfg_add() for the adm_add bitmap. The change prevents future
issues if the constants' values ever diverge.

>     Control domains are indexed by AP queue index (APQI) and their
>     bitmaps are sized by AP_DOMAINS, not AP_DEVICES. The two
>     constants are both 256 today so there is no functional impact,
>     but the wrong constant is inconsistent with the two bitmap_and()
>     calls immediately above it and with every other operation on
>     adm/aqm bitmaps in the file, all of which correctly use
>     AP_DOMAINS.

[Severity: High]
This is a pre-existing issue, but does every other operation correctly use
AP_DOMAINS?

Looking at vfio_ap_mdev_on_cfg_remove(), it appears AP_DEVICES is still used
for domain masks sized by AP_DOMAINS:

drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_on_cfg_remove() {
...
	do_remove |= bitmap_andnot(aqrem,
				   (unsigned long *)prev_config_info->aqm,
				   (unsigned long *)cur_config_info->aqm,
				   AP_DEVICES);
	do_remove |= bitmap_andnot(cdrem,
				   (unsigned long *)prev_config_info->adm,
				   (unsigned long *)cur_config_info->adm,
				   AP_DEVICES);
...
}

Similarly, in vfio_ap_mdev_hot_unplug_domain():

drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_hot_unplug_domain() {
...
	DECLARE_BITMAP(apqis, AP_DOMAINS);

	bitmap_zero(apqis, AP_DEVICES);
...
}

If the constants AP_DEVICES and AP_DOMAINS ever diverge, could these
operations cause out-of-bounds stack memory accesses?

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