Re: [PATCH v9 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code

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

Pre-existing issues:
- [High] Uninitialized kernel heap memory padding is leaked to the crypto hardware via the AP bus message buffer.
--

--- Patch [1]: [PATCH v9 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code ---
commit bbdecb3add414a50e3e90e0021b88d5f9c7bf242
Author: Harald Freudenberger <[email protected]>

    s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code
    
    Both CPRB alloc functions in zcrypt_ccamisc.c and zcrypt_ep11misc.c
    did not round up the memory allocation to a multiple of 4 bytes as it
    is needed by the zcrypt layer to process the CPRBs.
    
    Now the alloc_and_prep_cprbmem() and alloc_cprbmem() functions
    guarantee that the base CPRB struct and a possible parameter block are
    aligned to a 4-byte boundary and the backing memory allocation is
    rounded up to the next multiple of 4 byte. Also the free_cprbmem() is
    updated and scrubs the rounded up amount of memory.

> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index 84936a795b956..6819acd67c49b 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing how CPRB
requests are padded and aligned, I noticed a pre-existing issue in
drivers/s390/crypto/zcrypt_msgtype6.c.

drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6cprb_msgx() {
        ...
	/* prepare CPRB */
	if (z_copy_from_user(userspace, msg->userdata,
			     xcrb->request_control_blk_addr,
			     xcrb->request_control_blk_length))
		return -EFAULT;
        ...
}

Does this code leak uninitialized kernel heap memory padding to the crypto
hardware via the AP bus message buffer?

When a userspace application calls the zcrypt ioctl (e.g., ICARSAMODEXPO)
with an unaligned request_control_blk_length, the ap_msg->msg buffer is
allocated via kmalloc without __GFP_ZERO by ap_init_apmsg().

Since only the unaligned length is copied into the buffer, while the total
message length sent to the AP card uses the 4-byte aligned CEIL4() length,
could the unwritten padding bytes (up to 3 bytes) retain their previous
kernel heap contents and be transmitted to the hardware?


[Severity: High]
This is a pre-existing issue, but the same behavior appears in
xcrb_msg_to_type6_ep11cprb_msgx():

drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx() {
        ...
	/* Import CPRB data from the ioctl input parameter */
	if (z_copy_from_user(userspace, msg->userdata,
			     (char __force __user *)xcrb->req, xcrb->req_len)) {
		return -EFAULT;
	}
        ...
}

Can the unaligned request length copied here also leave padding bytes
uninitialized before being sent to the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1