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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but the patch completely misses identical logic flaws in the EP11 path (`xcrb_msg_to_type6_ep11cprb_msgx`). While the patch successfully fixes critical vulnerabilities in the CCA path, it leaves the kernel vulnerable to a catastrophic kernel panic (DoS), uninitialized kernel memory reads, and an uninitialized memory leak to hardware via the `ZSENDEP11CPRB` ioctl.
--

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

s390/zcrypt: Improve CCA CPRB length and overflow checks

This patch fixes integer overflows, minimum size validation, and potential
kernel memory leaks in xcrb_msg_to_type6cprb_msgx(). It adds U32_MAX
boundary checks, validates minimum control block sizes, and zeros trailing
bytes to prevent leaking uninitialized memory.

> 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;
> +	req_cblen = CEIL4((size_t)xcrb->request_control_blk_length);

[Severity: Critical]
This is a pre-existing issue, but does xcrb_msg_to_type6_ep11cprb_msgx()
need these same fixes?

It looks like the EP11 path has the same logic flaws that this patch fixes
for the CCA path. For example, if a large req_len is provided, couldn't the
length calculation wrap around and bypass the buffer size check?

drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx()
    ...
    ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);
    ...

If the buffer size check is bypassed, could z_copy_from_user() fail
access_ok() and zero out the destination buffer with a massive length,
crashing the kernel?

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)) {
    ...

Additionally, if req_len is very small, could this lead to uninitialized
memory reads when accessing msg->pld_lenfmt, since there doesn't appear to
be a minimum control block size check?

drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx()
    ...
    if ((msg->pld_lenfmt & 0x80) == 0x80) { /*ext.len.fmt 2 or 3*/
    ...

Finally, if req_len requires padding, does the trailing padding get zeroed
out, or could it leak uninitialized kernel heap memory to the hardware?

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