Re: [PATCH v1 1/2] s390/zcrypt: Validate length for CCA AES cipher key requests

This is fixed by patch 2/2 of this series.

On 7/29/26 11:58, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Heap buffer overflow in `cca_ecc2protkey` when copying a user-provided ECC key token into a fixed-size buffer.
> --
> 
> --- Patch [1]: [PATCH v1 1/2] s390/zcrypt: Validate length for CCA AES cipher key requests ---
> commit 7b35c8aa05e7fb7c7837b88e5f3b7fc85f721c98
> Author: Holger Dengler <[email protected]>
> 
> s390/zcrypt: Validate length for CCA AES cipher key requests
> 
> This commit adds a bounds check in cca_cipher2protkey() to ensure the copy
> length derived from the key token does not exceed the available space in the
> CPRB parameter block. If the length is too large, the request is rejected
> early with -EINVAL.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index 84936a795b956..db6a4211a97e0 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
>> @@ -1261,6 +1261,9 @@ int cca_cipher2protkey(u16 cardnr, u16 domain, const u8 *ckey,
>>  	} __packed * prepparm;
>>  	int keytoklen = ((struct cipherkeytoken *)ckey)->len;
>>  
>> +	if (keytoklen > PARMBSIZE - sizeof(struct aureqparm))
>> +		return -EINVAL;
>> +
> 
> [Severity: Critical]
> This is a pre-existing issue, but does a similar heap buffer overflow exist in
> cca_ecc2protkey()?
> 
> It appears cca_ecc2protkey() also copies a user-provided ECC key token into a
> fixed-size buffer without a similar bounds check. A user with access to
> /dev/pkey could call the PKEY_KBLOB2PROTK2 ioctl, passing an
> eccprivkeytoken with a large len field.
> 
> This would be passed down through cca_key2protkey() to cca_ecc2protkey(),
> where it allocates a 512-byte parameter block via
> alloc_and_prep_cprbmem() and copies the token:
> 
> drivers/s390/crypto/zcrypt_ccamisc.c:cca_ecc2protkey() {
>     ...
> 	int keylen = ((struct eccprivkeytoken *)key)->len;
> 
> 	/* get already prepared memory for 2 cprbs with param block each */
> 	rc = alloc_and_prep_cprbmem(PARMBSIZE, &mem,
> 				    &preqcblk, &prepcblk, xflags);
>     ...
> 	/* kb, cca token */
> 	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);
>     ...
> }
> 
> Since the parameter block header consumes 90 bytes, any keylen > 422 bytes
> could overflow the allocated memory. Is a similar check needed there?
> 
>>  	/* get already prepared memory for 2 cprbs with param block each */
>>  	rc = alloc_and_prep_cprbmem(PARMBSIZE, &mem,
>>  				    &preqcblk, &prepcblk, xflags);
> 

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler