Re: [PATCH v11 5/5] s390/zcrypt: Pad trailing CCA or EP11 message with zeros

On 2026-08-03 10:49, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [Medium] The function `zcrypt_type6_crt_key()` leaves up to 7 bytes
> of padding uninitialized before sending the key token to the crypto
> card, leaking kernel memory.
> --
> 
> --- Patch [5]: [PATCH v11 5/5] s390/zcrypt: Pad trailing CCA or EP11
> message with zeros ---
> commit a6bb8fce2d346e386b1306d838ac36f1853d13f8
> Author: Harald Freudenberger <[email protected]>
> 
> s390/zcrypt: Pad trailing CCA or EP11 message with zeros
> 
> This commit pads the gap between the message copied from userspace and
> the message buffer sent to the CCA/EP11 hardware. This prevents
> uninitialized kernel memory from being forwarded to the crypto card.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c 
>> b/drivers/s390/crypto/zcrypt_msgtype6.c
>> index e79e68b4dd41d..3df1d676de5d8 100644
>> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
>> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
>> @@ -398,6 +398,10 @@ static int xcrb_msg_to_type6cprb_msgx(bool 
>> userspace, struct ap_message *ap_msg,
>>  			     xcrb->request_control_blk_addr,
>>  			     xcrb->request_control_blk_length))
>>  		return -EFAULT;
>> +	/* pad tail with 0 up to req_cblen */
>> +	if (xcrb->request_control_blk_length < req_cblen)
>> +		memset(msg->userdata + xcrb->request_control_blk_length,
>> +		       0, req_cblen - xcrb->request_control_blk_length);
> 
> [Severity: Medium]
> This is a pre-existing issue, but while reviewing this fix for 
> uninitialized
> memory leaks, I noticed a similar issue in zcrypt_type6_crt_key() where
> pad_len bytes appear to be left uninitialized:
> 
> drivers/s390/crypto/zcrypt_cca_key.h:zcrypt_type6_crt_key() {
>     ...
>     memset(key, 0, sizeof(*key));
>     ...
>     if (copy_from_user(key->key_parts, crt->np_prime, long_len) ||
>         copy_from_user(key->key_parts + long_len,
>                        crt->nq_prime, short_len) ||
>         copy_from_user(key->key_parts + long_len + short_len,
>                        crt->bp_key, long_len) ||
>         copy_from_user(key->key_parts + 2 * long_len + short_len,
>                        crt->bq_key, short_len) ||
>         copy_from_user(key->key_parts + 2 * long_len + 2 * short_len,
>                        crt->u_mult_inv, long_len))
>         return -EFAULT;
>     memset(key->key_parts + 3 * long_len + 2 * short_len + pad_len,
>            0xff, crt->inputdatalength);
>     ...
> }
> 
> Because sizeof(*key) doesn't include the flexible array member 
> key_parts[],
> the memory inside key_parts is uninitialized before the copies.
> 
> Does the final memset() skip over pad_len bytes by adding it to the 
> offset,
> leaving those padding bytes uninitialized before being sent to the 
> firmware?

Different issue - will be addressed with another patch.