Re: [PATCH v5 2/2] s390/zcrypt: Improve EP11 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:
- [High] Padding bytes introduced by CEIL4 alignment are not zero-initialized, causing up to 3 bytes of uninitialized kernel heap memory to be leaked to the crypto hardware over the AP bus. This problem wasn't introduced by this patch, but it remains a security vulnerability.
--

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

s390/zcrypt: Improve EP11 CPRB length and overflow checks

The xcrb_msg_to_type6_ep11cprb_msgx() function lacks proper input
validation, creating security vulnerabilities. This patch fixes these by
using size_t for length calculations, adding U32_MAX boundary checks after
alignment, and validating minimum request size and minimum reply size
before copying from userspace.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index 2e4aef330b680..f7a504f5d54e3 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> @@ -477,16 +477,25 @@ static int xcrb_msg_to_type6_ep11cprb_msgx(bool userspace, struct ap_message *ap
[ ... ]
> +	req_len = CEIL4(xcrb->req_len);
> +	if (req_len < xcrb->req_len || req_len > U32_MAX)
> +		return -EINVAL;
> +	ap_msg->len = sizeof(struct type6_hdr) + req_len;
>  	if (ap_msg->len > ap_msg->bufsize)
>  		return -EINVAL;
>  
> -	if (CEIL4(xcrb->resp_len) < xcrb->resp_len)
> -		return -EINVAL; /* overflow after alignment*/
> +	/* reply length and overflow checks */
> +	if (xcrb->resp_len < sizeof(struct ep11_cprb))
> +		return -EINVAL;
> +	rep_len = CEIL4(xcrb->resp_len);
> +	if (rep_len < xcrb->resp_len || rep_len > U32_MAX)
> +		return -EINVAL;
>  
>  	/* prepare type6 header */
>  	msg->hdr = static_type6_ep11_hdr;

[Severity: High]
This is a pre-existing issue, but does this code leave up to 3 bytes of
uninitialized kernel memory in the message buffer when xcrb->req_len is not
4-byte aligned?

When copying the request data from userspace just below this code in
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;
	}

The copy only transfers xcrb->req_len bytes. Because the message length
dispatched to the hardware includes the padding bytes up to CEIL4(xcrb->req_len),
uninitialized memory could be leaked.

Should there be a memset to zero-initialize the remaining padding bytes before
the message is dispatched?

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