* [PATCH v3 0/1] Improve zcrypt reply message verification checks
@ 2026-07-09 9:22 Harald Freudenberger
2026-07-09 9:22 ` [PATCH v3 1/1] s390/zcrypt: " Harald Freudenberger
0 siblings, 1 reply; 3+ messages in thread
From: Harald Freudenberger @ 2026-07-09 9:22 UTC (permalink / raw)
To: fcallies
Cc: freude, linux-s390, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, dengler
Add or improve checks related to buffer sizes and reply sizes to the
handling of replies from the crypto cards for CCA and EP11 (AP message
type 6) messages. The verification code related to reply field length
was not designed well and thus firmware deficiencies could lead to
unexpected behavior in the zcrypt device driver. Thus improve the code
to more closely inspect especially length fields at message replies.
Changelog:
v1 - initial patch
v2 - fixed typo in header check_for_overflow -> check_add_overflow.
v3 - rephrased and smoothed subject and text of the patch. It is now
"s390/zcrypt: Improve zcrypt reply message verification checks"
and the text does not talk about malicious cards any more.
Updated Reviewed-by tags
Harald Freudenberger (1):
s390/zcrypt: Improve zcrypt reply message verification checks
drivers/s390/crypto/zcrypt_msgtype6.c | 39 ++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 7 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v3 1/1] s390/zcrypt: Improve zcrypt reply message verification checks 2026-07-09 9:22 [PATCH v3 0/1] Improve zcrypt reply message verification checks Harald Freudenberger @ 2026-07-09 9:22 ` Harald Freudenberger 2026-07-09 9:37 ` sashiko-bot 0 siblings, 1 reply; 3+ messages in thread From: Harald Freudenberger @ 2026-07-09 9:22 UTC (permalink / raw) To: fcallies Cc: freude, linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, dengler Add or improve checks related to buffer sizes and reply sizes to the handling of replies from the crypto cards for CCA and EP11 (AP message type 6) messages. The verification code related to reply field length was not designed well and thus firmware deficiencies could lead to unexpected behavior in the zcrypt device driver. Thus improve the code to more closely inspect especially length fields at message replies. The 3 hunks of this patch deal with CCA, EP11 and (CCA) RNG replies and improve the checking for reply buffer size by using size_t instead of int and the check_add_overflow() function. For RNG replies an additional check makes sure the hard coded limit of the data buffer is not exceeded. Signed-off-by: Harald Freudenberger <freude@linux.ibm.com> Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com> Reviewed-by: Holger Dengler <dengler@linux.ibm.com> Cc: stable@vger.kernel.org --- drivers/s390/crypto/zcrypt_msgtype6.c | 39 ++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c index 40f72cdf284d..5deef2e38f06 100644 --- a/drivers/s390/crypto/zcrypt_msgtype6.c +++ b/drivers/s390/crypto/zcrypt_msgtype6.c @@ -691,6 +691,13 @@ static int convert_type86_rng(struct zcrypt_queue *zq, if (msg->cprbx.ccp_rtcode != 0 || msg->cprbx.ccp_rscode != 0) return -EINVAL; + /* + * Note that offset2 and count2 have already been checked in + * zcrypt_msgtype6_receive(). So only check for not exceeding + * the hard coded rng buffer size. + */ + if (msg->fmt2.count2 > ZCRYPT_RNG_BUFFER_SIZE) + return -EMSGSIZE; memcpy(buffer, data + msg->fmt2.offset2, msg->fmt2.count2); return msg->fmt2.count2; } @@ -853,7 +860,7 @@ static void zcrypt_msgtype6_receive(struct ap_queue *aq, }; struct ap_response_type *resp_type = &msg->response; struct type86x_reply *t86r; - int len; + size_t len; /* Copy the reply message to the request message buffer. */ if (!reply) @@ -874,10 +881,23 @@ static void zcrypt_msgtype6_receive(struct ap_queue *aq, msg->len = len; break; case CEXXC_RESPONSE_TYPE_XCRB: - if (t86r->fmt2.count2) - len = t86r->fmt2.offset2 + t86r->fmt2.count2; - else - len = t86r->fmt2.offset1 + t86r->fmt2.count1; + if (t86r->fmt2.count2) { + if (check_add_overflow(t86r->fmt2.offset2, + t86r->fmt2.count2, + &len)) { + pr_debug("len overflow => EMSGSIZE\n"); + msg->rc = -EMSGSIZE; + goto out; + } + } else { + if (check_add_overflow(t86r->fmt2.offset1, + t86r->fmt2.count1, + &len)) { + pr_debug("len overflow => EMSGSIZE\n"); + msg->rc = -EMSGSIZE; + goto out; + } + } if (len > reply->bufsize || len > msg->bufsize || len != reply->len) { pr_debug("len mismatch => EMSGSIZE\n"); @@ -917,7 +937,7 @@ static void zcrypt_msgtype6_receive_ep11(struct ap_queue *aq, }; struct ap_response_type *resp_type = &msg->response; struct type86_ep11_reply *t86r; - int len; + size_t len; /* Copy the reply message to the request message buffer. */ if (!reply) @@ -927,7 +947,12 @@ static void zcrypt_msgtype6_receive_ep11(struct ap_queue *aq, t86r->cprbx.cprb_ver_id == 0x04) { switch (resp_type->type) { case CEXXC_RESPONSE_TYPE_EP11: - len = t86r->fmt2.offset1 + t86r->fmt2.count1; + if (check_add_overflow(t86r->fmt2.offset1, + t86r->fmt2.count1, &len)) { + pr_debug("len overflow => EMSGSIZE\n"); + msg->rc = -EMSGSIZE; + goto out; + } if (len > reply->bufsize || len > msg->bufsize || len != reply->len) { pr_debug("len mismatch => EMSGSIZE\n"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3 1/1] s390/zcrypt: Improve zcrypt reply message verification checks 2026-07-09 9:22 ` [PATCH v3 1/1] s390/zcrypt: " Harald Freudenberger @ 2026-07-09 9:37 ` sashiko-bot 0 siblings, 0 replies; 3+ messages in thread From: sashiko-bot @ 2026-07-09 9:37 UTC (permalink / raw) To: Harald Freudenberger Cc: Heiko Carstens, Vasily Gorbik, linux-s390, Alexander Gordeev Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [Low] The `check_add_overflow()` error paths are mathematically unreachable because 32-bit operands cannot overflow a 64-bit `size_t` destination. Pre-existing issues: - [High] Unvalidated `offset1` and `count1` fields in `zcrypt_msgtype6_receive()` lead to a kernel memory leak in `convert_type86_xcrb()` when `count2 != 0`. -- --- Patch [1]: [PATCH v3 1/1] s390/zcrypt: Improve zcrypt reply message verification checks --- commit b96e9311c01803c5c1f3023f735aa7268d688c81 Author: Harald Freudenberger <freude@linux.ibm.com> s390/zcrypt: Improve zcrypt reply message verification checks This commit adds and improves checks related to buffer sizes and reply sizes in the handling of replies from crypto cards for CCA and EP11 messages. It changes length variables from int to size_t and adds overflow checks to inspect length fields more closely. Cc: stable@vger.kernel.org > diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c > index 40f72cdf284d3..5deef2e38f065 100644 > --- a/drivers/s390/crypto/zcrypt_msgtype6.c > +++ b/drivers/s390/crypto/zcrypt_msgtype6.c [ ... ] > @@ -874,10 +881,23 @@ static void zcrypt_msgtype6_receive(struct ap_queue *aq, > msg->len = len; > break; > case CEXXC_RESPONSE_TYPE_XCRB: > - if (t86r->fmt2.count2) > - len = t86r->fmt2.offset2 + t86r->fmt2.count2; > - else > - len = t86r->fmt2.offset1 + t86r->fmt2.count1; > + if (t86r->fmt2.count2) { > + if (check_add_overflow(t86r->fmt2.offset2, > + t86r->fmt2.count2, > + &len)) { [Severity: High] This is a pre-existing issue, but does this conditional validation leave offset1 and count1 unchecked when count2 is non-zero? If the hardware or firmware returns a malformed XCRB reply with a non-zero count2, only offset2 and count2 are validated here against the buffer bounds. However, convert_type86_xcrb() appears to unconditionally copy count1 bytes from offset1 to userspace without validating if they remain within the bounds of the reply message: 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; ... } Can this lead to an out-of-bounds memory read and an information leak to userspace? [Severity: Low] Is this check_add_overflow() mathematically reachable? Since len was changed to a size_t, which is strictly 64-bit on the s390x architecture, and the offset and count variables are 32-bit unsigned integers, their sum can never overflow the 64-bit destination. Does this make the overflow check always evaluate to false and the error branch dead code? > + pr_debug("len overflow => EMSGSIZE\n"); > + msg->rc = -EMSGSIZE; > + goto out; > + } > + } else { > + if (check_add_overflow(t86r->fmt2.offset1, > + t86r->fmt2.count1, > + &len)) { > + pr_debug("len overflow => EMSGSIZE\n"); > + msg->rc = -EMSGSIZE; > + goto out; > + } > + } > if (len > reply->bufsize || len > msg->bufsize || > len != reply->len) { > pr_debug("len mismatch => EMSGSIZE\n"); [ ... ] > @@ -927,7 +947,12 @@ static void zcrypt_msgtype6_receive_ep11(struct ap_queue *aq, > t86r->cprbx.cprb_ver_id == 0x04) { > switch (resp_type->type) { > case CEXXC_RESPONSE_TYPE_EP11: > - len = t86r->fmt2.offset1 + t86r->fmt2.count1; > + if (check_add_overflow(t86r->fmt2.offset1, > + t86r->fmt2.count1, &len)) { [Severity: Low] Is this check_add_overflow() also mathematically unreachable for the same reasons mentioned above? > + pr_debug("len overflow => EMSGSIZE\n"); > + msg->rc = -EMSGSIZE; > + goto out; > + } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260709092219.57356-2-freude@linux.ibm.com?part=1 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-09 9:37 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-09 9:22 [PATCH v3 0/1] Improve zcrypt reply message verification checks Harald Freudenberger 2026-07-09 9:22 ` [PATCH v3 1/1] s390/zcrypt: " Harald Freudenberger 2026-07-09 9:37 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox