[PATCH v8 0/5] Improve code in zcrypt msg type 6 handling

[PATCH v8 0/5] Improve code in zcrypt msg type 6 handling

[Harald Freudenberger]

Rework the limit and overflow checks in the both functions
xcrb_msg_to_type6_ep11cprb_msgx() and xcrb_msg_to_type6cprb_msgx().
And more ... all related to the length checks done in these both
functions.

Changelog:

v1: initial version, one patch dealing with overflow checks around
    the use of the CEIL4 macro in zcrypt_msgtype6.c
v2: Sashiko found a regression: The check
      if (ap_msg->len > ap_msg->bufsize)
    was missing in the rework. Added again this important check.
v3: - Split into two patches  - one for CCA one for EP11
    - rework upper limit/overflow check 
    - add lower limit check
v4: - tried to address all the remaining complains from sashiko.
v5: - Yea, sashiko still found out that the min req size needs
      to take the minimal payload header into account. Fixed.
v6: - And finally add padding zeros when there is a gap between
      user space provided message size and rounded message size
      in kernel.
v7: - As requested by Holger, the padding code is now an own patch.
    - As the length checks are now so strict the allocation in the
      both misc files needed adaption to correctly allocate 4 byte
      rounded up message buffers.
    - The EP11 length check has been found to be a mess. The payload
      is asn1 encoded and now the length parses parts of this payload
      to extract the needed fields (function value).
    - Furthermore another function zcrypt_msgtype6_send_ep11_cprb()
      also needed to be adjusted to parse the asn1 payload to patch
      the domain value into the payload.
v8: - Sashiko found out that the cprb free with memory scrub also
      needs adaption when the allocation rounds up to a 4 byte
      boundary. So fixed just this in the first patch of the series. 

Harald Freudenberger (5):
  s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code
  s390/zcrypt: Improve CCA CPRB length and overflow checks
  s390/zcrypt: Improve EP11 CPRB length and overflow checks
  s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing
  s390/zcrypt: Pad trailing CCA or EP11 message with zeros

 drivers/s390/crypto/zcrypt_ccamisc.c  |  20 +-
 drivers/s390/crypto/zcrypt_ep11misc.c |  12 +-
 drivers/s390/crypto/zcrypt_msgtype6.c | 357 ++++++++++++++++++--------
 3 files changed, 267 insertions(+), 122 deletions(-)

--
2.43.0

[PATCH v8 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code

[Harald Freudenberger]

Both CPRB alloc functions in zcrypt_ccamisc.c and zcrypt_ep11misc.c
did not round up the memory allocation to a multiple of 4 bytes as it
is needed by the zcrypt layer to process the CPRBs.

Now the alloc_and_prep_cprbmem() and alloc_cprbmem() functions
guarantee that the base CPRB struct and a possible parameter block are
aligned to a 4-byte boundary and the backing memory allocation is
rounded up to the next multiple of 4 byte. Also the free_cprbmem() is
updated and scrubs the rounded up amount of memory.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/s390/crypto/zcrypt_ccamisc.c  | 20 ++++++++++++++------
 drivers/s390/crypto/zcrypt_ep11misc.c | 12 ++++++++----
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
index 84936a795b95..6819acd67c49 100644
--- a/drivers/s390/crypto/zcrypt_ccamisc.c
+++ b/drivers/s390/crypto/zcrypt_ccamisc.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/random.h>
+#include <linux/align.h>
 #include <asm/zcrypt.h>
 #include <asm/pkey.h>
 
@@ -231,6 +232,10 @@ EXPORT_SYMBOL(cca_check_sececckeytoken);
  * block, reply CPRB and reply param block and fill in values
  * for the common fields. Returns 0 on success or errno value
  * on failure.
+ * It is guaranteed that request and a possible param block
+ * are aligned to a 4 byte boundary. Furthermore if a param
+ * block is used, the memory allocated for this is rounded up to
+ * the next multiple of 4 bytes.
  */
 static int alloc_and_prep_cprbmem(size_t paramblen,
 				  u8 **p_cprb_mem,
@@ -239,7 +244,8 @@ static int alloc_and_prep_cprbmem(size_t paramblen,
 				  u32 xflags)
 {
 	u8 *cprbmem = NULL;
-	size_t cprbplusparamblen = sizeof(struct CPRBX) + paramblen;
+	size_t cprbplusparamblen =
+		ALIGN(sizeof(struct CPRBX), 4) + ALIGN(paramblen, 4);
 	size_t len = 2 * cprbplusparamblen;
 	struct CPRBX *preqcblk, *prepcblk;
 
@@ -266,10 +272,10 @@ static int alloc_and_prep_cprbmem(size_t paramblen,
 	memcpy(preqcblk->func_id, "T2", 2);
 	preqcblk->rpl_msgbl = cprbplusparamblen;
 	if (paramblen) {
-		preqcblk->req_parmb =
-			((u8 __user *)preqcblk) + sizeof(struct CPRBX);
-		preqcblk->rpl_parmb =
-			((u8 __user *)prepcblk) + sizeof(struct CPRBX);
+		preqcblk->req_parmb = ((u8 __user *)preqcblk) +
+			ALIGN(sizeof(struct CPRBX), 4);
+		preqcblk->rpl_parmb = ((u8 __user *)prepcblk) +
+			ALIGN(sizeof(struct CPRBX), 4);
 	}
 
 	*p_cprb_mem = cprbmem;
@@ -287,8 +293,10 @@ static int alloc_and_prep_cprbmem(size_t paramblen,
  */
 static void free_cprbmem(void *mem, size_t paramblen, bool scrub, u32 xflags)
 {
+	size_t cprblen = ALIGN(sizeof(struct CPRBX), 4) + ALIGN(paramblen, 4);
+
 	if (mem && scrub)
-		memzero_explicit(mem, 2 * (sizeof(struct CPRBX) + paramblen));
+		memzero_explicit(mem, 2 * cprblen);
 
 	if (xflags & ZCRYPT_XFLAG_NOMEMALLOC)
 		mempool_free(mem, cprb_mempool);
diff --git a/drivers/s390/crypto/zcrypt_ep11misc.c b/drivers/s390/crypto/zcrypt_ep11misc.c
index 3dda9589f2b9..e604c100ccce 100644
--- a/drivers/s390/crypto/zcrypt_ep11misc.c
+++ b/drivers/s390/crypto/zcrypt_ep11misc.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/random.h>
 #include <linux/slab.h>
+#include <linux/align.h>
 #include <asm/zcrypt.h>
 #include <asm/pkey.h>
 #include <crypto/aes.h>
@@ -355,21 +356,24 @@ EXPORT_SYMBOL(ep11_check_aes_key);
 
 /*
  * Allocate and prepare ep11 cprb plus additional payload.
+ * It is guaranteed that the memory is aligned to a 4 byte boundary.
+ * Furthermore the memory allocation is rounded up to the next
+ * multiple of 4 bytes (with taking the payload_len into account).
  */
 static void *alloc_cprbmem(size_t payload_len, u32 xflags)
 {
-	size_t len = sizeof(struct ep11_cprb) + payload_len;
+	size_t memlen = ALIGN(sizeof(struct ep11_cprb) + payload_len, 4);
 	struct ep11_cprb *cprb = NULL;
 
 	if (xflags & ZCRYPT_XFLAG_NOMEMALLOC) {
-		if (len <= CPRB_MEMPOOL_ITEM_SIZE)
+		if (memlen <= CPRB_MEMPOOL_ITEM_SIZE)
 			cprb = mempool_alloc_preallocated(cprb_mempool);
 	} else {
-		cprb = kmalloc(len, GFP_KERNEL);
+		cprb = kmalloc(memlen, GFP_KERNEL);
 	}
 	if (!cprb)
 		return NULL;
-	memset(cprb, 0, len);
+	memset(cprb, 0, memlen);
 
 	cprb->cprb_len = sizeof(struct ep11_cprb);
 	cprb->cprb_ver_id = 0x04;
-- 
2.43.0

Re: [PATCH v8 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete memory scrubbing in `free_cprbmem()` due to missing 4-byte alignment rounding.
--

--- Patch [1]: [PATCH v8 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code ---
commit 8719946bc97657e1ca3c996d1d00474456aa0ef0
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code
    
Both CPRB alloc functions in zcrypt_ccamisc.c and zcrypt_ep11misc.c
did not round up the memory allocation to a multiple of 4 bytes as it
is needed by the zcrypt layer to process the CPRBs.

Now the alloc_and_prep_cprbmem() and alloc_cprbmem() functions
guarantee that the base CPRB struct and a possible parameter block are
aligned to a 4-byte boundary and the backing memory allocation is
rounded up to the next multiple of 4 byte. Also the free_cprbmem() is
updated and scrubs the rounded up amount of memory.

> diff --git a/drivers/s390/crypto/zcrypt_ep11misc.c b/drivers/s390/crypto/zcrypt_ep11misc.c
> index 3dda9589f2b95..e604c100ccce2 100644
> --- a/drivers/s390/crypto/zcrypt_ep11misc.c
> +++ b/drivers/s390/crypto/zcrypt_ep11misc.c
> @@ -355,21 +356,24 @@ EXPORT_SYMBOL(ep11_check_aes_key);
>  
>  /*
>   * Allocate and prepare ep11 cprb plus additional payload.
> + * It is guaranteed that the memory is aligned to a 4 byte boundary.
> + * Furthermore the memory allocation is rounded up to the next
> + * multiple of 4 bytes (with taking the payload_len into account).
>   */
>  static void *alloc_cprbmem(size_t payload_len, u32 xflags)
>  {
> -	size_t len = sizeof(struct ep11_cprb) + payload_len;
> +	size_t memlen = ALIGN(sizeof(struct ep11_cprb) + payload_len, 4);

[Severity: Medium]
The commit message states that free_cprbmem() is updated to scrub the rounded
up amount of memory. While this change is present for zcrypt_ccamisc.c, was it
missed for zcrypt_ep11misc.c?

Since alloc_cprbmem() now allocates the rounded up memory size, does
free_cprbmem() in zcrypt_ep11misc.c also need to be updated to scrub the
matching size? 

Currently, it still uses the exact unrounded size:

    static void free_cprbmem(void *mem, size_t payload_len, bool scrub, u32 xflags)
    {
        if (mem && scrub)
            memzero_explicit(mem, sizeof(struct ep11_cprb) + payload_len);

Could this leave up to 3 bytes of the newly added padding unscrubbed when an
EP11 CPRB is freed? If the cryptographic hardware writes sensitive data into
the padding bytes of a 4-byte aligned response, this data would not be zeroed
before the memory is returned to the allocator.

[ ... ]

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

Re: [PATCH v8 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code

[Harald Freudenberger]

On 2026-07-29 16:52, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Medium] Incomplete memory scrubbing in `free_cprbmem()` due to
> missing 4-byte alignment rounding.
> --
> 
> --- Patch [1]: [PATCH v8 1/5] s390/zcrypt: Fix CPRB memory allocation
> in zcrypt misc code ---
> commit 8719946bc97657e1ca3c996d1d00474456aa0ef0
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code
> 
> Both CPRB alloc functions in zcrypt_ccamisc.c and zcrypt_ep11misc.c
> did not round up the memory allocation to a multiple of 4 bytes as it
> is needed by the zcrypt layer to process the CPRBs.
> 
> Now the alloc_and_prep_cprbmem() and alloc_cprbmem() functions
> guarantee that the base CPRB struct and a possible parameter block are
> aligned to a 4-byte boundary and the backing memory allocation is
> rounded up to the next multiple of 4 byte. Also the free_cprbmem() is
> updated and scrubs the rounded up amount of memory.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ep11misc.c 
>> b/drivers/s390/crypto/zcrypt_ep11misc.c
>> index 3dda9589f2b95..e604c100ccce2 100644
>> --- a/drivers/s390/crypto/zcrypt_ep11misc.c
>> +++ b/drivers/s390/crypto/zcrypt_ep11misc.c
>> @@ -355,21 +356,24 @@ EXPORT_SYMBOL(ep11_check_aes_key);
>> 
>>  /*
>>   * Allocate and prepare ep11 cprb plus additional payload.
>> + * It is guaranteed that the memory is aligned to a 4 byte boundary.
>> + * Furthermore the memory allocation is rounded up to the next
>> + * multiple of 4 bytes (with taking the payload_len into account).
>>   */
>>  static void *alloc_cprbmem(size_t payload_len, u32 xflags)
>>  {
>> -	size_t len = sizeof(struct ep11_cprb) + payload_len;
>> +	size_t memlen = ALIGN(sizeof(struct ep11_cprb) + payload_len, 4);
> 
> [Severity: Medium]
> The commit message states that free_cprbmem() is updated to scrub the 
> rounded
> up amount of memory. While this change is present for zcrypt_ccamisc.c, 
> was it
> missed for zcrypt_ep11misc.c?
> 
> Since alloc_cprbmem() now allocates the rounded up memory size, does
> free_cprbmem() in zcrypt_ep11misc.c also need to be updated to scrub 
> the
> matching size?
> 
> Currently, it still uses the exact unrounded size:
> 
>     static void free_cprbmem(void *mem, size_t payload_len, bool
> scrub, u32 xflags)
>     {
>         if (mem && scrub)
>             memzero_explicit(mem, sizeof(struct ep11_cprb) + 
> payload_len);
> 
> Could this leave up to 3 bytes of the newly added padding unscrubbed 
> when an
> EP11 CPRB is freed? If the cryptographic hardware writes sensitive data 
> into
> the padding bytes of a 4-byte aligned response, this data would not be 
> zeroed
> before the memory is returned to the allocator.
> 
> [ ... ]

Right, this hunk was on my disk but did not make it into the patch 
somehow -> v9 then.

[PATCH v8 2/5] s390/zcrypt: Improve CCA CPRB length and overflow checks

[Harald Freudenberger]

The xcrb_msg_to_type6cprb_msgx() function lacks proper input
validation, creating security vulnerabilities:
1. Integer overflow after CEIL4 alignment: Signed int variables could
   overflow during 4-byte boundary alignment, causing undersized
   buffer allocations or incorrect bounds checking.
2. Missing minimum size validation: The CPRBX structure is copied from
   userspace without verifying sufficient buffer length. Undersized
   buffers cause uninitialized memory access when reading structure
   fields like cprbx.cprb_len and cprbx.domain.
3. Arithmetic overflow in sum calculations: Adding control block and
   data block sizes could overflow, bypassing size checks and enabling
   buffer overflows.

Fix by using size_t for length calculations, adding U32_MAX boundary
checks after alignment, validating minimum control block size before
copying from userspace, and detecting sum calculation overflows.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/s390/crypto/zcrypt_msgtype6.c | 78 +++++++++++++--------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
index 40f72cdf284d..fb37e28c8242 100644
--- a/drivers/s390/crypto/zcrypt_msgtype6.c
+++ b/drivers/s390/crypto/zcrypt_msgtype6.c
@@ -342,49 +342,40 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
 		};
 	} __packed * msg = ap_msg->msg;
 
-	int rcblen = CEIL4(xcrb->request_control_blk_length);
-	int req_sumlen, resp_sumlen;
-	char *req_data = ap_msg->msg + sizeof(struct type6_hdr) + rcblen;
-	char *function_code;
+	size_t req_cblen, rep_cblen, req_sumlen, rep_sumlen;
+	char *function_code, *req_data;
 
-	if (CEIL4(xcrb->request_control_blk_length) <
-			xcrb->request_control_blk_length)
-		return -EINVAL; /* overflow after alignment*/
-
-	/* length checks */
+	/* request length and overflow checks */
+	if (xcrb->request_control_blk_length < sizeof(struct CPRBX))
+		return -EINVAL;
+	req_cblen = CEIL4((size_t)xcrb->request_control_blk_length);
+	if (req_cblen > U32_MAX)
+		return -EINVAL;
 	ap_msg->len = sizeof(struct type6_hdr) +
-		CEIL4(xcrb->request_control_blk_length) +
-		xcrb->request_data_length;
+		req_cblen + xcrb->request_data_length;
 	if (ap_msg->len > ap_msg->bufsize)
 		return -EINVAL;
-
-	/*
-	 * Overflow check
-	 * sum must be greater (or equal) than the largest operand
-	 */
-	req_sumlen = CEIL4(xcrb->request_control_blk_length) +
-			xcrb->request_data_length;
-	if ((CEIL4(xcrb->request_control_blk_length) <=
-	     xcrb->request_data_length) ?
+	req_sumlen = req_cblen + xcrb->request_data_length;
+	if (req_sumlen > U32_MAX)
+		return -EINVAL;
+	if (req_cblen <= xcrb->request_data_length ?
 	    req_sumlen < xcrb->request_data_length :
-	    req_sumlen < CEIL4(xcrb->request_control_blk_length)) {
+	    req_sumlen < req_cblen) {
 		return -EINVAL;
 	}
 
-	if (CEIL4(xcrb->reply_control_blk_length) <
-			xcrb->reply_control_blk_length)
-		return -EINVAL; /* overflow after alignment*/
-
-	/*
-	 * Overflow check
-	 * sum must be greater (or equal) than the largest operand
-	 */
-	resp_sumlen = CEIL4(xcrb->reply_control_blk_length) +
-			xcrb->reply_data_length;
-	if ((CEIL4(xcrb->reply_control_blk_length) <=
-	     xcrb->reply_data_length) ?
-	    resp_sumlen < xcrb->reply_data_length :
-	    resp_sumlen < CEIL4(xcrb->reply_control_blk_length)) {
+	/* reply length and overflow checks */
+	if (xcrb->reply_control_blk_length < sizeof(struct CPRBX))
+		return -EINVAL;
+	rep_cblen = CEIL4((size_t)xcrb->reply_control_blk_length);
+	if (rep_cblen > U32_MAX)
+		return -EINVAL;
+	rep_sumlen = rep_cblen + xcrb->reply_data_length;
+	if (rep_sumlen > U32_MAX)
+		return -EINVAL;
+	if (rep_cblen <= xcrb->reply_data_length ?
+	    rep_sumlen < xcrb->reply_data_length :
+	    rep_sumlen < rep_cblen) {
 		return -EINVAL;
 	}
 
@@ -393,7 +384,7 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
 	memcpy(msg->hdr.agent_id, &xcrb->agent_ID, sizeof(xcrb->agent_ID));
 	msg->hdr.tocardlen1 = xcrb->request_control_blk_length;
 	if (xcrb->request_data_length) {
-		msg->hdr.offset2 = msg->hdr.offset1 + rcblen;
+		msg->hdr.offset2 = msg->hdr.offset1 + req_cblen;
 		msg->hdr.tocardlen2 = xcrb->request_data_length;
 	}
 	msg->hdr.fromcardlen1 = xcrb->reply_control_blk_length;
@@ -404,8 +395,8 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
 			     xcrb->request_control_blk_addr,
 			     xcrb->request_control_blk_length))
 		return -EFAULT;
-	if (msg->cprbx.cprb_len + sizeof(msg->hdr.function_code) >
-	    xcrb->request_control_blk_length)
+	/* copy subfunction code into AP msg type 6 function code field */
+	if (msg->cprbx.cprb_len > req_cblen - sizeof(msg->hdr.function_code))
 		return -EINVAL;
 	function_code = ((unsigned char *)&msg->cprbx) + msg->cprbx.cprb_len;
 	memcpy(msg->hdr.function_code, function_code,
@@ -437,10 +428,13 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
 	}
 
 	/* copy data block */
-	if (xcrb->request_data_length &&
-	    z_copy_from_user(userspace, req_data, xcrb->request_data_address,
-			     xcrb->request_data_length))
-		return -EFAULT;
+	if (xcrb->request_data_length) {
+		req_data = ap_msg->msg + sizeof(struct type6_hdr) + req_cblen;
+		if (z_copy_from_user(userspace, req_data,
+				     xcrb->request_data_address,
+				     xcrb->request_data_length))
+			return -EFAULT;
+	}
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v8 2/5] s390/zcrypt: Improve CCA CPRB length and overflow checks

[sashiko-bot]

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

Pre-existing issues:
- [Critical] An integer overflow in the length calculation of EP11 requests bypasses the AP message buffer bounds check, leading to a massive heap buffer overflow or immediate kernel panic during the `copy_from_user` operation.
- [High] The `xcrb_msg_to_type6_ep11cprb_msgx()` function does not verify that the user-provided `xcrb->req_len` is sufficiently large (e.g., at least `sizeof(struct ep11_cprb)`) before copying from userspace, allowing undersized requests to leave critical message fields populated with uninitialized kernel memory.
--

--- Patch [2]: [PATCH v8 2/5] s390/zcrypt: Improve CCA CPRB length and overflow checks ---
commit 9af37e5b015a5cc6a88563feffcb032b0b3d9669
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Improve CCA CPRB length and overflow checks

The commit fixes integer overflows and missing length checks in
xcrb_msg_to_type6cprb_msgx() by validating input lengths and checking for
addition overflows.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index 40f72cdf284d3..fb37e28c82424 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c

[ ... ]

> @@ -437,10 +428,13 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
>  	}
>  
>  	/* copy data block */
> -	if (xcrb->request_data_length &&
> -	    z_copy_from_user(userspace, req_data, xcrb->request_data_address,
> -			     xcrb->request_data_length))
> -		return -EFAULT;
> +	if (xcrb->request_data_length) {
> +		req_data = ap_msg->msg + sizeof(struct type6_hdr) + req_cblen;
> +		if (z_copy_from_user(userspace, req_data,
> +				     xcrb->request_data_address,
> +				     xcrb->request_data_length))
> +			return -EFAULT;
> +	}
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but looking further down in the
same file, xcrb_msg_to_type6_ep11cprb_msgx() seems to have the same missing
minimum size validation that this patch fixes for CCA CPRB.

In drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx():

	/* Import CPRB data from the ioctl input parameter */
	if (z_copy_from_user(userspace, msg->userdata,
			     (char __force __user *)xcrb->req, xcrb->req_len)) {

If the user-provided xcrb->req_len is smaller than sizeof(struct ep11_cprb),
the rest of the message header could remain uninitialized. Could this result
in uninitialized kernel memory dictating control flow or being used to route
requests?

[Severity: Critical]
This is a pre-existing issue, but xcrb_msg_to_type6_ep11cprb_msgx() also
appears to suffer from the same integer overflow issue:

In drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx():

	/* length checks */
	ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);
	if (ap_msg->len > ap_msg->bufsize)
		return -EINVAL;

If a user provides a large 64-bit xcrb->req_len like 0xFFFFFFFFFFFFFFB0,
could CEIL4(xcrb->req_len) wrap around, bypassing the ap_msg->bufsize
check, and resulting in a huge buffer overflow or kernel panic during the
subsequent z_copy_from_user()?

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

Re: [PATCH v8 2/5] s390/zcrypt: Improve CCA CPRB length and overflow checks

[Harald Freudenberger]

On 2026-07-29 16:59, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] An integer overflow in the length calculation of EP11
> requests bypasses the AP message buffer bounds check, leading to a
> massive heap buffer overflow or immediate kernel panic during the
> `copy_from_user` operation.
> - [High] The `xcrb_msg_to_type6_ep11cprb_msgx()` function does not
> verify that the user-provided `xcrb->req_len` is sufficiently large
> (e.g., at least `sizeof(struct ep11_cprb)`) before copying from
> userspace, allowing undersized requests to leave critical message
> fields populated with uninitialized kernel memory.
> --
> 
> --- Patch [2]: [PATCH v8 2/5] s390/zcrypt: Improve CCA CPRB length and
> overflow checks ---
> commit 9af37e5b015a5cc6a88563feffcb032b0b3d9669
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/zcrypt: Improve CCA CPRB length and overflow checks
> 
> The commit fixes integer overflows and missing length checks in
> xcrb_msg_to_type6cprb_msgx() by validating input lengths and checking 
> for
> addition overflows.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c 
>> b/drivers/s390/crypto/zcrypt_msgtype6.c
>> index 40f72cdf284d3..fb37e28c82424 100644
>> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
>> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> 
> [ ... ]
> 
>> @@ -437,10 +428,13 @@ static int xcrb_msg_to_type6cprb_msgx(bool 
>> userspace, struct ap_message *ap_msg,
>>  	}
>> 
>>  	/* copy data block */
>> -	if (xcrb->request_data_length &&
>> -	    z_copy_from_user(userspace, req_data, 
>> xcrb->request_data_address,
>> -			     xcrb->request_data_length))
>> -		return -EFAULT;
>> +	if (xcrb->request_data_length) {
>> +		req_data = ap_msg->msg + sizeof(struct type6_hdr) + req_cblen;
>> +		if (z_copy_from_user(userspace, req_data,
>> +				     xcrb->request_data_address,
>> +				     xcrb->request_data_length))
>> +			return -EFAULT;
>> +	}
>> 
>>  	return 0;
>>  }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but looking further down in 
> the
> same file, xcrb_msg_to_type6_ep11cprb_msgx() seems to have the same 
> missing
> minimum size validation that this patch fixes for CCA CPRB.
> 
> In 
> drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx():
> 
> 	/* Import CPRB data from the ioctl input parameter */
> 	if (z_copy_from_user(userspace, msg->userdata,
> 			     (char __force __user *)xcrb->req, xcrb->req_len)) {
> 
> If the user-provided xcrb->req_len is smaller than sizeof(struct 
> ep11_cprb),
> the rest of the message header could remain uninitialized. Could this 
> result
> in uninitialized kernel memory dictating control flow or being used to 
> route
> requests?
> 
> [Severity: Critical]
> This is a pre-existing issue, but xcrb_msg_to_type6_ep11cprb_msgx() 
> also
> appears to suffer from the same integer overflow issue:
> 
> In 
> drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx():
> 
> 	/* length checks */
> 	ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);
> 	if (ap_msg->len > ap_msg->bufsize)
> 		return -EINVAL;
> 
> If a user provides a large 64-bit xcrb->req_len like 
> 0xFFFFFFFFFFFFFFB0,
> could CEIL4(xcrb->req_len) wrap around, bypassing the ap_msg->bufsize
> check, and resulting in a huge buffer overflow or kernel panic during 
> the
> subsequent z_copy_from_user()?

And these both are fixed with the next patch in the series !!!

[PATCH v8 3/5] s390/zcrypt: Improve EP11 CPRB length and overflow checks

[Harald Freudenberger]

The xcrb_msg_to_type6_ep11cprb_msgx() function lacks proper input
validation, creating security vulnerabilities:
1. Missing minimum size validation: The ep11_cprb structure and
   subsequent payload fields (pld_tag, pld_lenfmt) are copied from
   userspace without verifying sufficient buffer length.
2. Arithmetic overflow in length calculations: CEIL4 alignment could
   overflow, bypassing size checks and enabling buffer overflows.
3. The payload is asn1 encoded but the function just uses a simple c
   struct overlay to access some fields of the payload.

Fix by using size_t for length calculations, adding U32_MAX boundary
checks after alignment, and validating minimum request size and
minimum reply size before copying from userspace. Do a very simple
asn1 parsing of the payload up to the function value field.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/s390/crypto/zcrypt_msgtype6.c | 148 +++++++++++++++++++-------
 1 file changed, 109 insertions(+), 39 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
index fb37e28c8242..c4b5d9b85f7a 100644
--- a/drivers/s390/crypto/zcrypt_msgtype6.c
+++ b/drivers/s390/crypto/zcrypt_msgtype6.c
@@ -439,12 +439,61 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
 	return 0;
 }
 
+/*
+ * Simple asn1 int reader/decoder helper function
+ * Returns number of bytes processed or < 0 on failure
+ * Only accepts int length values of 1, 2 or 4.
+ */
+static inline int asn1_int_decode(const u8 *buf, size_t intlen, u32 *u)
+{
+	switch (intlen) {
+	case 1:
+		*u = (u32)(*((u8 *)buf));
+		return 1;
+	case 2:
+		*u = (u32)be16_to_cpup((u16 *)buf);
+		return 2;
+	case 4:
+		*u = (u32)be32_to_cpup((u32 *)buf);
+		return 4;
+	default:
+		return -EINVAL;
+	}
+}
+
+/*
+ * Simple asn1 length parse helper function
+ * Returns number of bytes processed or < 0 on failure
+ * Only accepts length encoded within the length octet
+ * or for long form 1, 2 or 4 octet length bytes.
+ */
+static inline int asn1_length_decode(const u8 *buf, size_t buflen, u32 *u)
+{
+	int i;
+
+	if (buflen < 1)
+		return -EINVAL;
+
+	if (*buf < 128) {
+		*u = (u32)(*buf & 0x7F);
+		return 1;
+	}
+
+	i = *buf & 0x7F;
+	if (--buflen < i)
+		return -EINVAL;
+	i = asn1_int_decode(++buf, i, u);
+
+	return i < 0 ? i : i + 1;
+}
+
 static int xcrb_msg_to_type6_ep11cprb_msgx(bool userspace, struct ap_message *ap_msg,
 					   struct ep11_urb *xcrb,
 					   unsigned int *fcode,
 					   unsigned int *domain)
 {
-	unsigned int lfmt;
+	static const size_t min_pld_size = 5 * sizeof(unsigned char);
+
 	static struct type6_hdr static_type6_ep11_hdr = {
 		.type		=  0x06,
 		.rqid		= {0x00, 0x01},
@@ -456,34 +505,32 @@ static int xcrb_msg_to_type6_ep11cprb_msgx(bool userspace, struct ap_message *ap
 	struct {
 		struct type6_hdr hdr;
 		union {
-			struct {
-				struct ep11_cprb cprbx;
-				unsigned char pld_tag;    /* fixed value 0x30 */
-				unsigned char pld_lenfmt; /* length format */
-			} __packed;
+			struct ep11_cprb cprbx;
 			DECLARE_FLEX_ARRAY(u8, userdata);
 		};
 	} __packed * msg = ap_msg->msg;
 
-	struct pld_hdr {
-		unsigned char	func_tag;	/* fixed value 0x4 */
-		unsigned char	func_len;	/* fixed value 0x4 */
-		unsigned int	func_val;	/* function ID	   */
-		unsigned char	dom_tag;	/* fixed value 0x4 */
-		unsigned char	dom_len;	/* fixed value 0x4 */
-		unsigned int	dom_val;	/* domain id	   */
-	} __packed * payload_hdr = NULL;
-
-	if (CEIL4(xcrb->req_len) < xcrb->req_len)
-		return -EINVAL; /* overflow after alignment*/
+	size_t req_len, rep_len, pld_len;
+	unsigned char *pld;
+	int offs = 0, i;
+	unsigned int u;
 
-	/* length checks */
-	ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);
+	/* request length and overflow checks */
+	if (xcrb->req_len < sizeof(struct ep11_cprb) + min_pld_size)
+		return -EINVAL;
+	req_len = CEIL4(xcrb->req_len);
+	if (req_len < xcrb->req_len || req_len > U32_MAX)
+		return -EINVAL;
+	ap_msg->len = sizeof(struct type6_hdr) + req_len;
 	if (ap_msg->len > ap_msg->bufsize)
 		return -EINVAL;
 
-	if (CEIL4(xcrb->resp_len) < xcrb->resp_len)
-		return -EINVAL; /* overflow after alignment*/
+	/* reply length and overflow checks */
+	if (xcrb->resp_len < sizeof(struct ep11_cprb))
+		return -EINVAL;
+	rep_len = CEIL4(xcrb->resp_len);
+	if (rep_len < xcrb->resp_len || rep_len > U32_MAX)
+		return -EINVAL;
 
 	/* prepare type6 header */
 	msg->hdr = static_type6_ep11_hdr;
@@ -492,26 +539,49 @@ static int xcrb_msg_to_type6_ep11cprb_msgx(bool userspace, struct ap_message *ap
 
 	/* Import CPRB data from the ioctl input parameter */
 	if (z_copy_from_user(userspace, msg->userdata,
-			     (char __force __user *)xcrb->req, xcrb->req_len)) {
+			     (char __force __user *)xcrb->req, xcrb->req_len))
 		return -EFAULT;
-	}
 
-	if ((msg->pld_lenfmt & 0x80) == 0x80) { /*ext.len.fmt 2 or 3*/
-		switch (msg->pld_lenfmt & 0x03) {
-		case 1:
-			lfmt = 2;
-			break;
-		case 2:
-			lfmt = 3;
-			break;
-		default:
-			return -EINVAL;
-		}
-	} else {
-		lfmt = 1; /* length format #1 */
-	}
-	payload_hdr = (struct pld_hdr *)((&msg->pld_lenfmt) + lfmt);
-	*fcode = payload_hdr->func_val & 0xFFFF;
+	pld = msg->userdata + sizeof(struct ep11_cprb);
+	pld_len = msg->cprbx.payload_len;
+	if (pld_len != xcrb->req_len - sizeof(struct ep11_cprb))
+		return -EINVAL;
+	/*
+	 * At this point pld_len is always >= min_pld_size
+	 * and the smallest supported asn1 payload is:
+	 *   payload tag (1 octet)
+	 *   payload length (1-5 octets)
+	 *   function tag (1 octet)
+	 *   function length (1-5 octets)
+	 *   function value (1-4 octets)
+	 */
+
+	/* payload tag */
+	if (pld[offs++] != 0x30)
+		return -EINVAL;
+	/* payload length field */
+	i = asn1_length_decode(pld + offs, pld_len - offs, &u);
+	if (i < 0)
+		return -EINVAL;
+	offs += i;
+	if (u > pld_len - offs)
+		return -EINVAL;
+	/* function tag */
+	if (pld[offs++] != 0x04)
+		return -EINVAL;
+	/* function length */
+	i = asn1_length_decode(pld + offs, pld_len - offs, &u);
+	if (i < 0)
+		return -EINVAL;
+	offs += i;
+	if (u > pld_len - offs)
+		return -EINVAL;
+	/* function value */
+	i = asn1_int_decode(pld + offs, u, &u);
+	if (i < 0)
+		return -EINVAL;
+	offs += i;
+	*fcode = 0xFFFF & u;
 
 	/* enable special processing based on the cprbs flags special bit */
 	if (msg->cprbx.flags & 0x20)
-- 
2.43.0

Re: [PATCH v8 3/5] s390/zcrypt: Improve EP11 CPRB length and overflow checks

[sashiko-bot]

> The xcrb_msg_to_type6_ep11cprb_msgx() function lacks proper input
> validation, creating security vulnerabilities:
> 1. Missing minimum size validation: The ep11_cprb structure and
>    subsequent payload fields (pld_tag, pld_lenfmt) are copied from
>    userspace without verifying sufficient buffer length.
> 2. Arithmetic overflow in length calculations: CEIL4 alignment could
>    overflow, bypassing size checks and enabling buffer overflows.
> 3. The payload is asn1 encoded but the function just uses a simple c
>    struct overlay to access some fields of the payload.
> 
> Fix by using size_t for length calculations, adding U32_MAX boundary
> checks after alignment, and validating minimum request size and
> minimum reply size before copying from userspace. Do a very simple
> asn1 parsing of the payload up to the function value field.
> 
> 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/20260729143928.191969-1-freude@linux.ibm.com?part=3

[PATCH v8 4/5] s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing

[Harald Freudenberger]

The zcrypt_msgtype6_send_ep11_cprb() function uses fragile struct
overlays to access and modify the domain field in the EP11 CPRB
payload, creating maintainability and security concerns:
1. Struct overlay approach (pld_hdr) assumes fixed payload structure
   and doesn't validate the actual ASN.1 encoding.
2. Complex length format detection logic is error-prone and doesn't
   properly validate bounds at each parsing step.
3. Direct struct member access bypasses proper ASN.1 validation.

Fix by replacing struct overlays with explicit ASN.1 parsing that
validates each field (payload tag/length, function tag/length/value,
optional domain tag/length/value) with proper bounds checking at every
step. Add asn1_int_encode() helper function to safely write integer
values with correct endianness conversion. This makes the code
consistent with the validation pattern introduced with the rework of
the xcrb_msg_to_type6_ep11cprb_msgx() function.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/s390/crypto/zcrypt_msgtype6.c | 123 +++++++++++++++++++-------
 1 file changed, 92 insertions(+), 31 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
index c4b5d9b85f7a..59e182d1bddf 100644
--- a/drivers/s390/crypto/zcrypt_msgtype6.c
+++ b/drivers/s390/crypto/zcrypt_msgtype6.c
@@ -1223,6 +1223,28 @@ int prep_ep11_ap_msg(bool userspace, struct ep11_urb *xcrb,
 					       func_code, domain);
 }
 
+/*
+ * Simple asn1 int writer/encoder helper function
+ * Returns number of bytes processed or < 0 on failure
+ * Only accepts int length values of 1, 2 or 4.
+ */
+static inline int asn1_int_encode(u8 *buf, size_t intlen, u32 u)
+{
+	switch (intlen) {
+	case 1:
+		*((u8 *)buf) = (u8)u;
+		return 1;
+	case 2:
+		*((u16 *)buf) = cpu_to_be16(u);
+		return 2;
+	case 4:
+		*((u32 *)buf) = cpu_to_be32(u);
+		return 4;
+	default:
+		return -EINVAL;
+	}
+}
+
 /*
  * The request distributor calls this function if it picked the CEX4P
  * device to handle a send_ep11_cprb request.
@@ -1235,51 +1257,90 @@ static long zcrypt_msgtype6_send_ep11_cprb(bool userspace, struct zcrypt_queue *
 					   struct ap_message *ap_msg)
 {
 	int rc;
-	unsigned int lfmt;
 	struct ap_response_type *resp_type = &ap_msg->response;
 	struct {
 		struct type6_hdr hdr;
 		struct ep11_cprb cprbx;
-		unsigned char	pld_tag;	/* fixed value 0x30 */
-		unsigned char	pld_lenfmt;	/* payload length format */
 	} __packed * msg = ap_msg->msg;
-	struct pld_hdr {
-		unsigned char	func_tag;	/* fixed value 0x4 */
-		unsigned char	func_len;	/* fixed value 0x4 */
-		unsigned int	func_val;	/* function ID	   */
-		unsigned char	dom_tag;	/* fixed value 0x4 */
-		unsigned char	dom_len;	/* fixed value 0x4 */
-		unsigned int	dom_val;	/* domain id	   */
-	} __packed * payload_hdr = NULL;
 
 	/*
 	 * The target domain field within the cprb body/payload block will be
 	 * replaced by the usage domain for non-management commands only.
 	 * Therefore we check the first bit of the 'flags' parameter for
 	 * management command indication.
-	 *   0 - non management command
-	 *   1 - management command
 	 */
-	if (!((msg->cprbx.flags & 0x80) == 0x80)) {
-		msg->cprbx.target_id = (unsigned int)
-					AP_QID_QUEUE(zq->queue->qid);
-
-		if ((msg->pld_lenfmt & 0x80) == 0x80) { /*ext.len.fmt 2 or 3*/
-			switch (msg->pld_lenfmt & 0x03) {
-			case 1:
-				lfmt = 2;
-				break;
-			case 2:
-				lfmt = 3;
-				break;
-			default:
+	if (!(msg->cprbx.flags & 0x80)) {
+		int i, offs = 0;
+		size_t pld_len;
+		u8 *pld;
+		u32 u;
+
+		/* update target field in ep11_cprb */
+		msg->cprbx.target_id = (u32)AP_QID_QUEUE(zq->queue->qid);
+
+		/* ptr and length to payload */
+		pld = ap_msg->msg +
+			sizeof(struct type6_hdr) + sizeof(struct ep11_cprb);
+		pld_len = msg->cprbx.payload_len;
+
+		/*
+		 * Parse the asn1 payload, at least we have
+		 *   pld tag (1 octet)
+		 *   payload length (1-5 octets)
+		 *   function tag (1 octet)
+		 *   function length (1-5 octets)
+		 *   function value (1-4 octets)
+		 *   ----- optional fields -----
+		 *   domain tag (1 octet)
+		 *   domain length (1-5 octets)
+		 *   domain value (1-4 octets)
+		 *   ... maybe much more data ...
+		 */
+
+		/* payload tag */
+		if (pld[offs++] != 0x30)
+			return -EINVAL;
+		/* payload length field */
+		i = asn1_length_decode(pld + offs, pld_len - offs, &u);
+		if (i < 0)
+			return -EINVAL;
+		offs += i;
+		if (u > pld_len - offs)
+			return -EINVAL;
+		/* function tag */
+		if (pld[offs++] != 0x04)
+			return -EINVAL;
+		/* function length */
+		i = asn1_length_decode(pld + offs, pld_len - offs, &u);
+		if (i < 0)
+			return -EINVAL;
+		offs += i;
+		if (u > pld_len - offs)
+			return -EINVAL;
+		/* skip over the function value */
+		offs += u;
+		/* is there some payload left which could hold a domain value ? */
+		if (pld_len - offs >= 3) {
+			/* domain tag */
+			if (pld[offs++] != 0x04)
 				return -EINVAL;
-			}
-		} else {
-			lfmt = 1; /* length format #1 */
+			/* domain length */
+			i = asn1_length_decode(pld + offs, pld_len - offs, &u);
+			if (i < 0)
+				return -EINVAL;
+			offs += i;
+			if (u > pld_len - offs)
+				return -EINVAL;
+			/*
+			 * pld[offs] is now at the start of the domain value
+			 * with the value sprawled in u octets.
+			 */
+			i = asn1_int_encode(pld + offs, u,
+					    AP_QID_QUEUE(zq->queue->qid));
+			if (i < 0)
+				return -EINVAL;
+			offs += i;
 		}
-		payload_hdr = (struct pld_hdr *)((&msg->pld_lenfmt) + lfmt);
-		payload_hdr->dom_val = AP_QID_QUEUE(zq->queue->qid);
 	}
 
 	/*
-- 
2.43.0

Re: [PATCH v8 4/5] s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Undefined behavior due to unaligned memory access via direct pointer casting.
--

--- Patch [4]: [PATCH v8 4/5] s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing ---
commit 78d69567226ac24ed3207a9812dbc20b238a13e1
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing

This commit replaces struct overlays with explicit ASN.1 parsing for
EP11 CPRB payload domain fields, adding validation and a new helper
function for safely encoding integer values.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index c4b5d9b85f7a9..59e182d1bddf9 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> @@ -1223,6 +1223,28 @@ int prep_ep11_ap_msg(bool userspace, struct ep11_urb *xcrb,
>  					       func_code, domain);
>  }
>  
> +/*
> + * Simple asn1 int writer/encoder helper function
> + * Returns number of bytes processed or < 0 on failure
> + * Only accepts int length values of 1, 2 or 4.
> + */
> +static inline int asn1_int_encode(u8 *buf, size_t intlen, u32 u)
> +{
> +	switch (intlen) {
> +	case 1:
> +		*((u8 *)buf) = (u8)u;
> +		return 1;
> +	case 2:
> +		*((u16 *)buf) = cpu_to_be16(u);

[Severity: Medium]
Could this lead to unaligned memory accesses?

When asn1_int_encode() is called from zcrypt_msgtype6_send_ep11_cprb()
using pld + offs, the offset is dynamically calculated from variable length
ASN.1 tags and lengths, which may leave the buf pointer unaligned.

While s390 hardware handles unaligned accesses, directly casting a potentially
unaligned u8 pointer to u16* could violate C standard alignment rules and
potentially lead to compiler misoptimization. Would it be safer to use
put_unaligned_be16() here?

> +		return 2;
> +	case 4:
> +		*((u32 *)buf) = cpu_to_be32(u);

[Severity: Medium]
Similarly, should this use put_unaligned_be32() instead of a direct cast
to u32* for the same unaligned pointer reasons?

> +		return 4;
> +	default:
> +		return -EINVAL;
> +	}
> +}

[ ... ]

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

Re: [PATCH v8 4/5] s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing

[Harald Freudenberger]

On 2026-07-29 16:50, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Medium] Undefined behavior due to unaligned memory access via
> direct pointer casting.
> --
> 
> --- Patch [4]: [PATCH v8 4/5] s390/zcrypt: Improve EP11 CPRB domain
> handling with ASN.1 parsing ---
> commit 78d69567226ac24ed3207a9812dbc20b238a13e1
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing
> 
> This commit replaces struct overlays with explicit ASN.1 parsing for
> EP11 CPRB payload domain fields, adding validation and a new helper
> function for safely encoding integer values.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c 
>> b/drivers/s390/crypto/zcrypt_msgtype6.c
>> index c4b5d9b85f7a9..59e182d1bddf9 100644
>> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
>> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
>> @@ -1223,6 +1223,28 @@ int prep_ep11_ap_msg(bool userspace, struct 
>> ep11_urb *xcrb,
>>  					       func_code, domain);
>>  }
>> 
>> +/*
>> + * Simple asn1 int writer/encoder helper function
>> + * Returns number of bytes processed or < 0 on failure
>> + * Only accepts int length values of 1, 2 or 4.
>> + */
>> +static inline int asn1_int_encode(u8 *buf, size_t intlen, u32 u)
>> +{
>> +	switch (intlen) {
>> +	case 1:
>> +		*((u8 *)buf) = (u8)u;
>> +		return 1;
>> +	case 2:
>> +		*((u16 *)buf) = cpu_to_be16(u);
> 
> [Severity: Medium]
> Could this lead to unaligned memory accesses?
> 
> When asn1_int_encode() is called from zcrypt_msgtype6_send_ep11_cprb()
> using pld + offs, the offset is dynamically calculated from variable 
> length
> ASN.1 tags and lengths, which may leave the buf pointer unaligned.
> 
> While s390 hardware handles unaligned accesses, directly casting a 
> potentially
> unaligned u8 pointer to u16* could violate C standard alignment rules 
> and
> potentially lead to compiler misoptimization. Would it be safer to use
> put_unaligned_be16() here?
> 
>> +		return 2;
>> +	case 4:
>> +		*((u32 *)buf) = cpu_to_be32(u);
> 
> [Severity: Medium]
> Similarly, should this use put_unaligned_be32() instead of a direct 
> cast
> to u32* for the same unaligned pointer reasons?
> 
>> +		return 4;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
> 
> [ ... ]

These both unaligned issues will be fixed with v9 by using 
put_unaligned_be16/32()

[PATCH v8 5/5] s390/zcrypt: Pad trailing CCA or EP11 message with zeros

[Harald Freudenberger]

The both functions xcrb_msg_to_type6cprb_msgx() and
xcrb_msg_to_type6_ep11cprb_msgx() copy the user space message into a
kernel buffer based on the message length. But on further processing
the message is supposed to be 4 byte length adjusted. Thus up to 3
bytes of uninitialized kernel memory are forwarded to further
processing steps and may unwanted expose kernel memory to the crypto
card firmware.

This patch contains code to pad the gap between user space copied
message and message buffer length sent down to further processing of
the CCA or EP11 message to zeros.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/s390/crypto/zcrypt_msgtype6.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
index 59e182d1bddf..0269c92a84bd 100644
--- a/drivers/s390/crypto/zcrypt_msgtype6.c
+++ b/drivers/s390/crypto/zcrypt_msgtype6.c
@@ -395,6 +395,10 @@ static int xcrb_msg_to_type6cprb_msgx(bool userspace, struct ap_message *ap_msg,
 			     xcrb->request_control_blk_addr,
 			     xcrb->request_control_blk_length))
 		return -EFAULT;
+	/* pad tail with 0 up to req_cblen */
+	if (xcrb->request_control_blk_length < req_cblen)
+		memset(msg->userdata + xcrb->request_control_blk_length,
+		       0, req_cblen - xcrb->request_control_blk_length);
 	/* copy subfunction code into AP msg type 6 function code field */
 	if (msg->cprbx.cprb_len > req_cblen - sizeof(msg->hdr.function_code))
 		return -EINVAL;
@@ -541,6 +545,10 @@ static int xcrb_msg_to_type6_ep11cprb_msgx(bool userspace, struct ap_message *ap
 	if (z_copy_from_user(userspace, msg->userdata,
 			     (char __force __user *)xcrb->req, xcrb->req_len))
 		return -EFAULT;
+	/* pad tail with 0 up to req_len */
+	if (xcrb->req_len < req_len)
+		memset(msg->userdata + xcrb->req_len, 0,
+		       req_len - xcrb->req_len);
 
 	pld = msg->userdata + sizeof(struct ep11_cprb);
 	pld_len = msg->cprbx.payload_len;
-- 
2.43.0

Re: [PATCH v8 5/5] s390/zcrypt: Pad trailing CCA or EP11 message with zeros

[sashiko-bot]

> The both functions xcrb_msg_to_type6cprb_msgx() and
> xcrb_msg_to_type6_ep11cprb_msgx() copy the user space message into a
> kernel buffer based on the message length. But on further processing
> the message is supposed to be 4 byte length adjusted. Thus up to 3
> bytes of uninitialized kernel memory are forwarded to further
> processing steps and may unwanted expose kernel memory to the crypto
> card firmware.
> 
> This patch contains code to pad the gap between user space copied
> message and message buffer length sent down to further processing of
> the CCA or EP11 message to zeros.
> 
> 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/20260729143928.191969-1-freude@linux.ibm.com?part=5