Re: [PATCH v1] s390/ap: Support new APMLM field with PQAP/TAPQ instruction

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] s390/ap: APMLM field is zeroed out by TAPQ_CARD_HWINFO_MASK
--

commit a92b6e193aedcdab18cc73823dae8ff72c9b6ca0
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/ap: Support new APMLM field with PQAP/TAPQ instruction

This commit introduces support for the new AP message limit multiplier (APMLM)
field defined within the GR2 register on successful invocation of the TAPQ
subfunction. The formula to calculate the AP max message limit is updated
to incorporate this new multiplier.

> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
> index c91b6ace199d7..01d575c358875 100644
> --- a/arch/s390/include/asm/ap.h
> +++ b/arch/s390/include/asm/ap.h
> @@ -123,8 +123,8 @@ struct ap_tapq_hwinfo {
>  			unsigned int	   : 14;
>  			unsigned int at	   :  8; /* ap type */
>  			unsigned int nd	   :  8; /* nr of domains */
> -			unsigned int	   :  4;
> -			unsigned int ml	   :  4; /* apxl ml */
> +			unsigned int mlm   :  4; /* AP msg limit multiplier */
> +			unsigned int ml	   :  4; /* AP msg limit */
>  			unsigned int	   :  3;
>  			unsigned int qd	   :  5; /* queue depth */
>  		};

[ ... ]

> diff --git a/drivers/s390/crypto/ap_card.c b/drivers/s390/crypto/ap_card.c
> index c86397f4ddcd9..e9a8180f08c24 100644
> --- a/drivers/s390/crypto/ap_card.c
> +++ b/drivers/s390/crypto/ap_card.c
> @@ -242,7 +242,8 @@ struct ap_card *ap_card_create(int id, struct ap_tapq_hwinfo hwinfo,
>  	ac->hwinfo = hwinfo;
>  	ac->id = id;
>  	ac->maxmsgsize = hwinfo.ml > 3 ?
> -		hwinfo.ml * AP_TAPQ_ML_FIELD_CHUNK_SIZE : AP_DEFAULT_MAX_MSG_SIZE;
> +		hwinfo.ml * (hwinfo.mlm + 1) * AP_TAPQ_ML_FIELD_CHUNK_SIZE :
> +		AP_DEFAULT_MAX_MSG_SIZE;

[Severity: Medium]
Will the new hwinfo.mlm value read here always evaluate to 0?

Looking at ap_scan_adapter() in drivers/s390/crypto/ap_bus.c, hwinfo.value
is masked with TAPQ_CARD_HWINFO_MASK (0xFFFF0000FFFF0F1FUL) before being
passed to ap_card_create():

drivers/s390/crypto/ap_bus.c:ap_scan_adapter() {
	...
	hwinfo.value &= TAPQ_CARD_HWINFO_MASK; /* filter card specific hwinfo */
	...
}

The 0x0F1F portion of the mask applies to bits 48-63. The leading 0 nibble
clears bits 48-51, effectively forcing hwinfo.mlm to always be 0. As a
result, the new max message size calculation hwinfo.ml * (hwinfo.mlm + 1)
seems to always evaluate to hwinfo.ml * 1, nullifying the intended
functionality.

Does TAPQ_CARD_HWINFO_MASK also need to be updated to preserve the newly
defined 4-bit mlm field?

Note: This masking behavior might actually be preventing a severe memory
exhaustion vulnerability. If the multiplier were not forced to 0,
unconditionally allocating up to ~960KB via kmalloc for hundreds of queues
in zcrypt_queue_alloc could cause frequent order-8 allocation failures.

>  
>  	return ac;
>  }

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