Re: [PATCH v1 1/1] s390/zcrypt: Improve length check and overflow detection at zcrypt msg type 6

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Heap buffer overflow in xcrb_msg_to_type6cprb_msgx() due to missing ap_msg->bufsize bound check.
--

--- Patch [1]: [PATCH v1 1/1] s390/zcrypt: Improve length check and overflow detection at zcrypt msg type 6 ---
commit 817ae0ed2183d762b91c6888629d3fa414135f83
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Improve length check and overflow detection at zcrypt msg type 6

This commit replaces manual overflow checks with check_add_overflow() and
changes some variable types to unsigned. It also aims to prevent buffer
overflow risks by improving message length calculations.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index 40f72cdf284d3..8bf735a643835 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> @@ -342,50 +342,49 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,

[ ... ]

>  	/* length checks */
> -	ap_msg->len = sizeof(struct type6_hdr) +
> -		CEIL4(xcrb->request_control_blk_length) +
> -		xcrb->request_data_length;
> -	if (ap_msg->len > ap_msg->bufsize)
> +	if (check_add_overflow(sizeof(struct type6_hdr), req_cblen, &u))
>  		return -EINVAL;
> +	if (check_add_overflow(u, xcrb->request_data_length, &u))
> +		return -EINVAL;
> +	ap_msg->len = u;

[Severity: Critical]
Does removing the check against ap_msg->bufsize allow an arbitrarily large
length to bypass validation?

If u is successfully calculated without integer overflow but is larger than
ap_msg->bufsize, it appears the subsequent calls to z_copy_from_user() could
write out of bounds into the ap_msg->msg heap buffer:

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;
	...
	/* copy data block */
	if (xcrb->request_data_length) {
		req_data = ap_msg->msg + sizeof(struct type6_hdr) + req_cblen;
		if (z_copy_from_user(userspace, req_data,
				     xcrb->request_data_address,
				     xcrb->request_data_length))
			return -EFAULT;
	}
}

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