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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unvalidated length field `t->len` from an untrusted hardware response leads to severe out-of-bounds memory accesses during `memcpy()`.
--

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

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

This commit adds a parameter to _ip_cprb_helper() to scrub the CPRB buffer 
after processing clear key material. It also adds explicit zeroing of the 
buffers before freeing them in cca_clr2cipherkey() to prevent sensitive 
data exposure.

> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index 84936a795b956..9cf268efd4abf 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
[ ... ]
> @@ -1111,7 +1112,7 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
>  	*key_token_size = t->len;

[Severity: High]
This is a pre-existing issue, but is it possible for a compromised or
malfunctioning hardware device to return a maliciously crafted response
length in t->len?

In _ip_cprb_helper(), the unvalidated t->len from the hardware response is
directly used as the length for copying just before this line:

	t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
	memcpy(key_token, t, t->len);
	*key_token_size = t->len;

Without bounds checking against the actual buffer size, could this lead to an
out-of-bounds read from the device response buffer and an out-of-bounds write
into the key_token buffer?

Also, on subsequent iterations in cca_clr2cipherkey(), the poisoned
*key_token_size is used to construct the next request:

	preq_key_block->tlv1.len = sizeof(preq_key_block->tlv1) + n;
	preq_key_block->tlv1.flag = 0x0030;
	memcpy(preq_key_block->tlv1.key_token, key_token, *key_token_size);

Does this cause another unbounded copy into the smaller parameter buffer during
the next loop iteration?

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

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