Re: [PATCH v1 1/1] s390/zcrypt: Fix and improve zcrypt reply message verification checks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The length validation for XCRB replies with a secondary data segment (`count2 > 0`) fails to verify that the primary segment's boundaries (`offset1 + count1`) fit within the message buffer, allowing an out-of-bounds read and uninitialized kernel heap memory disclosure to userspace.
--

commit e0149d402b0c0e1ac4ba64b32211aa5a4effb09f
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Fix and improve zcrypt reply message verification checks

This commit reworks length validation for crypto card replies in the
zcrypt device driver. It adds bounds checks to prevent out-of-bounds
reads when handling CCA, EP11, and ICA messages.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c

[ ... ]

> @@ -928,48 +935,82 @@ static void zcrypt_msgtype6_receive(struct ap_queue *aq,

[ ... ]

> +	case CEXXC_RESPONSE_TYPE_XCRB:
> +		len = (size_t)t86r->fmt2.offset1 + (size_t)t86r->fmt2.count1;
> +		if (t86r->fmt2.offset1 > reply->len ||
> +		    t86r->fmt2.count1 > reply->len) {
> +			msg->rc = -EMSGSIZE;
> +			pr_debug("offset1 %u count1 %u rpl.len %zu mismatch, msg.rc=%d\n",
> +				 t86r->fmt2.offset1, t86r->fmt2.count1,
> +				 reply->len, msg->rc);
> +			goto out;
> +		}
> +		if (t86r->fmt2.count2) {
> +			len = (size_t)t86r->fmt2.offset2 +
> +				(size_t)t86r->fmt2.count2;

[Severity: High]
This is a pre-existing issue, but does this overwrite of len bypass the
bounds checking for offset1 and count1 against reply->len?

When count2 is greater than 0, len is overwritten with the secondary
segment's length (offset2 + count2). Later in zcrypt_msgtype6_receive(),
the length validation check only verifies:

if (len != reply->len) {
        ...
}
if (len > reply->bufsize || len > msg->bufsize) {
        ...
}

Since len is now based entirely on offset2 and count2, the primary
segment's boundaries (offset1 + count1) are never verified to ensure
they actually fit within the message buffer (offset1 + count1 <= reply->len).

Could this lead to an out-of-bounds read later when convert_type86_xcrb()
handles the reply?

If a malformed reply has count2 > 0 but crafted offset1 and count1
values, convert_type86_xcrb() will use the unvalidated offset1 and count1
to copy kernel data to userspace:

drivers/s390/crypto/zcrypt_msgtype6.c:convert_type86_xcrb() {
        ...
        if (z_copy_to_user(userspace, xcrb->reply_control_blk_addr,
                           data + msg->fmt2.offset1, msg->fmt2.count1))
                return -EFAULT;
        ...
}

Since the message buffer is allocated without zeroing, could this copy
uninitialized heap memory past the end of the valid firmware reply
back to userspace?

> +			if (t86r->fmt2.offset2 > reply->len ||
> +			    t86r->fmt2.count2 > reply->len) {
>  				msg->rc = -EMSGSIZE;
> +				pr_debug("offset2 %u count2 %u rpl.len %zu mismatch, msg.rc=%d\n",
> +					 t86r->fmt2.offset2, t86r->fmt2.count2,
> +					 reply->len, msg->rc);
>  				goto out;
>  			}

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