Re: [PATCH v1 2/2] s390/zcrypt: Validate length for CCA ECC private key requests

[Holger Dengler <dengler@linux.ibm.com>]

The findings will be covered in a separate series.

On 7/29/26 12:16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing bounds validation on user-provided keys leads to Slab Out-of-Bounds Reads.
> - [High] Unvalidated token length from hardware reply causes Heap Buffer Overflow and Out-of-Bounds Read.
> --
> 
> --- Patch [2]: [PATCH v1 2/2] s390/zcrypt: Validate length for CCA ECC private key requests ---
> commit dd9e9a635152d7bae8edba05dc9b9aad02d9a1e9
> Author: Holger Dengler <dengler@linux.ibm.com>
> 
> s390/zcrypt: Validate length for CCA ECC private key requests
> 
> This patch adds a length validation check for CCA ECC private key requests in
> cca_ecc2protkey(). It ensures the key token length does not exceed the
> available space in the parameter block before proceeding with CPRB memory
> allocation.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index db6a4211a97e0..322677a8d3200 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
>> @@ -1428,6 +1428,9 @@ int cca_ecc2protkey(u16 cardnr, u16 domain, const u8 *key,
>>  	} __packed * prepparm;
>>  	int keylen = ((struct eccprivkeytoken *)key)->len;
>>  
>> +	if (keylen > PARMBSIZE - sizeof(struct aureqparm))
>> +		return -EINVAL;
>> +
>>  	/* get already prepared memory for 2 cprbs with param block each */
>>  	rc = alloc_and_prep_cprbmem(PARMBSIZE, &mem,
>>  				    &preqcblk, &prepcblk, xflags);
> 
> [Severity: High]
> This is a pre-existing issue, but since we are adding length validations in 
> this subsystem, is it possible for unprivileged users to trigger slab 
> out-of-bounds reads in other functions like cca_apqns4key(), 
> cca_verifykey(), and cca_key2protkey() in drivers/s390/crypto/pkey_cca.c?
> 
> If a user triggers an ioctl like PKEY_APQNS4K with an 8-byte buffer and sets 
> keylen=8, _copy_key_from_user() will allocate exactly 8 bytes. The code only 
> verifies that keylen is at least 8 bytes, but then casts the buffer to 
> larger structures.
> 
> In drivers/s390/crypto/pkey_cca.c:cca_apqns4key():
> 
>     if (flags & PKEY_FLAGS_MATCH_CUR_MKVP)
>         ptr_cur_mkvp = t->mkvp;
> 
> Could this access t->mkvp well beyond the 8-byte allocation?
> 
> Similarly, in drivers/s390/crypto/pkey_cca.c:cca_key2protkey():
> 
>     if (cca_check_secaescipherkey(pkey_dbf_info,
>                   3, key, 0, 1))
> 
> Does this pass the key buffer to a function that reads up to 60 bytes, 
> without verifying the key is actually large enough (since checking against 
> hdr->len could be bypassed if the user sets hdr->len = 8)?
> 
> And in drivers/s390/crypto/pkey_cca.c:cca_verifykey():
> 
>     *keybitsize = t->bitsize;
> 
> Does this access t->bitsize at offset 56 directly after only verifying the 
> 8-byte header?
> 
> [Severity: High]
> This is also a pre-existing issue, but I noticed there might be another spot 
> in zcrypt_ccamisc.c where we trust token lengths without sufficient bounds 
> checking. 
> 
> If a malfunctioning or compromised APQN crypto device returns a CPRB hardware 
> reply with a large t->len value, could this lead to an out-of-bounds read or 
> heap buffer overflow?
> 
> In drivers/s390/crypto/zcrypt_ccamisc.c:_ip_cprb_helper():
> 
>     memcpy(key_token, t, t->len);
> 
> Does this trust the t->len value from the hardware response blindly, 
> potentially causing an unbounded write to the destination buffer?
> 
> Also, in drivers/s390/crypto/zcrypt_ccamisc.c:cca_gencipherkey():
> 
>     if (*keybufsize >= t->len)
>         memcpy(keybuf, t, t->len);
> 
> Since *keybufsize is user-controlled (up to 8192) but the CPRB parameter 
> block backing t is strictly allocated to PARMBSIZE (512 bytes), could an 
> oversized t->len bypass this check? Would this cause memcpy() to read 
> beyond the 512-byte allocation and leak kernel memory back to userspace?
> 

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