Re: [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing upper bounds check on `prepparm->kb.tlv1.len` in `cca_gencipherkey` allows massive kernel memory leak.
- [High] Unvalidated array lengths in `cca_query_crypto_facility` can cause out-of-bounds pointer arithmetic and memory accesses.
--

--- Patch [1]: [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it ---
commit eab8ad18f4433da6c81f9cb5f87967a1dc92a89b
Author: Holger Dengler <dengler@linux.ibm.com>

    s390/zcrypt: Validate length in reply before using it

This patch addresses a potential buffer over-read and over-write in
_ip_cprb_helper() by validating the key token length in the CPRB reply
against the previously validated parameter block length.

> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index d4ce6352b5b2e..19909bf43dc91 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
>  
>  	/* do not check the key here, it may be incomplete */
>  
> -	/* copy the vlsc key token back */
> +	/*
> +	 * Copy the vlsc key token back.
> +	 * The available space in the destination (key_token) and the source
> +	 * (t) buffer is always larger as the valid range of prepparm->kb.len.
> +	 * Validate t->len by comparing it with the length information in the
> +	 * param block of the request (prepparm->kb.len)
> +	 * The value range of prepparm->kb.len has been checked above.
> +	 */
>  	t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
> +	if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) {
> +		ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n",
> +			       __func__, t->len);
> +		rc = -EIO;
> +		goto out;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but since we are fixing reply
length validations here, does cca_gencipherkey() have a similar missing
upper bounds check?

In drivers/s390/crypto/zcrypt_ccamisc.c:cca_gencipherkey():

	keybuflen = prepparm->kb.tlv1.len - 2 * sizeof(uint16_t);
	rc = cca_check_secaescipherkey(zcrypt_dbf_info, DBF_ERR,
				       prepparm->kb.tlv1.gen_key,
				       keybuflen, keybitsize, 1);
	// ...
	if (keybuf) {
		if (*keybufsize >= keybuflen)
			memcpy(keybuf, t, keybuflen);

The prepparm->kb.tlv1.len field comes from the device reply. It is checked for
a minimum bound, but it doesn't seem to be checked against the total parameter
block size. Could a corrupted or malfunctioning hardware crypto adapter (or a
spoofed virtual device) return a valid kb.len but a forged large tlv1.len?

If so, could this unbounded length drive an out-of-bounds memcpy() read from
the cprbmem allocation into keybuf, potentially leaking unrelated kernel data
to userspace?


[Severity: High]
This is a pre-existing issue, but I see another area where unvalidated
reply lengths might cause memory access issues.

In drivers/s390/crypto/zcrypt_ccamisc.c:cca_query_crypto_facility():

		ptr += len;
	}
	/* check and possible copy reply var array */
	len = *((u16 *)ptr);

The untrusted len is added directly to ptr without bounds validation. If a
malicious or malfunctioning crypto adapter returns a CPRB reply payload with
a very large len field (e.g., 65535) for the rule or variable array, wouldn't
the subsequent dereference read from an unvalidated memory location?

Could this out-of-bounds pointer arithmetic and memory access trigger a
kernel panic via a page fault on unmapped memory?

>  	memcpy(key_token, t, t->len);
>  	*key_token_size = t->len;
>

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