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

[sashiko-bot@kernel.org]

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729093616.4184642-1-dengler@linux.ibm.com?part=2