All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/1] s390/zcrypt: Validate length information in reply
@ 2026-08-20 15:50 Holger Dengler
  2026-08-20 15:50 ` [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler
  2026-08-23 21:21 ` [PATCH v2 0/1] s390/zcrypt: Validate length information in reply Vasily Gorbik
  0 siblings, 2 replies; 4+ messages in thread
From: Holger Dengler @ 2026-08-20 15:50 UTC (permalink / raw)
  To: Harald Freudenberger
  Cc: dengler, linux-s390, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Ingo Franzki

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it
  2026-08-20 15:50 [PATCH v2 0/1] s390/zcrypt: Validate length information in reply Holger Dengler
@ 2026-08-20 15:50 ` Holger Dengler
  2026-08-20 16:07   ` sashiko-bot
  2026-08-23 21:21 ` [PATCH v2 0/1] s390/zcrypt: Validate length information in reply Vasily Gorbik
  1 sibling, 1 reply; 4+ messages in thread
From: Holger Dengler @ 2026-08-20 15:50 UTC (permalink / raw)
  To: Harald Freudenberger
  Cc: dengler, linux-s390, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Ingo Franzki

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it
  2026-08-20 15:50 ` [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler
@ 2026-08-20 16:07   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-20 16:07 UTC (permalink / raw)
  To: Holger Dengler
  Cc: linux-s390, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Heiko Carstens

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 0/1] s390/zcrypt: Validate length information in reply
  2026-08-20 15:50 [PATCH v2 0/1] s390/zcrypt: Validate length information in reply Holger Dengler
  2026-08-20 15:50 ` [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler
@ 2026-08-23 21:21 ` Vasily Gorbik
  1 sibling, 0 replies; 4+ messages in thread
From: Vasily Gorbik @ 2026-08-23 21:21 UTC (permalink / raw)
  To: Holger Dengler
  Cc: Harald Freudenberger, linux-s390, Heiko Carstens,
	Alexander Gordeev, Ingo Franzki

On Thu, Aug 20, 2026 at 05:50:02PM +0200, Holger Dengler wrote:
> 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(-)

Applied, thank you!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-23 21:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 15:50 [PATCH v2 0/1] s390/zcrypt: Validate length information in reply Holger Dengler
2026-08-20 15:50 ` [PATCH v2 1/1] s390/zcrypt: Validate length in reply before using it Holger Dengler
2026-08-20 16:07   ` sashiko-bot
2026-08-23 21:21 ` [PATCH v2 0/1] s390/zcrypt: Validate length information in reply Vasily Gorbik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.