[PATCH v1 0/1] Fix missing mem scrub at clear key import in cca_clr2cipherkey()

[PATCH v1 0/1] Fix missing mem scrub at clear key import in cca_clr2cipherkey()

[Harald Freudenberger]

The helper function _ip_cprb_helper() uses internal buffer memory for
building and processing CPRBs. After use this buffer was never
scrubbed which could lead to leaving for example clear key material in
memory which could be exposed via tricky reuse of this same memory.

Extent the _ip_cprb_helper() function with another parameter 'scrub'
used to steer scrubbing of this buffer. So now the caller has the
opportunity to decide if scrubbing is needed or not.

Extent the clear key to secure key token import process in function
cca_clr2cipherkey() to tell the helper function from above to scrub
the cprb buffer when the clear key value is part of the request data.

Overall this cleans the internal used buffer in case of clear key
import to prevent sensitive data to get exposed.

Changelog:
v1: initial version

Harald Freudenberger (1):
  s390/zcrypt: Fix missing mem scrub at clear key import in
    cca_clr2cipherkey()

 drivers/s390/crypto/zcrypt_ccamisc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

--
2.43.0

[PATCH v1 1/1] s390/zcrypt: Fix missing mem scrub at clear key import in cca_clr2cipherkey()

[Harald Freudenberger]

The helper function _ip_cprb_helper() uses internal buffer memory for
building and processing CPRBs. After use this buffer was never
scrubbed which could lead to leaving for example clear key material in
memory which could be exposed via tricky reuse of this same memory.

Extent the _ip_cprb_helper() function with another parameter 'scrub'
used to steer scrubbing of this buffer. So now the caller has the
opportunity to decide if scrubbing is needed or not.

Extent the clear key to secure key token import process in function
cca_clr2cipherkey() to tell the helper function from above to scrub
the cprb buffer when the clear key value is part of the request data.

Overall this cleans the internal used buffer in case of clear key
import to prevent sensitive data to get exposed.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Cc: stable@vger.kernel.org
---
 drivers/s390/crypto/zcrypt_ccamisc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
index 84936a795b95..6a237e880b40 100644
--- a/drivers/s390/crypto/zcrypt_ccamisc.c
+++ b/drivers/s390/crypto/zcrypt_ccamisc.c
@@ -971,7 +971,8 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
 			   int clr_key_bit_size,
 			   u8 *key_token,
 			   int *key_token_size,
-			   u32 xflags)
+			   u32 xflags,
+			   bool scrub)
 {
 	int rc, n;
 	u8 *mem, *ptr;
@@ -1111,7 +1112,7 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
 	*key_token_size = t->len;
 
 out:
-	free_cprbmem(mem, PARMBSIZE, false, xflags);
+	free_cprbmem(mem, PARMBSIZE, scrub, xflags);
 	return rc;
 }
 
@@ -1162,28 +1163,32 @@ int cca_clr2cipherkey(u16 card, u16 dom, u32 keybitsize, u32 keygenflags,
 	 * 4/4 COMPLETE the secure cipher key import
 	 */
 	rc = _ip_cprb_helper(card, dom, "AES     ", "FIRST   ", "MIN3PART",
-			     exorbuf, keybitsize, token, &tokensize, xflags);
+			     exorbuf, keybitsize, token, &tokensize,
+			     xflags, false);
 	if (rc) {
 		ZCRYPT_DBF_ERR("%s clear key import 1/4 with CSNBKPI2 failed, rc=%d\n",
 			       __func__, rc);
 		goto out;
 	}
 	rc = _ip_cprb_helper(card, dom, "AES     ", "ADD-PART", NULL,
-			     clrkey, keybitsize, token, &tokensize, xflags);
+			     clrkey, keybitsize, token, &tokensize,
+			     xflags, true);
 	if (rc) {
 		ZCRYPT_DBF_ERR("%s clear key import 2/4 with CSNBKPI2 failed, rc=%d\n",
 			       __func__, rc);
 		goto out;
 	}
 	rc = _ip_cprb_helper(card, dom, "AES     ", "ADD-PART", NULL,
-			     exorbuf, keybitsize, token, &tokensize, xflags);
+			     exorbuf, keybitsize, token, &tokensize,
+			     xflags, false);
 	if (rc) {
 		ZCRYPT_DBF_ERR("%s clear key import 3/4 with CSNBKPI2 failed, rc=%d\n",
 			       __func__, rc);
 		goto out;
 	}
 	rc = _ip_cprb_helper(card, dom, "AES     ", "COMPLETE", NULL,
-			     NULL, keybitsize, token, &tokensize, xflags);
+			     NULL, keybitsize, token, &tokensize,
+			     xflags, false);
 	if (rc) {
 		ZCRYPT_DBF_ERR("%s clear key import 4/4 with CSNBKPI2 failed, rc=%d\n",
 			       __func__, rc);
-- 
2.43.0

Re: [PATCH v1 1/1] s390/zcrypt: Fix missing mem scrub at clear key import in cca_clr2cipherkey()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing bounds validation on the hardware-provided length field `t->len` leads to out-of-bounds reads and writes from the CPRB reply block.
--

--- Patch [1]: [PATCH v1 1/1] s390/zcrypt: Fix missing mem scrub at clear key import in cca_clr2cipherkey() ---
commit ca71a93310e3d552d6d70f4020264235acf62e73
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Fix missing mem scrub at clear key import in cca_clr2cipherkey()

This commit adds a scrub parameter to _ip_cprb_helper() to control whether
the internal CPRB buffer is zeroed out before freeing. This prevents clear
key material from being left in memory during key token import.

> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index 84936a795b956..6a237e880b40b 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> @@ -971,7 +971,8 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
>  			   int clr_key_bit_size,
>  			   u8 *key_token,
>  			   int *key_token_size,
> -			   u32 xflags)
> +			   u32 xflags,
> +			   bool scrub)
>  {

[ ... ]

> @@ -1111,7 +1112,7 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
>  	*key_token_size = t->len;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the
hardware-provided t->len field to cause an out-of-bounds read or write?

Looking at _ip_cprb_helper() in drivers/s390/crypto/zcrypt_ccamisc.c, the
t->len field is used directly from the untrusted hardware response:

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 a malfunctioning or compromised coprocessor returns a CPRB reply block
with a valid outer kb.len but a maliciously large inner t->len, could this
memcpy copy up to 64KB of adjacent kernel memory, overflowing the 16KB
token allocation in cca_clr2cipherkey()?

A similar pattern also appears to exist in cca_gencipherkey(). Would it be
safer to validate t->len against the source CPRB reply block size and the
destination buffer size before using it?

>  out:
> -	free_cprbmem(mem, PARMBSIZE, false, xflags);
> +	free_cprbmem(mem, PARMBSIZE, scrub, xflags);
>  	return rc;
>  }

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