Re: [PATCH v3 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:
- [High] Legitimate XCRB and RNG responses will be spuriously rejected due to enforcing an incorrect minimum length of sizeof(struct type86x_reply).
--

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

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

This commit adds and improves checks related to buffer sizes and reply sizes
for CCA, EP11, and ICA messages in the zcrypt driver. It adds length
validation before accessing reply message structures to prevent out-of-bounds
reads. It also reworks payload evaluation functions.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index 3df1d676de5d8..a7d392272cfa2 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
[ ... ]
> @@ -820,6 +833,8 @@ static int convert_response_xcrb(bool userspace, struct zcrypt_queue *zq,
>  {
>  	struct type86x_reply *msg = reply->msg;
>  
> +	/* reply->len is always >= sizeof(struct error_hdr) here */
> +
>  	switch (msg->hdr.type) {
>  	case TYPE82_RSP_CODE:
>  	case TYPE88_RSP_CODE:
> @@ -827,9 +842,14 @@ static int convert_response_xcrb(bool userspace, struct zcrypt_queue *zq,
>  		return convert_error(zq, reply);
>  	case TYPE86_RSP_CODE:
>  		if (msg->hdr.reply_code) {
> -			xcrb->status = msg->fmt2.apfs;
> +			if (reply->len < sizeof(struct type86_fmt2_msg))
> +				xcrb->status = 0x0008044DL;
> +			else
> +				xcrb->status = msg->fmt2.apfs;
>  			return convert_error(zq, reply);
>  		}
> +		if (reply->len < sizeof(struct type86x_reply))
> +			return -EINVAL;

[Severity: High]
Does this code incorrectly reject legitimate XCRB responses?

It appears that this check assumes XCRB replies use the large ICA-specific
type86x_reply struct. If a userspace application sends a valid XCRB request
without a data payload, the hardware replies successfully without padding.

While zcrypt_msgtype6_receive() correctly computes len as offset1 + count1
and verifies it exactly matches the hardware reply length, this check evaluates
reply->len < sizeof(struct type86x_reply). Since type86x_reply includes 6
additional bytes (pad[4] and length) intended only for ICA responses, its size
is larger than an empty-payload XCRB response.

Could this result in valid XCRB responses being rejected with -EINVAL,
causing a regression for userspace applications using the ICAXCRB ioctl?

>  		if (msg->cprbx.cprb_ver_id == 0x02)
>  			return convert_type86_xcrb(userspace, zq, reply, xcrb);
>  		fallthrough;	/* wrong cprb version is an unknown response */
[ ... ]
> @@ -885,6 +909,8 @@ static int convert_response_rng(struct zcrypt_queue *zq,
>  {
>  	struct type86x_reply *msg = reply->msg;
>  
> +	/* reply->len is always >= sizeof(struct error_hdr) here */
> +
>  	switch (msg->hdr.type) {
>  	case TYPE82_RSP_CODE:
>  	case TYPE88_RSP_CODE:
> @@ -892,6 +918,8 @@ static int convert_response_rng(struct zcrypt_queue *zq,
>  	case TYPE86_RSP_CODE:
>  		if (msg->hdr.reply_code)
>  			return -EINVAL;
> +		if (reply->len < sizeof(struct type86x_reply))
> +			return -EINVAL;

[Severity: High]
Does this check similarly apply a logically flawed minimum-size limit to RNG
hardware responses?

It seems this could fail if the hardware RNG response buffer size
configuration ever drops below the 6-byte delta caused by the ICA-specific
padding in type86x_reply.

>  		if (msg->cprbx.cprb_ver_id == 0x02)
>  			return convert_type86_rng(zq, reply, data);
>  		fallthrough;	/* wrong cprb version is an unknown response */

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