Re: [PATCH v4 1/1] s390/zcrypt: Fix buffer over-read in cca_cipher2protkey

On 2026-07-29 14:06, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Unbounded token length allows heap buffer overflow in
> `cca_cipher2protkey` and `cca_ecc2protkey` when building CPRB
> requests.
> - [High] Unvalidated firmware-provided length in `cca_gencipherkey`
> can lead to a massive out-of-bounds read and memory leak to
> user-space.
> - [High] Unbounded `t->len` in `_ip_cprb_helper` can cause a heap
> buffer overflow when copying a response from the crypto card.
> --
> 
> --- Patch [1]: [PATCH v4 1/1] s390/zcrypt: Fix buffer over-read in
> cca_cipher2protkey ---
> commit df110319f8dcc4a61962c0699f5ad1038f1b9085
> Author: Harald Freudenberger <[email protected]>
> 
> s390/zcrypt: Fix buffer over-read in cca_cipher2protkey
> 
> This commit adds validation of the actual key buffer size and token 
> length
> fields in the cca_check_sec*token functions. It prevents buffer 
> over-reads
> and under-runs that could lead to kernel crashes or memory exposure.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c 
>> b/drivers/s390/crypto/zcrypt_ccamisc.c
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> [ ... ]
>> @@ -936,23 +975,28 @@ int cca_gencipherkey(u16 cardnr, u16 domain, u32 
>> keybitsize, u32 keygenflags,
>>  	}
>> 
>>  	/* and some checks on the generated key */
>> +	t = (struct cipherkeytoken *)prepparm->kb.tlv1.gen_key;
>> +	if (prepparm->kb.tlv1.len < 2 * sizeof(uint16_t) + sizeof(*t)) {
>> +		rc = -EIO;
>> +		goto out;
>> +	}
>> +	keybuflen = prepparm->kb.tlv1.len - 2 * sizeof(uint16_t);
>>  	rc = cca_check_secaescipherkey(zcrypt_dbf_info, DBF_ERR,
>>  				       prepparm->kb.tlv1.gen_key,
>> -				       keybitsize, 1);
>> +				       keybuflen, keybitsize, 1);
>>  	if (rc) {
>>  		rc = -EIO;
>>  		goto out;
>>  	}
>> 
>>  	/* copy the generated vlsc key token */
>> -	t = (struct cipherkeytoken *)prepparm->kb.tlv1.gen_key;
>>  	if (keybuf) {
>> -		if (*keybufsize >= t->len)
>> -			memcpy(keybuf, t, t->len);
>> +		if (*keybufsize >= keybuflen)
>> +			memcpy(keybuf, t, keybuflen);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can an unvalidated 
> firmware
> response lead to an out-of-bounds read here?
> 
> The hardware returns a response containing a tlv1.len field which is
> used to calculate keybuflen. If a compromised or buggy crypto card 
> firmware
> sets an abnormally large tlv1.len, does this memcpy read past the 
> 512-byte
> parameter block into adjacent kernel heap memory and leak it to 
> userspace?
> 
> Could we add a bounds check against the 512-byte allocation limit 
> before
> performing the memcpy?
> 
>>  		else
>>  			rc = -EINVAL;
>>  	}
>> -	*keybufsize = t->len;
>> +	*keybufsize = keybuflen;
>> 
>>  out:
> 
> [Severity: Critical]
> This is a pre-existing issue, but I noticed a potential regression 
> involving
> buffer overflows in cca_cipher2protkey and cca_ecc2protkey when 
> building
> request blocks.
> 
> In cca_cipher2protkey, if a user issues an ioctl providing a malicious 
> token
> with a large len field, does this overwrite the 512-byte buffer?
> 
> drivers/s390/crypto/zcrypt_ccamisc.c:cca_cipher2protkey() {
>     ...
>     preqparm->kb.len = keytoklen + 3 * sizeof(uint16_t);
>     preqparm->kb.cca_key_token_len = keytoklen + 2 * sizeof(uint16_t);
>     memcpy(preqparm->kb.cca_key_token, ckey, keytoklen);
>     ...
> }
> 
> The cca_check_secaescipherkey function ensures t->len is less than or 
> equal
> to keysize, where keysize is the user-provided size that can be up to
> 8192 bytes. If keytoklen is larger than the 512-byte limit, could this 
> memcpy
> overwrite the allocation?
> 
> A similar pattern exists in cca_ecc2protkey:
> 
> drivers/s390/crypto/zcrypt_ccamisc.c:cca_ecc2protkey() {
>     ...
>     preqparm->kb.len = keylen + 3 * sizeof(uint16_t);
>     preqparm->kb.cca_key_token_len = keylen + 2 * sizeof(uint16_t);
>     memcpy(preqparm->kb.cca_key_token, key, keylen);
>     ...
> }
> 
> [Severity: High]
> This is also a pre-existing issue, but I observed a similar unbounded 
> length
> usage in _ip_cprb_helper that might cause an out-of-bounds read and 
> write.
> 
> When copying a response from the crypto card, the t->len field is 
> trusted
> directly from the hardware:
> 
> drivers/s390/crypto/zcrypt_ccamisc.c:_ip_cprb_helper() {
>     ...
>     t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
>     memcpy(key_token, t, t->len);
>     *key_token_size = t->len;
>     ...
> }
> 
> If the hardware provides a malformed response with a large t->len, does 
> this
> memcpy read out-of-bounds from the 512-byte parameter block?
> 
> Additionally, if this corrupted length is returned to 
> cca_clr2cipherkey,
> could it be used to write back into a new 512-byte request block and 
> cause
> another overflow?

These issues are valid but are already addressed with another patch 
series.