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

[sashiko-bot@kernel.org]

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

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?

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