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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Integer overflow in EP11 length calculation bypasses bounds check, leading to DoS.
- [High] Missing minimum size validation and uninitialized padding memory leak in EP11 message processing.
--

--- Patch [1]: [PATCH v5 1/2] s390/zcrypt: Improve CCA CPRB length and overflow checks ---
commit db060c7765ef46ae0c3f096c8f02b7e395cb8a36
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Improve CCA CPRB length and overflow checks

This commit fixes several security vulnerabilities in the CCA CPRB message
processing function by adding minimum size validation, fixing integer
overflows during alignment, and preventing memory leaks by zeroing trailing
bytes.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index 40f72cdf284d3..2e4aef330b680 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> @@ -342,49 +342,40 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
>  		};
>  	} __packed * msg = ap_msg->msg;
>  
> -	int rcblen = CEIL4(xcrb->request_control_blk_length);
> -	int req_sumlen, resp_sumlen;
> -	char *req_data = ap_msg->msg + sizeof(struct type6_hdr) + rcblen;
> -	char *function_code;
> +	size_t req_cblen, rep_cblen, req_sumlen, rep_sumlen;
> +	char *function_code, *req_data;
>  
> -	if (CEIL4(xcrb->request_control_blk_length) <
> -			xcrb->request_control_blk_length)
> -		return -EINVAL; /* overflow after alignment*/
> -
> -	/* length checks */
> +	/* request length and overflow checks */
> +	if (xcrb->request_control_blk_length < sizeof(struct CPRBX))
> +		return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but does the same missing size
validation issue exist in the EP11 counterpart
xcrb_msg_to_type6_ep11cprb_msgx() in the same file?

In xcrb_msg_to_type6_ep11cprb_msgx():

    ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);

Without a minimum size check on xcrb->req_len (e.g., against
sizeof(struct ep11_cprb)), could this parse uninitialized kernel memory from
ap_init_apmsg() if userspace passes a very small length via the
ZSENDEP11CPRB ioctl?

Later in the same function:

    if (msg->cprbx.flags & 0x80)
            ap_msg->flags |= AP_MSG_FLAG_ADMIN;

Could this uninitialized memory dictate administration privileges, or leak
uninitialized padding to the hardware?

> +	req_cblen = CEIL4((size_t)xcrb->request_control_blk_length);
> +	if (req_cblen > U32_MAX)
> +		return -EINVAL;
>  	ap_msg->len = sizeof(struct type6_hdr) +
> -		CEIL4(xcrb->request_control_blk_length) +
> -		xcrb->request_data_length;
> +		req_cblen + xcrb->request_data_length;
>  	if (ap_msg->len > ap_msg->bufsize)
>  		return -EINVAL;

[Severity: Critical]
This is a pre-existing issue, but could the lack of an overflow check in
xcrb_msg_to_type6_ep11cprb_msgx() lead to a denial of service?

In xcrb_msg_to_type6_ep11cprb_msgx(), the length calculation lacks similar
boundary checks:

    ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);
    if (ap_msg->len > ap_msg->bufsize)
            return -EINVAL;

If a local user supplies a massive value for xcrb->req_len
(e.g., 0xFFFFFFFFFFFFFFF0), could CEIL4() wrap around a 64-bit boundary,
resulting in a small length that bypasses the bufsize check?

If bypassed, it would reach the copy operation:

    if (z_copy_from_user(userspace, msg->userdata,
                         (char __force __user *)xcrb->req, xcrb->req_len)) {

When copy_from_user() fails its access_ok check with this massive length,
would it execute memset(..., 0, 0xFFFFFFFFFFFFFFF0), causing an
out-of-bounds write that immediately panics the kernel?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724151616.147765-1-freude@linux.ibm.com?part=1