Re: [PATCH v11 2/5] s390/zcrypt: Improve CCA CPRB length and overflow checks

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

New issues:
- [High] Out-of-bounds read into uninitialized padding due to relaxed bounds check.

Pre-existing issues:
- [High] Missing minimum size validation for EP11 requests allows uninitialized memory access.
--

--- Patch [2]: [PATCH v11 2/5] s390/zcrypt: Improve CCA CPRB length and overflow checks ---
commit 91475dced80fa0f4ff3795e01d4bba627b9d0bfe
Author: Harald Freudenberger <[email protected]>
s390/zcrypt: Improve CCA CPRB length and overflow checks

This commit improves length and overflow validations for CCA CPRB structures.
It updates calculations to use size_t, adds U32_MAX boundary checks, and
enforces minimum control block size requirements.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index 40f72cdf284d3..3e19e79d747cf 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c

[ ... ]

> @@ -404,8 +394,8 @@ 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;
> -	if (msg->cprbx.cprb_len + sizeof(msg->hdr.function_code) >
> -	    xcrb->request_control_blk_length)
> +	/* copy subfunction code into AP msg type 6 function code field */
> +	if (msg->cprbx.cprb_len > req_cblen - sizeof(msg->hdr.function_code))
>  		return -EINVAL;
>  	function_code = ((unsigned char *)&msg->cprbx) + msg->cprbx.cprb_len;
>  	memcpy(msg->hdr.function_code, function_code,

[Severity: High]
Does this change allow an out-of-bounds read into uninitialized padding?

The bounds check was relaxed from checking against
request_control_blk_length to checking against the 4-byte aligned req_cblen.

If a user supplies an unaligned xcrb->request_control_blk_length (e.g., 37)
and sets msg->cprbx.cprb_len to point into the uninitialized padding
(e.g., 38), it appears up to 2 bytes of uninitialized kernel heap memory can
be read into msg->hdr.function_code and subsequently sent to the
cryptographic hardware.

[Severity: High]
This is a pre-existing issue, but could we apply similar minimum size
validation to xcrb_msg_to_type6_ep11cprb_msgx() for EP11 requests?

When a user calls the zcrypt ioctl for an EP11 request with an undersized
xcrb->req_len (e.g., 0):

drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx() {
    ...
    if (z_copy_from_user(userspace, msg->userdata,
                         (char __force __user *)xcrb->req, xcrb->req_len)) {
    ...
}

User-supplied data is copied using the user-controlled length without
verifying it meets the minimum required size for the struct fields that
are subsequently accessed.

This allows uninitialized heap memory in ap_msg->msg to dictate control flow
and be sent to the cryptographic hardware when reading fields like
msg->pld_lenfmt or msg->cprbx.flags.

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