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

[Harald Freudenberger <freude@linux.ibm.com>]

On 2026-09-10 18:35, sashiko-bot@kernel.org wrote:
> 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?
> 

Will fix this in the next version.

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

Does not apply. A valid RNG reply always need to be >= sizeof(struct 
type86x_reply)
and even more. However to access the fields cprbx.ccp_rtcode and 
cprbx.ccp_rscode in
function convert_type86_rng() this check is a precondition.

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