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

[PATCH v3 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.
    2nd patch unchanged.
v3: Well this spectre thing points out to be really wired. So here is
    again a rework claiming it shall protect against speculative
    execution with respect to the domain value.
    2nd patch unchanged.

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     | 35 ++++++++++++++++++++--------
 drivers/s390/crypto/zcrypt_ccamisc.c | 22 +++++++++++++++--
 2 files changed, 45 insertions(+), 12 deletions(-)

--
2.43.0

[PATCH v3 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. Hold the sanitized domain value in
another variable so that the later use can choose to use either the
original value (subject to speculative execution) or the sanitized
value (which may in case of AUTOSEL_DOM reflect the wrong
value). 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 | 35 +++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
index ec6a4c2f9f04..625578b3fabe 100644
--- a/drivers/s390/crypto/zcrypt_api.c
+++ b/drivers/s390/crypto/zcrypt_api.c
@@ -854,7 +854,7 @@ static long _zcrypt_send_cprb(u32 xflags, struct ap_perms *perms,
 			      struct ica_xcRB *xcrb)
 {
 	bool userspace = xflags & ZCRYPT_XFLAG_USERSPACE;
-	unsigned int card, domain, func_code = 0;
+	unsigned int card, domain, _dom, func_code = 0;
 	unsigned int wgt = 0, pref_wgt = 0;
 	struct zcrypt_queue *zq, *pref_zq;
 	struct zcrypt_card *zc, *pref_zc;
@@ -877,10 +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);
 
+	/* Check domain for either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
+	if (domain != AUTOSEL_DOM && domain >= AP_DOMAINS) {
+		rc = -EINVAL;
+		goto out;
+	}
+	/* Spectre-v1: sanitize domain unconditionally for later use */
+	_dom = 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)) {
+			if (!test_bit_inv(_dom, perms->adm)) {
 				rc = -ENODEV;
 				goto out;
 			}
@@ -893,10 +900,11 @@ static long _zcrypt_send_cprb(u32 xflags, struct ap_perms *perms,
 	 * If a valid target domain is set and this domain is NOT a usage
 	 * domain but a control only domain, autoselect target domain.
 	 */
-	if (domain < AP_DOMAINS &&
-	    !ap_test_config_usage_domain(domain) &&
-	    ap_test_config_ctrl_domain(domain))
-		domain = AUTOSEL_DOM;
+	if (domain < AP_DOMAINS) {
+		if (!ap_test_config_usage_domain(_dom) &&
+		    ap_test_config_ctrl_domain(_dom))
+			domain = AUTOSEL_DOM;
+	}
 
 	pref_zc = NULL;
 	pref_zq = NULL;
@@ -1041,7 +1049,7 @@ static long _zcrypt_send_ep11_cprb(u32 xflags, struct ap_perms *perms,
 	struct ep11_target_dev *targets = NULL;
 	unsigned short target_num;
 	unsigned int wgt = 0, pref_wgt = 0;
-	unsigned int func_code = 0, domain;
+	unsigned int func_code = 0, domain, _dom;
 	struct ap_message ap_msg;
 	int cpen, qpen, qid = 0, rc;
 	struct module *mod;
@@ -1078,10 +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);
 
+	/* Check domain for either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
+	if (domain != AUTOSEL_DOM && domain >= AP_DOMAINS) {
+		rc = -EINVAL;
+		goto out;
+	}
+	/* Spectre-v1: sanitize domain unconditionally for later use */
+	_dom = 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)) {
+			if (!test_bit_inv(_dom, perms->adm)) {
 				rc = -ENODEV;
 				goto out;
 			}
-- 
2.43.0

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

[sashiko-bot]

> 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. Hold the sanitized domain value in
> another variable so that the later use can choose to use either the
> original value (subject to speculative execution) or the sanitized
> value (which may in case of AUTOSEL_DOM reflect the wrong
> value). 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>

Sashiko has reviewed this patch and found no issues. It looks great!

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

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

[Holger Dengler]

On 10/6/26 17:46, Harald Freudenberger wrote:
> 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. Hold the sanitized domain value in
> another variable so that the later use can choose to use either the
> original value (subject to speculative execution) or the sanitized
> value (which may in case of AUTOSEL_DOM reflect the wrong
> value). 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

Reviewed-by: Holger Dengler <dengler@linux.ibm.com>

> ---
>  drivers/s390/crypto/zcrypt_api.c | 35 +++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index ec6a4c2f9f04..625578b3fabe 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
[...]> @@ -877,10 +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);
>  
> +	/* Check domain for either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */

typo in comment: AP_DOMAIN-1 --> AP_DOMAINS-1 (missing S)

[...]
> @@ -1078,10 +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);
>  
> +	/* Check domain for either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */

Same here.
typo: AP_DOMAIN-1 --> AP_DOMAINS-1 (missing S)

[...]

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler

[PATCH v3 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 v3 2/2] s390/zcrypt: Fix out-of-bounds ptr advance in cca_query_crypto_facility()

[sashiko-bot]

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

Sashiko has reviewed this patch and found no issues. It looks great!

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

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

[Holger Dengler]

On 10/6/26 17:46, Harald Freudenberger wrote:
> 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

Reviewed-by: Holger Dengler <dengler@linux.ibm.com>

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler

Re: [PATCH v3 0/2] Fix minor LLM findings in zcrypt dd

[Heiko Carstens]

On Tue, Oct 06, 2026 at 05:46:01PM +0200, Harald Freudenberger wrote:
> 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.
>     2nd patch unchanged.
> v3: Well this spectre thing points out to be really wired. So here is
>     again a rework claiming it shall protect against speculative
>     execution with respect to the domain value.
>     2nd patch unchanged.
> 
> 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     | 35 ++++++++++++++++++++--------
>  drivers/s390/crypto/zcrypt_ccamisc.c | 22 +++++++++++++++--
>  2 files changed, 45 insertions(+), 12 deletions(-)

Fixed typed reported by Holger, and applied. Thanks!

Re: [PATCH v3 0/2] Fix minor LLM findings in zcrypt dd

[Heiko Carstens]

On Thu, Oct 08, 2026 at 08:02:25PM +0200, Heiko Carstens wrote:
> On Tue, Oct 06, 2026 at 05:46:01PM +0200, Harald Freudenberger wrote:
> > 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.
> >     2nd patch unchanged.
> > v3: Well this spectre thing points out to be really wired. So here is
> >     again a rework claiming it shall protect against speculative
> >     execution with respect to the domain value.
> >     2nd patch unchanged.
> > 
> > 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     | 35 ++++++++++++++++++++--------
> >  drivers/s390/crypto/zcrypt_ccamisc.c | 22 +++++++++++++++--
> >  2 files changed, 45 insertions(+), 12 deletions(-)
> 
> Fixed typed reported by Holger, and applied. Thanks!
        ^^^^^
typos... :)