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

On 8/24/26 10:11 AM, [email protected] wrote:
> 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?

Yes, those all need to be changed to use AP_DOMAINS

>