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

[sashiko-bot@kernel.org]

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 <dengler@linux.ibm.com>

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);

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