[PATCH v1 0/1] 390/zcrypt: Validate length information in reply

[PATCH v1 0/1] 390/zcrypt: Validate length information in reply

[Holger Dengler]

This series contains adds validation of a length information in a
hardware reply, to prevent buffer over-read/over-write in case of a
broken/modified replies.

This series applies on-top of Linus' master branch.

Holger Dengler (1):
  s390/zcrypt: Validate length in reply before using it

 drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)


base-commit: 59e6295fac26b8e85c1ea859cdd89fa1e47519d7
-- 
2.55.0

[PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it

[Holger Dengler]

The length information in the reply is used to copy the key token to
the target buffer. An invalid information in t->len of the reply may
cause an over-read of the target buffer and also a over-write of the
target buffer. To prevent that, check t->len before using it.

As the available space in destination and source buffer is always
larger than the valid length value in the parameter block in the
reply, compare t->len with this (already validated) length
information. As a side effect, this check also prevents buffer
over-read and over-write.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
---
 drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
index d4ce6352b5b2..54b8edd1dfc7 100644
--- a/drivers/s390/crypto/zcrypt_ccamisc.c
+++ b/drivers/s390/crypto/zcrypt_ccamisc.c
@@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
 
 	/* do not check the key here, it may be incomplete */
 
-	/* copy the vlsc key token back */
+	/*
+	 * Copy the vlsc key token back.
+	 * The available space in the destination (key_token) and the source
+	 * (t) buffer is always larger as the valid range of prepparm->kb.len.
+	 * Validate t->len ba comparing it with the length information in the
+	 * param block of the request (prepparm->kb.len)
+	 * The value range of prepparm->kb.len has been checked above.
+	 */
 	t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
+	if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) {
+		ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n",
+			       __func__, t->len);
+		rc = -EIO;
+		goto out;
+	}
 	memcpy(key_token, t, t->len);
 	*key_token_size = t->len;
 
-- 
2.55.0

Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes an out-of-bounds read in `_ip_cprb_helper()` but misses identical vulnerabilities in `cca_gencipherkey()` and `_ep11_genaeskey()`.
- [High] Unvalidated length fields in the CPRBX response payload are used to advance a memory pointer, causing severe out-of-bounds reads in `cca_query_crypto_facility()`.
--

--- Patch [1]: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it ---
commit 3fcaad86f04a2a9419691d5f1a54cc873f326675
Author: Holger Dengler <dengler@linux.ibm.com>

s390/zcrypt: Validate length in reply before using it

The length information in the reply is used to copy the key token to
the target buffer. An invalid information in t->len of the reply may
cause an over-read of the target buffer and also a over-write of the
target buffer. To prevent that, check t->len before using it.

> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index d4ce6352b5b2e..54b8edd1dfc74 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
>  
>  	/* do not check the key here, it may be incomplete */
>  
> -	/* copy the vlsc key token back */
> +	/*
> +	 * Copy the vlsc key token back.
> +	 * The available space in the destination (key_token) and the source
> +	 * (t) buffer is always larger as the valid range of prepparm->kb.len.
> +	 * Validate t->len ba comparing it with the length information in the
> +	 * param block of the request (prepparm->kb.len)
> +	 * The value range of prepparm->kb.len has been checked above.
> +	 */
>  	t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
> +	if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) {
> +		ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n",
> +			       __func__, t->len);
> +		rc = -EIO;
> +		goto out;
> +	}
>  	memcpy(key_token, t, t->len);
>  	*key_token_size = t->len;

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing the
unvalidated length field from the device reply here, should similar validation
be added to cca_gencipherkey() and _ep11_genaeskey()?

In drivers/s390/crypto/zcrypt_ccamisc.c:cca_gencipherkey(), the code reads
the generated token length directly from the untrusted reply and calculates
the copy length without checking bounds against the structure size:

    keybuflen = prepparm->kb.tlv1.len - 2 * sizeof(uint16_t);
    ...
    if (keybuf) {
        if (*keybufsize >= keybuflen)
            memcpy(keybuf, t, keybuflen);

Could a malicious or compromised device return an inflated length, causing
memcpy() to read thousands of bytes out-of-bounds from the cprbmem
allocation and potentially leak kernel memory to userspace?

A similar unvalidated copy appears in
drivers/s390/crypto/zcrypt_ep11misc.c:_ep11_genaeskey(), which relies entirely
on the untrusted device data length:

    memcpy(keybuf, rep_pl->data, rep_pl->data_len);

[Severity: High]
This is a pre-existing issue, but does a similar vulnerability exist in
drivers/s390/crypto/zcrypt_ccamisc.c:cca_query_crypto_facility() where the
unvalidated length field is used to advance a memory pointer?

In that function, the code blindly advances ptr using an unchecked length
provided by the device payload:

    len = *((u16 *)ptr);
    if (len > sizeof(u16)) {
        ...
        ptr += len;
    }

If the hardware returns a corrupt or malicious length, could ptr advance
far beyond the cprbmem buffer bounds, leading to an out-of-bounds dereference
on the next iteration or exposing arbitrary kernel memory during the
subsequent memcpy()?

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

Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it

[Harald Freudenberger]

On 2026-08-20 16:04, Holger Dengler wrote:
> The length information in the reply is used to copy the key token to
> the target buffer. An invalid information in t->len of the reply may
> cause an over-read of the target buffer and also a over-write of the
> target buffer. To prevent that, check t->len before using it.
> 
> As the available space in destination and source buffer is always
> larger than the valid length value in the parameter block in the
> reply, compare t->len with this (already validated) length
> information. As a side effect, this check also prevents buffer
> over-read and over-write.
> 
> Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
> ---
>  drivers/s390/crypto/zcrypt_ccamisc.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c
> b/drivers/s390/crypto/zcrypt_ccamisc.c
> index d4ce6352b5b2..54b8edd1dfc7 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 
> domain,
> 
>  	/* do not check the key here, it may be incomplete */
> 
> -	/* copy the vlsc key token back */
> +	/*
> +	 * Copy the vlsc key token back.
> +	 * The available space in the destination (key_token) and the source
> +	 * (t) buffer is always larger as the valid range of 
> prepparm->kb.len.
> +	 * Validate t->len ba comparing it with the length information in the

what is this funny "ba" in the previous sentence ?

> +	 * param block of the request (prepparm->kb.len)
> +	 * The value range of prepparm->kb.len has been checked above.
> +	 */
>  	t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
> +	if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) {
> +		ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n",
> +			       __func__, t->len);
> +		rc = -EIO;
> +		goto out;
> +	}
>  	memcpy(key_token, t, t->len);
>  	*key_token_size = t->len;

Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>

Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it

[Holger Dengler]

On 8/20/26 16:46, Harald Freudenberger wrote:
> On 2026-08-20 16:04, Holger Dengler wrote:
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c
>> b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index d4ce6352b5b2..54b8edd1dfc7 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
>> @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
>>
>>      /* do not check the key here, it may be incomplete */
>>
>> -    /* copy the vlsc key token back */
>> +    /*
>> +     * Copy the vlsc key token back.
>> +     * The available space in the destination (key_token) and the source
>> +     * (t) buffer is always larger as the valid range of prepparm-
>> >kb.len.
>> +     * Validate t->len ba comparing it with the length information in
>> the
> 
> what is this funny "ba" in the previous sentence ?

A typo. Should be "by". Will be fixed in v2.

> 
>> +     * param block of the request (prepparm->kb.len)
>> +     * The value range of prepparm->kb.len has been checked above.
>> +     */
>>      t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
>> +    if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) {
>> +        ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n",
>> +                   __func__, t->len);
>> +        rc = -EIO;
>> +        goto out;
>> +    }
>>      memcpy(key_token, t, t->len);
>>      *key_token_size = t->len;
> 
> Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>

Thx, I'll pick it for the v2.

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

Re: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it

[Holger Dengler]

On 8/20/26 16:20, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The patch fixes an out-of-bounds read in `_ip_cprb_helper()` but misses identical vulnerabilities in `cca_gencipherkey()` and `_ep11_genaeskey()`.
> - [High] Unvalidated length fields in the CPRBX response payload are used to advance a memory pointer, causing severe out-of-bounds reads in `cca_query_crypto_facility()`.
> --
> 
> --- Patch [1]: [PATCH v1 1/1] s390/zcrypt: Validate length in reply before using it ---
> commit 3fcaad86f04a2a9419691d5f1a54cc873f326675
> Author: Holger Dengler <dengler@linux.ibm.com>
> 
> s390/zcrypt: Validate length in reply before using it
> 
> The length information in the reply is used to copy the key token to
> the target buffer. An invalid information in t->len of the reply may
> cause an over-read of the target buffer and also a over-write of the
> target buffer. To prevent that, check t->len before using it.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index d4ce6352b5b2e..54b8edd1dfc74 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
>> @@ -1158,8 +1158,21 @@ static int _ip_cprb_helper(u16 cardnr, u16 domain,
>>  
>>  	/* do not check the key here, it may be incomplete */
>>  
>> -	/* copy the vlsc key token back */
>> +	/*
>> +	 * Copy the vlsc key token back.
>> +	 * The available space in the destination (key_token) and the source
>> +	 * (t) buffer is always larger as the valid range of prepparm->kb.len.
>> +	 * Validate t->len ba comparing it with the length information in the
>> +	 * param block of the request (prepparm->kb.len)
>> +	 * The value range of prepparm->kb.len has been checked above.
>> +	 */
>>  	t = (struct cipherkeytoken *)prepparm->kb.tlv1.key_token;
>> +	if (t->len != prepparm->kb.len - 3 * sizeof(uint16_t)) {
>> +		ZCRYPT_DBF_ERR("%s reply with invalid key_token length %u\n",
>> +			       __func__, t->len);
>> +		rc = -EIO;
>> +		goto out;
>> +	}
>>  	memcpy(key_token, t, t->len);
>>  	*key_token_size = t->len;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but since you are fixing the
> unvalidated length field from the device reply here, should similar validation
> be added to cca_gencipherkey() and _ep11_genaeskey()?
> 
> In drivers/s390/crypto/zcrypt_ccamisc.c:cca_gencipherkey(), the code reads
> the generated token length directly from the untrusted reply and calculates
> the copy length without checking bounds against the structure size:
> 
>     keybuflen = prepparm->kb.tlv1.len - 2 * sizeof(uint16_t);
>     ...
>     if (keybuf) {
>         if (*keybufsize >= keybuflen)
>             memcpy(keybuf, t, keybuflen);
> 
> Could a malicious or compromised device return an inflated length, causing
> memcpy() to read thousands of bytes out-of-bounds from the cprbmem
> allocation and potentially leak kernel memory to userspace?

The memcpy is only called, if keylen is less equal *keysize (which is
the available space in keybuf). As far as I can see, this prevents any
over-writes of the keybuf buffer, even with a malicious response from
the hardware.

This objection is not relevant.

> A similar unvalidated copy appears in
> drivers/s390/crypto/zcrypt_ep11misc.c:_ep11_genaeskey(), which relies entirely
> on the untrusted device data length:
> 
>     memcpy(keybuf, rep_pl->data, rep_pl->data_len);

In _ep11_genaeskey(), the rep_pl->data_len must be less equal
*keybufsize, otherwise the function returns with an error.

This objection is false.

> [Severity: High]
> This is a pre-existing issue, but does a similar vulnerability exist in
> drivers/s390/crypto/zcrypt_ccamisc.c:cca_query_crypto_facility() where the
> unvalidated length field is used to advance a memory pointer?
> 
> In that function, the code blindly advances ptr using an unchecked length
> provided by the device payload:
> 
>     len = *((u16 *)ptr);
>     if (len > sizeof(u16)) {
>         ...
>         ptr += len;
>     }
> 
> If the hardware returns a corrupt or malicious length, could ptr advance
> far beyond the cprbmem buffer bounds, leading to an out-of-bounds dereference
> on the next iteration or exposing arbitrary kernel memory during the
> subsequent memcpy()?

This objection is under investigation and will be fixed (if required) in
another series.

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