[PATCH v2 0/2] Fix minor LLM findings in zcrypt dd

[PATCH v2 0/2] Fix minor LLM findings in zcrypt dd

[Harald Freudenberger]

Fix some minor LLM findings related to the zcrypt device driver.

Changelog:
v1: Two patches:
    - The first patch fixes a gap related to speculative execution
      with the domain index.
    - The second patch fixes a length/pointer check with parsing the
      FQ reply from a CCA card.
v2: Fix a complain from Sashiko about silently dropping invalid
    domains down to domain value 0. So now a explicit check makes
    sure domain is either AUTOSEL_DOM or in range 0...255.

Harald Freudenberger (2):
  s390/zcrypt: Guard domain index uses against speculative bypass
  s390/zcrypt: Fix out-of-bounds ptr advance in
    cca_query_crypto_facility()

 drivers/s390/crypto/zcrypt_api.c     | 20 ++++++++++++++++++--
 drivers/s390/crypto/zcrypt_ccamisc.c | 22 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)

--
2.43.0

[PATCH v2 1/2] s390/zcrypt: Guard domain index uses against speculative bypass

[Harald Freudenberger]

The previous array_index_nospec() placement only covered the admin
permission check, leaving ap_test_config_usage_domain() and
ap_test_config_ctrl_domain() in the CCA path exposed to speculative
execution.

Move the sanitization to before all permission and config checks in
both the CCA and EP11 paths so a single call covers all downstream
uses of the domain index. Before that check the domain value to be
either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 and return with -EINVAL
in case this check does not pass.

Fixes: e935cd525af4 ("s390/zcrypt: Close speculative mem read possibility")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Cc: stable@vger.kernel.org
---
 drivers/s390/crypto/zcrypt_api.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
index ec6a4c2f9f04..d2ff7ab54c8d 100644
--- a/drivers/s390/crypto/zcrypt_api.c
+++ b/drivers/s390/crypto/zcrypt_api.c
@@ -877,9 +877,17 @@ static long _zcrypt_send_cprb(u32 xflags, struct ap_perms *perms,
 	print_hex_dump_debug("ccareq: ", DUMP_PREFIX_ADDRESS, 16, 1,
 			     ap_msg.msg, ap_msg.len, false);
 
+	/* Make sure domain is either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
+	if (domain != AUTOSEL_DOM) {
+		if (domain >= AP_DOMAINS) {
+			rc = -EINVAL;
+			goto out;
+		}
+		domain = array_index_nospec(domain, AP_DOMAINS);
+	}
+
 	if (perms != &ap_perms && domain < AP_DOMAINS) {
 		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
-			domain = array_index_nospec(domain, AP_DOMAINS);
 			if (!test_bit_inv(domain, perms->adm)) {
 				rc = -ENODEV;
 				goto out;
@@ -1078,9 +1086,17 @@ static long _zcrypt_send_ep11_cprb(u32 xflags, struct ap_perms *perms,
 	print_hex_dump_debug("ep11req: ", DUMP_PREFIX_ADDRESS, 16, 1,
 			     ap_msg.msg, ap_msg.len, false);
 
+	/* Make sure domain is either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
+	if (domain != AUTOSEL_DOM) {
+		if (domain >= AP_DOMAINS) {
+			rc = -EINVAL;
+			goto out;
+		}
+		domain = array_index_nospec(domain, AP_DOMAINS);
+	}
+
 	if (perms != &ap_perms && domain < AP_DOMAINS) {
 		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
-			domain = array_index_nospec(domain, AP_DOMAINS);
 			if (!test_bit_inv(domain, perms->adm)) {
 				rc = -ENODEV;
 				goto out;
-- 
2.43.0

Re: [PATCH v2 1/2] s390/zcrypt: Guard domain index uses against speculative bypass

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/zcrypt: speculative bypass of array_index_nospec() in domain checks
--

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

s390/zcrypt: Guard domain index uses against speculative bypass

The previous array_index_nospec() placement only covered the admin
permission check, leaving ap_test_config_usage_domain() and
ap_test_config_ctrl_domain() in the CCA path exposed to speculative
execution. This commit moves the sanitization to before all permission
and config checks in both the CCA and EP11 paths.

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index ec6a4c2f9f04a..d2ff7ab54c8d5 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -877,9 +877,17 @@ static long _zcrypt_send_cprb(u32 xflags, struct ap_perms *perms,
>  	print_hex_dump_debug("ccareq: ", DUMP_PREFIX_ADDRESS, 16, 1,
>  			     ap_msg.msg, ap_msg.len, false);
>  
> +	/* Make sure domain is either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
> +	if (domain != AUTOSEL_DOM) {
> +		if (domain >= AP_DOMAINS) {
> +			rc = -EINVAL;
> +			goto out;
> +		}
> +		domain = array_index_nospec(domain, AP_DOMAINS);
> +	}

[Severity: High]
Could placing array_index_nospec() inside this conditional block lead to a
speculative execution bypass?

If the CPU's branch predictor is mistrained to assume domain == AUTOSEL_DOM,
the speculative execution could skip this block entirely. This would leave an
out-of-bounds domain completely unclamped.

> +
>  	if (perms != &ap_perms && domain < AP_DOMAINS) {
>  		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
> -			domain = array_index_nospec(domain, AP_DOMAINS);
>  			if (!test_bit_inv(domain, perms->adm)) {

[Severity: High]
Since the nospec clamping could be bypassed speculatively above, could this
block be entered with an out-of-bounds domain?

If the CPU then mispredicts the if (domain < AP_DOMAINS) check as true, does
this result in a speculative out-of-bounds read when test_bit_inv() is called
with the unclamped domain?

>  				rc = -ENODEV;
>  				goto out;

[ ... ]

> @@ -1078,9 +1086,17 @@ static long _zcrypt_send_ep11_cprb(u32 xflags, struct ap_perms *perms,
>  	print_hex_dump_debug("ep11req: ", DUMP_PREFIX_ADDRESS, 16, 1,
>  			     ap_msg.msg, ap_msg.len, false);
>  
> +	/* Make sure domain is either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
> +	if (domain != AUTOSEL_DOM) {
> +		if (domain >= AP_DOMAINS) {
> +			rc = -EINVAL;
> +			goto out;
> +		}
> +		domain = array_index_nospec(domain, AP_DOMAINS);
> +	}

[Severity: High]
Does this identical pattern in the EP11 path also expose the domain variable
to the same speculative execution bypass?

> +
>  	if (perms != &ap_perms && domain < AP_DOMAINS) {
>  		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
> -			domain = array_index_nospec(domain, AP_DOMAINS);
>  			if (!test_bit_inv(domain, perms->adm)) {

[Severity: High]
Does this result in the same speculative out-of-bounds read during the EP11
admin check, similar to the CCA path above?

>  				rc = -ENODEV;
>  				goto out;

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

[PATCH v2 2/2] s390/zcrypt: Fix out-of-bounds ptr advance in cca_query_crypto_facility()

[Harald Freudenberger]

The FQ reply parser code blindly advanced the walk pointer by a length
value read directly from the hardware reply payload without checking
that the advance stayed within the allocated reply buffer. A corrupt
or malicious device response could push ptr beyond the cprbmem region,
causing an out-of-bounds dereference or kernel memory exposure via the
subsequent memcpy().

Fix this by tracking the remaining reply buffer space in a variable
and validating each device-supplied length field against it before
advancing or dereferencing the pointer.

Fixes: 2004b57cde6b ("s390/zcrypt: code cleanup")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Cc: stable@vger.kernel.org
---
 drivers/s390/crypto/zcrypt_ccamisc.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
index 19909bf43dc9..862947431164 100644
--- a/drivers/s390/crypto/zcrypt_ccamisc.c
+++ b/drivers/s390/crypto/zcrypt_ccamisc.c
@@ -1627,6 +1627,7 @@ int cca_query_crypto_facility(u16 cardnr, u16 domain,
 		u8  subfunc_code[2];
 		u8  lvdata[];
 	} __packed * prepparm;
+	size_t datalen;
 
 	/* get already prepared memory for 2 cprbs with param block each */
 	rc = alloc_and_prep_cprbmem(parmbsize, &mem,
@@ -1673,27 +1674,44 @@ int cca_query_crypto_facility(u16 cardnr, u16 domain,
 	prepcblk->rpl_parmb = (u8 __user *)ptr;
 	prepparm = (struct fqrepparm *)ptr;
 	ptr = prepparm->lvdata;
+	datalen = parmbsize - 2 * sizeof(u8);
 
 	/* check and possibly copy reply rule array */
 	len = *((u16 *)ptr);
+	if (len > datalen) {
+		ZCRYPT_DBF_ERR("%s reply rule array len %u exceeds datalen %zu\n",
+			       __func__, len, datalen);
+		rc = -EIO;
+		goto out;
+	}
+	datalen -= sizeof(u16);
+	ptr += sizeof(u16);
 	if (len > sizeof(u16)) {
-		ptr += sizeof(u16);
 		len -= sizeof(u16);
 		if (rarray && rarraylen && *rarraylen > 0) {
 			*rarraylen = (len > *rarraylen ? *rarraylen : len);
 			memcpy(rarray, ptr, *rarraylen);
 		}
+		datalen -= len;
 		ptr += len;
 	}
 	/* check and possible copy reply var array */
 	len = *((u16 *)ptr);
+	if (len > datalen) {
+		ZCRYPT_DBF_ERR("%s reply var array len %u exceeds datalen %zu\n",
+			       __func__, len, datalen);
+		rc = -EIO;
+		goto out;
+	}
+	datalen -= sizeof(u16);
+	ptr += sizeof(u16);
 	if (len > sizeof(u16)) {
-		ptr += sizeof(u16);
 		len -= sizeof(u16);
 		if (varray && varraylen && *varraylen > 0) {
 			*varraylen = (len > *varraylen ? *varraylen : len);
 			memcpy(varray, ptr, *varraylen);
 		}
+		datalen -= len;
 		ptr += len;
 	}
 
-- 
2.43.0

Re: [PATCH v2 2/2] s390/zcrypt: Fix out-of-bounds ptr advance in cca_query_crypto_facility()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] crypto: zcrypt: OOB read and underflow in cca_query_crypto_facility()
--

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

s390/zcrypt: Fix out-of-bounds ptr advance in cca_query_crypto_facility()

The FQ reply parser code blindly advanced the walk pointer by a length
value read directly from the hardware reply payload without checking
that the advance stayed within the allocated reply buffer.

> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index 19909bf43dc91..862947431164f 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
[ ... ]
> @@ -1673,27 +1674,44 @@ int cca_query_crypto_facility(u16 cardnr, u16 domain,
>  	prepcblk->rpl_parmb = (u8 __user *)ptr;
>  	prepparm = (struct fqrepparm *)ptr;
>  	ptr = prepparm->lvdata;
> +	datalen = parmbsize - 2 * sizeof(u8);
>  
>  	/* check and possibly copy reply rule array */
>  	len = *((u16 *)ptr);
> +	if (len > datalen) {
> +		ZCRYPT_DBF_ERR("%s reply rule array len %u exceeds datalen %zu\n",
> +			       __func__, len, datalen);
> +		rc = -EIO;
> +		goto out;
> +	}
> +	datalen -= sizeof(u16);
> +	ptr += sizeof(u16);
>  	if (len > sizeof(u16)) {
> -		ptr += sizeof(u16);
>  		len -= sizeof(u16);
>  		if (rarray && rarraylen && *rarraylen > 0) {
>  			*rarraylen = (len > *rarraylen ? *rarraylen : len);
>  			memcpy(rarray, ptr, *rarraylen);
>  		}
> +		datalen -= len;
>  		ptr += len;
>  	}
>  	/* check and possible copy reply var array */
>  	len = *((u16 *)ptr);

[Severity: High]
Does this code introduce an out-of-bounds read and subsequent integer
underflow?

If a corrupt or malicious device response sets the first array length (len) to
exactly or almost exactly the remaining reply buffer space (datalen), the
first array parsing will advance ptr to the end of the buffer and reduce
datalen to 0 or 1.

When we reach the second array parsing here, a 16-bit length field is
unconditionally read from ptr before checking if there are at least 2 bytes
remaining in the buffer (datalen >= sizeof(u16)). This could read 2 bytes past
the end of the allocated buffer.

> +	if (len > datalen) {
> +		ZCRYPT_DBF_ERR("%s reply var array len %u exceeds datalen %zu\n",
> +			       __func__, len, datalen);
> +		rc = -EIO;
> +		goto out;
> +	}
> +	datalen -= sizeof(u16);

[Severity: High]
If the out-of-bounds read above results in a value smaller than or equal to
datalen (e.g., 0), the check (len > datalen) is bypassed, and the subtraction
here will blindly consume 2 bytes. This would cause an integer underflow if
datalen was 0 or 1, wrapping the tracking variable to a huge value.

> +	ptr += sizeof(u16);
>  	if (len > sizeof(u16)) {
> -		ptr += sizeof(u16);
>  		len -= sizeof(u16);
>  		if (varray && varraylen && *varraylen > 0) {
>  			*varraylen = (len > *varraylen ? *varraylen : len);
>  			memcpy(varray, ptr, *varraylen);
>  		}
> +		datalen -= len;
>  		ptr += len;
>  	}
>

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

Re: [PATCH v2 2/2] s390/zcrypt: Fix out-of-bounds ptr advance in cca_query_crypto_facility()

[Harald Freudenberger]

On 2026-10-06 16:22, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] crypto: zcrypt: OOB read and underflow in 
> cca_query_crypto_facility()
> --
> 
> commit 5de45449fe732b12f3310f3ce76a2abecc04ed97
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/zcrypt: Fix out-of-bounds ptr advance in 
> cca_query_crypto_facility()
> 
> The FQ reply parser code blindly advanced the walk pointer by a length
> value read directly from the hardware reply payload without checking
> that the advance stayed within the allocated reply buffer.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c 
>> b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index 19909bf43dc91..862947431164f 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> [ ... ]
>> @@ -1673,27 +1674,44 @@ int cca_query_crypto_facility(u16 cardnr, u16 
>> domain,
>>  	prepcblk->rpl_parmb = (u8 __user *)ptr;
>>  	prepparm = (struct fqrepparm *)ptr;
>>  	ptr = prepparm->lvdata;
>> +	datalen = parmbsize - 2 * sizeof(u8);
>> 
>>  	/* check and possibly copy reply rule array */
>>  	len = *((u16 *)ptr);
>> +	if (len > datalen) {
>> +		ZCRYPT_DBF_ERR("%s reply rule array len %u exceeds datalen %zu\n",
>> +			       __func__, len, datalen);
>> +		rc = -EIO;
>> +		goto out;
>> +	}
>> +	datalen -= sizeof(u16);
>> +	ptr += sizeof(u16);
>>  	if (len > sizeof(u16)) {
>> -		ptr += sizeof(u16);
>>  		len -= sizeof(u16);
>>  		if (rarray && rarraylen && *rarraylen > 0) {
>>  			*rarraylen = (len > *rarraylen ? *rarraylen : len);
>>  			memcpy(rarray, ptr, *rarraylen);
>>  		}
>> +		datalen -= len;
>>  		ptr += len;
>>  	}
>>  	/* check and possible copy reply var array */
>>  	len = *((u16 *)ptr);
> 
> [Severity: High]
> Does this code introduce an out-of-bounds read and subsequent integer
> underflow?
> 
> If a corrupt or malicious device response sets the first array length 
> (len) to
> exactly or almost exactly the remaining reply buffer space (datalen), 
> the
> first array parsing will advance ptr to the end of the buffer and 
> reduce
> datalen to 0 or 1.
> 
> When we reach the second array parsing here, a 16-bit length field is
> unconditionally read from ptr before checking if there are at least 2 
> bytes
> remaining in the buffer (datalen >= sizeof(u16)). This could read 2 
> bytes past
> the end of the allocated buffer.
> 
>> +	if (len > datalen) {
>> +		ZCRYPT_DBF_ERR("%s reply var array len %u exceeds datalen %zu\n",
>> +			       __func__, len, datalen);
>> +		rc = -EIO;
>> +		goto out;
>> +	}
>> +	datalen -= sizeof(u16);
> 
> [Severity: High]
> If the out-of-bounds read above results in a value smaller than or 
> equal to
> datalen (e.g., 0), the check (len > datalen) is bypassed, and the 
> subtraction
> here will blindly consume 2 bytes. This would cause an integer 
> underflow if
> datalen was 0 or 1, wrapping the tracking variable to a huge value.
> 
>> +	ptr += sizeof(u16);
>>  	if (len > sizeof(u16)) {
>> -		ptr += sizeof(u16);
>>  		len -= sizeof(u16);
>>  		if (varray && varraylen && *varraylen > 0) {
>>  			*varraylen = (len > *varraylen ? *varraylen : len);
>>  			memcpy(varray, ptr, *varraylen);
>>  		}
>> +		datalen -= len;
>>  		ptr += len;
>>  	}
>> 

Ok i see ... this needs further investigation as well.