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

On 2026-07-29 16:12, [email protected] wrote:
> 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 <[email protected]>
> 
> 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;
>>  }

These issues will be addressed with another patch (series)