Re: [PATCH v2 2/2] s390/zcrypt: Fix out-of-bounds ptr advance in cca_query_crypto_facility()

[Harald Freudenberger <freude@linux.ibm.com>]

On 2026-10-06 16:22, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] crypto: zcrypt: OOB read and underflow in 
> cca_query_crypto_facility()
> --
> 
> commit 5de45449fe732b12f3310f3ce76a2abecc04ed97
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/zcrypt: Fix out-of-bounds ptr advance in 
> cca_query_crypto_facility()
> 
> The FQ reply parser code blindly advanced the walk pointer by a length
> value read directly from the hardware reply payload without checking
> that the advance stayed within the allocated reply buffer.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c 
>> b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index 19909bf43dc91..862947431164f 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> [ ... ]
>> @@ -1673,27 +1674,44 @@ int cca_query_crypto_facility(u16 cardnr, u16 
>> domain,
>>  	prepcblk->rpl_parmb = (u8 __user *)ptr;
>>  	prepparm = (struct fqrepparm *)ptr;
>>  	ptr = prepparm->lvdata;
>> +	datalen = parmbsize - 2 * sizeof(u8);
>> 
>>  	/* check and possibly copy reply rule array */
>>  	len = *((u16 *)ptr);
>> +	if (len > datalen) {
>> +		ZCRYPT_DBF_ERR("%s reply rule array len %u exceeds datalen %zu\n",
>> +			       __func__, len, datalen);
>> +		rc = -EIO;
>> +		goto out;
>> +	}
>> +	datalen -= sizeof(u16);
>> +	ptr += sizeof(u16);
>>  	if (len > sizeof(u16)) {
>> -		ptr += sizeof(u16);
>>  		len -= sizeof(u16);
>>  		if (rarray && rarraylen && *rarraylen > 0) {
>>  			*rarraylen = (len > *rarraylen ? *rarraylen : len);
>>  			memcpy(rarray, ptr, *rarraylen);
>>  		}
>> +		datalen -= len;
>>  		ptr += len;
>>  	}
>>  	/* check and possible copy reply var array */
>>  	len = *((u16 *)ptr);
> 
> [Severity: High]
> Does this code introduce an out-of-bounds read and subsequent integer
> underflow?
> 
> If a corrupt or malicious device response sets the first array length 
> (len) to
> exactly or almost exactly the remaining reply buffer space (datalen), 
> the
> first array parsing will advance ptr to the end of the buffer and 
> reduce
> datalen to 0 or 1.
> 
> When we reach the second array parsing here, a 16-bit length field is
> unconditionally read from ptr before checking if there are at least 2 
> bytes
> remaining in the buffer (datalen >= sizeof(u16)). This could read 2 
> bytes past
> the end of the allocated buffer.
> 
>> +	if (len > datalen) {
>> +		ZCRYPT_DBF_ERR("%s reply var array len %u exceeds datalen %zu\n",
>> +			       __func__, len, datalen);
>> +		rc = -EIO;
>> +		goto out;
>> +	}
>> +	datalen -= sizeof(u16);
> 
> [Severity: High]
> If the out-of-bounds read above results in a value smaller than or 
> equal to
> datalen (e.g., 0), the check (len > datalen) is bypassed, and the 
> subtraction
> here will blindly consume 2 bytes. This would cause an integer 
> underflow if
> datalen was 0 or 1, wrapping the tracking variable to a huge value.
> 
>> +	ptr += sizeof(u16);
>>  	if (len > sizeof(u16)) {
>> -		ptr += sizeof(u16);
>>  		len -= sizeof(u16);
>>  		if (varray && varraylen && *varraylen > 0) {
>>  			*varraylen = (len > *varraylen ? *varraylen : len);
>>  			memcpy(varray, ptr, *varraylen);
>>  		}
>> +		datalen -= len;
>>  		ptr += len;
>>  	}
>> 

Ok i see ... this needs further investigation as well.