Re: [PATCH v9 4/5] s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing

On 2026-07-29 18:46, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Critical] Missing bounds checking before reading `pld[offs++]`
> causes an integer underflow in `pld_len - offs`, bypassing bounds
> checks and enabling an Out-Of-Bounds (OOB) read and write.
> --
> 
> --- Patch [4]: [PATCH v9 4/5] s390/zcrypt: Improve EP11 CPRB domain
> handling with ASN.1 parsing ---
> commit 750aceaeb85ad5fa9ff8e2614110f89a4c147522
> Author: Harald Freudenberger <[email protected]>
> 
> s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing
> 
> This commit replaces struct overlays with explicit ASN.1 parsing for 
> EP11
> CPRB domain handling. It introduces bounds checking at each parsing 
> step
> and adds a helper function to safely write integer values.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c 
>> b/drivers/s390/crypto/zcrypt_msgtype6.c
>> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
>> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> 
> [ ... ]
> 
>> @@ -1235,51 +1258,90 @@ static long 
>> zcrypt_msgtype6_send_ep11_cprb(bool userspace, struct zcrypt_queue *
>>  					   struct ap_message *ap_msg)
>>  {
> 
> [ ... ]
> 
>> +		/* payload length field */
>> +		i = asn1_length_decode(pld + offs, pld_len - offs, &u);
>> +		if (i < 0)
>> +			return -EINVAL;
>> +		offs += i;
>> +		if (u > pld_len - offs)
>> +			return -EINVAL;
>> +		/* function tag */
>> +		if (pld[offs++] != 0x04)
> 
> [Severity: Critical]
> If the ASN.1 payload tag and length encoding completely consume the 
> payload
> buffer, couldn't offs become equal to pld_len here?
> 
> If offs equals pld_len, does this array access read out of bounds and
> increment offs past pld_len?
> 
> If offs exceeds pld_len, it appears the subsequent pld_len - offs
> calculations will underflow to SIZE_MAX. Would this bypass the 
> remaining
> bounds checks, such as pld_len - offs >= 3, and potentially allow
> asn1_int_encode() to write out of bounds?
> 
> Additionally, it looks like a similar pattern exists in
> xcrb_msg_to_type6_ep11cprb_msgx() where pld[offs++] is checked for the
> function tag without a prior bounds check.
> 

I'll add checks about exceeding the payload for these both functions
into the next version of this patch series.

>> +			return -EINVAL;
>> +		/* function length */
>> +		i = asn1_length_decode(pld + offs, pld_len - offs, &u);
>> +		if (i < 0)
>> +			return -EINVAL;
>> +		offs += i;
>> +		if (u > pld_len - offs)
>> +			return -EINVAL;
> 
> [ ... ]