Re: [PATCH v1 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] 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