[PATCH v2 0/1] s390/zcrypt: Validate length information in reply

[PATCH v2 0/1] s390/zcrypt: Validate length information in reply

[Holger Dengler]

This series contains adds validation of a length information in a
hardware reply, to prevent buffer over-read/over-write in case of a
broken/modified replies.

Changes since v1:
- fix subject line in cover-letter
- fix typo
- pick R-b

This series applies on-top of Linus' master branch.

Holger Dengler (1):
  s390/zcrypt: Validate length in reply before using it

 drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)


base-commit: 59e6295fac26b8e85c1ea859cdd89fa1e47519d7
-- 
2.55.0

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

[Holger Dengler]

The length information in the reply is used to copy the key token to
the target buffer. An invalid information in t->len of the reply may
cause an over-read of the target buffer and also a over-write of the
target buffer. To prevent that, check t->len before using it.

As the available space in destination and source buffer is always
larger than the valid length value in the parameter block in the
reply, compare t->len with this (already validated) length
information. As a side effect, this check also prevents buffer
over-read and over-write.

Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
---
 drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
index d4ce6352b5b2..19909bf43dc9 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;
+	}
 	memcpy(key_token, t, t->len);
 	*key_token_size = t->len;
 
-- 
2.55.0

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

[sashiko-bot]

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