[PATCH v1 1/1] s390/zcrypt: Improve length check and overflow detection at zcrypt msg type 6

[Harald Freudenberger <freude@linux.ibm.com>]

The existing overflow checks in zcrypt_msgtype6.c in function
xcrb_msg_to_type6cprb_msgx() and xcrb_msg_to_type6_ep11cprb_msgx() had
several security weaknesses:

1. The CEIL4() macro used for overflow detection has itself an
   overflow problem.
2. Post-calculation overflow detection could allow undefined behavior
   to occur before the check was performed.
2. Missing overflow checks on message length calculations created
   potential buffer overflow vulnerabilities.
3. Use of signed integer types for size calculations risked signed
   integer overflow undefined behavior.

The patch improves this by:

- Implementing overflow detection before the CEIL4() macro is used via
  kernel function check_add_overflow().
- Adding explicit overflow checks for all message length calculations
  to eliminate buffer overflow risks.
- Converting size variables from signed to unsigned types and thus
  prevents from signed overflow.
- Improving code readability by replacing complex ternary operators
  with clear if-else structures.

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

diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
index 40f72cdf284d..8bf735a64383 100644
--- a/drivers/s390/crypto/zcrypt_msgtype6.c
+++ b/drivers/s390/crypto/zcrypt_msgtype6.c
@@ -342,50 +342,49 @@ 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;
+	u32 u, req_cblen, rep_cblen, req_sumlen, rep_sumlen;
+	char *req_data, *function_code;
 
-	if (CEIL4(xcrb->request_control_blk_length) <
-			xcrb->request_control_blk_length)
-		return -EINVAL; /* overflow after alignment*/
+	if (check_add_overflow(xcrb->request_control_blk_length, 3, &u))
+		return -EINVAL; /* would overflow at CEIL4 */
+	req_cblen = CEIL4(xcrb->request_control_blk_length);
 
 	/* length checks */
-	ap_msg->len = sizeof(struct type6_hdr) +
-		CEIL4(xcrb->request_control_blk_length) +
-		xcrb->request_data_length;
-	if (ap_msg->len > ap_msg->bufsize)
+	if (check_add_overflow(sizeof(struct type6_hdr), req_cblen, &u))
 		return -EINVAL;
+	if (check_add_overflow(u, xcrb->request_data_length, &u))
+		return -EINVAL;
+	ap_msg->len = u;
 
 	/*
 	 * 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 < xcrb->request_data_length :
-	    req_sumlen < CEIL4(xcrb->request_control_blk_length)) {
-		return -EINVAL;
+	req_sumlen = req_cblen + xcrb->request_data_length;
+	if (req_cblen <= xcrb->request_data_length) {
+		if (req_sumlen < xcrb->request_data_length)
+			return -EINVAL;
+	} else {
+		if (req_sumlen < req_cblen)
+			return -EINVAL;
+
 	}
 
-	if (CEIL4(xcrb->reply_control_blk_length) <
-			xcrb->reply_control_blk_length)
-		return -EINVAL; /* overflow after alignment*/
+	if (check_add_overflow(xcrb->reply_control_blk_length, 3, &u))
+		return -EINVAL; /* would overflow at CEIL4 */
+	rep_cblen = CEIL4(xcrb->reply_control_blk_length);
 
 	/*
 	 * 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)) {
-		return -EINVAL;
+	rep_sumlen = rep_cblen + xcrb->reply_data_length;
+	if (rep_cblen <= xcrb->reply_data_length) {
+		if (rep_sumlen < xcrb->reply_data_length)
+			return -EINVAL;
+	} else {
+		if (rep_sumlen < rep_cblen)
+			return -EINVAL;
 	}
 
 	/* prepare type6 header */
@@ -393,7 +392,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;
@@ -437,10 +436,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;
 }
@@ -480,16 +482,20 @@ static int xcrb_msg_to_type6_ep11cprb_msgx(bool userspace, struct ap_message *ap
 		unsigned int	dom_val;	/* domain id	   */
 	} __packed * payload_hdr = NULL;
 
-	if (CEIL4(xcrb->req_len) < xcrb->req_len)
-		return -EINVAL; /* overflow after alignment*/
+	u64 u;
+
+	if (check_add_overflow(xcrb->req_len, 3, &u))
+		return -EINVAL; /* would overflow when used with CEIL4 */
 
 	/* length checks */
-	ap_msg->len = sizeof(struct type6_hdr) + CEIL4(xcrb->req_len);
-	if (ap_msg->len > ap_msg->bufsize)
+	if (check_add_overflow(sizeof(struct type6_hdr),
+			       CEIL4(xcrb->req_len), &u) ||
+	    u > ap_msg->bufsize)
 		return -EINVAL;
+	ap_msg->len = u;
 
-	if (CEIL4(xcrb->resp_len) < xcrb->resp_len)
-		return -EINVAL; /* overflow after alignment*/
+	if (check_add_overflow(xcrb->resp_len, 3, &u))
+		return -EINVAL; /* would overflow when used with CEIL4 */
 
 	/* prepare type6 header */
 	msg->hdr = static_type6_ep11_hdr;
-- 
2.43.0