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

[PATCH v7 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.

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  |  16 +-
 drivers/s390/crypto/zcrypt_ep11misc.c |  12 +-
 drivers/s390/crypto/zcrypt_msgtype6.c | 357 ++++++++++++++++++--------
 3 files changed, 264 insertions(+), 121 deletions(-)

--
2.43.0

[PATCH v7 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.

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

diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
index 84936a795b95..0aa72593ae2b 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;
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

[PATCH v7 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

[PATCH v7 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

[PATCH v7 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

[PATCH v7 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 v7 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code

[sashiko-bot]

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

New issues:
- [Medium] Incomplete memory scrubbing in CPRB free functions leaves padding bytes unzeroed.

Pre-existing issues:
- [High] Information leak of uninitialized kernel heap memory to AP hardware due to unpadded EP11 request lengths.
- [High] Incomplete fix in EP11 CPRB memory allocation causes valid 4-byte padded hardware replies to be rejected with -EMSGSIZE.
--

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

s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code

This commit rounds up the memory allocation for CPRB structures to a
multiple of 4 bytes. It ensures the base CPRB and parameter block are
aligned to a 4-byte boundary, which is required by the zcrypt layer.

> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index 84936a795b956..0aa72593ae2bd 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> @@ -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);

[Severity: Medium]
If the allocation size is rounded up here, do the corresponding
free_cprbmem() functions need to be updated to clear the padded size?

For example, in drivers/s390/crypto/zcrypt_ccamisc.c:free_cprbmem():

    memzero_explicit(mem, 2 * (sizeof(struct CPRBX) + paramblen));

And in drivers/s390/crypto/zcrypt_ep11misc.c:free_cprbmem():

    memzero_explicit(mem, sizeof(struct ep11_cprb) + payload_len);

Currently they use the unpadded lengths. Can this leave the padding bytes
unzeroed when freed, potentially leaking sensitive hardware response data
that spills into the padding?

[ ... ]

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

[Severity: High]
This is a pre-existing issue, but since we are aligning EP11 lengths, does
the AP message creation leak uninitialized kernel memory?

In drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx(),
the message length is rounded up:

    ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);

But only the unpadded length is copied into the buffer:

    if (z_copy_from_user(userspace, msg->userdata, (char __force __user *)xcrb->req, xcrb->req_len))

Since drivers/s390/crypto/ap_bus.c:ap_init_apmsg() allocates the buffer
without __GFP_ZERO:

    ap_msg->msg = kmalloc(maxmsgsize, GFP_KERNEL);

Can the padding gap between xcrb->req_len and CEIL4(xcrb->req_len) leak up
to 3 bytes of uninitialized kernel heap memory to the AP hardware?


[Severity: High]
This is a pre-existing issue, but does aligning the memory allocation
resolve the EMSGSIZE failures if prep_urb() still uses the unpadded length?

In drivers/s390/crypto/zcrypt_ep11misc.c:prep_urb(), the unpadded length
is passed:

    u->resp_len = rep_len;

When the hardware replies with a 4-byte padded length, 
convert_type86_ep11_xcrb() in drivers/s390/crypto/zcrypt_msgtype6.c
checks it against the unpadded resp_len:

    if (xcrb->resp_len < msg->fmt2.count1) {
        pr_debug("resp_len %u < required %u => EMSGSIZE\n",
                 (unsigned int)xcrb->resp_len, msg->fmt2.count1);
        return -EMSGSIZE;
    }

Does this cause valid padded hardware replies to be rejected with
-EMSGSIZE for unaligned requests?

>  
>  	cprb->cprb_len = sizeof(struct ep11_cprb);
>  	cprb->cprb_ver_id = 0x04;

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch addresses integer overflow and uninitialized memory read vulnerabilities in the CCA CPRB processing function (`xcrb_msg_to_type6cprb_msgx`), but completely overlooks the sibling function `xcrb_msg_to_type6_ep11cprb_msgx` which suffers from identical flaws.
--

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

s390/zcrypt: Improve CCA CPRB length and overflow checks

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.

> 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 the sibling function
xcrb_msg_to_type6_ep11cprb_msgx() in this same file appears to suffer from
the exact same vulnerabilities being fixed here.

For example, when req_len is smaller than the required struct size,
z_copy_from_user() can complete successfully but leave trailing buffer
bytes uninitialized:

drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx() {
...
	if (z_copy_from_user(userspace, msg->userdata,
			     (char __force __user *)xcrb->req, xcrb->req_len)) {
		return -EFAULT;
	}

	if ((msg->pld_lenfmt & 0x80) == 0x80) { /*ext.len.fmt 2 or 3*/
...
}

Because ap_msg->msg is allocated without zeroing, this reads uninitialized
kernel heap memory to perform routing and access control logic (like
domain, fcode, and whether the request requires admin or usage permission).
Additionally, CEIL4 can overflow for lengths near U64_MAX, bypassing
initial length checks.

Should xcrb_msg_to_type6_ep11cprb_msgx() also receive similar minimum
size validation and overflow checks?

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

Re: [PATCH v7 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/20260729095318.184141-1-freude@linux.ibm.com?part=3

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

[sashiko-bot]

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

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

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing bounds check on `offset1` and `count1` in `zcrypt_msgtype6_receive` when `count2` is present, leading to an arbitrary kernel memory disclosure to userspace.
--

--- Patch [5]: [PATCH v7 5/5] s390/zcrypt: Pad trailing CCA or EP11 message with zeros ---
commit d39d55b2525dc84382b638f22d1bbcec3350871f
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Pad trailing CCA or EP11 message with zeros

This commit pads the gap between the user space copied message and the
message buffer length with zeros. This prevents up to 3 bytes of
uninitialized kernel memory from being exposed to the crypto card firmware.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index 59e182d1bddf9..0269c92a84bd4 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but while looking at this file, does this code
in zcrypt_msgtype6_receive() lack a bounds check for offset1 and count1 when
count2 is present?

drivers/s390/crypto/zcrypt_msgtype6.c:zcrypt_msgtype6_receive() {
    ...
			if (t86r->fmt2.count2)
				len = t86r->fmt2.offset2 + t86r->fmt2.count2;
			else
				len = t86r->fmt2.offset1 + t86r->fmt2.count1;
			if (len > reply->bufsize || len > msg->bufsize ||
			    len != reply->len) {
    ...
}

If a faulty or compromised crypto card firmware returns a response where
count2 != 0 but offset1 + count1 is maliciously large, len is calculated
entirely from offset2 + count2. This skips bounds checking for offset1 +
count1 against the buffer bounds.

Later, in convert_type86_xcrb(), the unvalidated offset1 and count1 are used
to copy kernel memory to userspace:

drivers/s390/crypto/zcrypt_msgtype6.c:convert_type86_xcrb() {
    ...
	if (z_copy_to_user(userspace, xcrb->reply_control_blk_addr,
			   data + msg->fmt2.offset1, msg->fmt2.count1))
		return -EFAULT;
    ...
}

Could this lead to an out-of-bounds read that leaks uninitialized slab
memory or unrelated kernel memory to userspace?

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

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

[Harald Freudenberger]

On 2026-07-29 12:15, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider:
> 
> New issues:
> - [Medium] Incomplete memory scrubbing in CPRB free functions leaves
> padding bytes unzeroed.
> 
> Pre-existing issues:
> - [High] Information leak of uninitialized kernel heap memory to AP
> hardware due to unpadded EP11 request lengths.
> - [High] Incomplete fix in EP11 CPRB memory allocation causes valid
> 4-byte padded hardware replies to be rejected with -EMSGSIZE.
> --
> 
> --- Patch [1]: [PATCH v7 1/5] s390/zcrypt: Fix CPRB memory allocation
> in zcrypt misc code ---
> commit cba05c09d2194f550fb91ddbd8234cd389b1848a
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code
> 
> This commit rounds up the memory allocation for CPRB structures to a
> multiple of 4 bytes. It ensures the base CPRB and parameter block are
> aligned to a 4-byte boundary, which is required by the zcrypt layer.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c 
>> b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index 84936a795b956..0aa72593ae2bd 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
>> @@ -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);
> 
> [Severity: Medium]
> If the allocation size is rounded up here, do the corresponding
> free_cprbmem() functions need to be updated to clear the padded size?
> 
> For example, in drivers/s390/crypto/zcrypt_ccamisc.c:free_cprbmem():
> 
>     memzero_explicit(mem, 2 * (sizeof(struct CPRBX) + paramblen));
> 
> And in drivers/s390/crypto/zcrypt_ep11misc.c:free_cprbmem():
> 
>     memzero_explicit(mem, sizeof(struct ep11_cprb) + payload_len);
> 
> Currently they use the unpadded lengths. Can this leave the padding 
> bytes
> unzeroed when freed, potentially leaking sensitive hardware response 
> data
> that spills into the padding?

Valid and will be addressed with version 8 of this patch series.

> 
> [ ... ]
> 
>> 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);
>>  	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);
> 
> [Severity: High]
> This is a pre-existing issue, but since we are aligning EP11 lengths, 
> does
> the AP message creation leak uninitialized kernel memory?
> 
> In 
> drivers/s390/crypto/zcrypt_msgtype6.c:xcrb_msg_to_type6_ep11cprb_msgx(),
> the message length is rounded up:
> 
>     ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);
> 
> But only the unpadded length is copied into the buffer:
> 
>     if (z_copy_from_user(userspace, msg->userdata, (char __force
> __user *)xcrb->req, xcrb->req_len))
> 
> Since drivers/s390/crypto/ap_bus.c:ap_init_apmsg() allocates the buffer
> without __GFP_ZERO:
> 
>     ap_msg->msg = kmalloc(maxmsgsize, GFP_KERNEL);
> 
> Can the padding gap between xcrb->req_len and CEIL4(xcrb->req_len) leak 
> up
> to 3 bytes of uninitialized kernel heap memory to the AP hardware?
> 

Addressed with patch #5 of this series.

> 
> [Severity: High]
> This is a pre-existing issue, but does aligning the memory allocation
> resolve the EMSGSIZE failures if prep_urb() still uses the unpadded 
> length?
> 
> In drivers/s390/crypto/zcrypt_ep11misc.c:prep_urb(), the unpadded 
> length
> is passed:
> 
>     u->resp_len = rep_len;
> 
> When the hardware replies with a 4-byte padded length,
> convert_type86_ep11_xcrb() in drivers/s390/crypto/zcrypt_msgtype6.c
> checks it against the unpadded resp_len:
> 
>     if (xcrb->resp_len < msg->fmt2.count1) {
>         pr_debug("resp_len %u < required %u => EMSGSIZE\n",
>                  (unsigned int)xcrb->resp_len, msg->fmt2.count1);
>         return -EMSGSIZE;
>     }
> 
> Does this cause valid padded hardware replies to be rejected with
> -EMSGSIZE for unaligned requests?
> 
>> 
>>  	cprb->cprb_len = sizeof(struct ep11_cprb);
>>  	cprb->cprb_ver_id = 0x04;

This may be valid but will eventually be covered with another patch 
outside of this series.