[PATCH v6 1/1] s390/zcrypt: Improve zcrypt reply message verification checks
Harald Freudenberger <[email protected]>
| Newsgroups | org.kernel.vger.linux-s390 |
|---|---|
| Message-ID | <[email protected]> |
Add or improve checks related to buffer sizes and reply sizes to the handling of replies from the crypto cards for CCA, EP11 (AP message type 6) and ICA (AP type 50) 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. Rework zcrypt_msgtype6_receive(), zcrypt_msgtype6_receive_ep11() and zcrypt_msgtype50_receive() to validate reply lengths more carefully before copying data back into the request buffer. Use size_t for length calculations, reject inconsistent reply sizes, and add defensive handling for short invalid replies. For XCRB replies, validate both reply segments and derive the effective message length from the covered range instead of trusting only the second segment. Signed-off-by: Harald Freudenberger <[email protected]> --- drivers/s390/crypto/zcrypt_msgtype50.c | 39 ++++--- drivers/s390/crypto/zcrypt_msgtype6.c | 151 ++++++++++++++++--------- 2 files changed, 126 insertions(+), 64 deletions(-) diff --git a/drivers/s390/crypto/zcrypt_msgtype50.c b/drivers/s390/crypto/zcrypt_msgtype50.c index d6fc2d8e7fad..ef925b399806 100644 --- a/drivers/s390/crypto/zcrypt_msgtype50.c +++ b/drivers/s390/crypto/zcrypt_msgtype50.c @@ -416,26 +416,39 @@ static void zcrypt_msgtype50_receive(struct ap_queue *aq, .reply_code = REP82_ERROR_MACHINE_FAILURE, }; struct type80_hdr *t80h; - int len; + size_t len; /* Copy the reply message to the request message buffer. */ if (!reply) goto out; /* ap_msg->rc indicates the error */ + t80h = reply->msg; - if (t80h->type == TYPE80_RSP_CODE) { - len = t80h->len; - if (len > reply->bufsize || len > msg->bufsize || - len != reply->len) { - pr_debug("len mismatch => EMSGSIZE\n"); - msg->rc = -EMSGSIZE; - goto out; - } - memcpy(msg->msg, reply->msg, len); - msg->len = len; - } else { - memcpy(msg->msg, reply->msg, sizeof(error_reply)); + + if (t80h->type != TYPE80_RSP_CODE) { + if (reply->len < sizeof(error_reply)) + memcpy(msg->msg, &error_reply, sizeof(error_reply)); + else + memcpy(msg->msg, reply->msg, sizeof(error_reply)); msg->len = sizeof(error_reply); + goto out; + } + + len = t80h->len; + if (len != reply->len) { + pr_warn_ratelimited("len %zu rpl.len %zu mismatch => EMSGSIZE\n", + len, reply->len); + msg->rc = -EMSGSIZE; + goto out; } + if (len > reply->bufsize || len > msg->bufsize) { + pr_warn_ratelimited("len %zu exceeds buf %zu/%zu => EMSGSIZE\n", + len, reply->bufsize, msg->bufsize); + msg->rc = -EMSGSIZE; + goto out; + } + memcpy(msg->msg, reply->msg, len); + msg->len = len; + out: complete(&msg->response.work); } diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c index 3df1d676de5d..b98449913e24 100644 --- a/drivers/s390/crypto/zcrypt_msgtype6.c +++ b/drivers/s390/crypto/zcrypt_msgtype6.c @@ -766,6 +766,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; } @@ -928,48 +935,75 @@ 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, len1, len2 = 0; /* Copy the reply message to the request message buffer. */ if (!reply) goto out; /* ap_msg->rc indicates the error */ + t86r = reply->msg; - if (t86r->hdr.type == TYPE86_RSP_CODE && - t86r->cprbx.cprb_ver_id == 0x02) { - switch (resp_type->type) { - case CEXXC_RESPONSE_TYPE_ICA: - len = sizeof(struct type86x_reply) + t86r->length; - if (len > reply->bufsize || len > msg->bufsize || - len != reply->len) { - pr_debug("len mismatch => EMSGSIZE\n"); - msg->rc = -EMSGSIZE; - goto out; - } - memcpy(msg->msg, reply->msg, len); - 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 (len > reply->bufsize || len > msg->bufsize || - len != reply->len) { - pr_debug("len mismatch => EMSGSIZE\n"); + + if (t86r->hdr.type != TYPE86_RSP_CODE || + t86r->cprbx.cprb_ver_id != 0x02) { + if (reply->len < sizeof(error_reply)) + memcpy(msg->msg, &error_reply, sizeof(error_reply)); + else + memcpy(msg->msg, reply->msg, sizeof(error_reply)); + msg->len = sizeof(error_reply); + goto out; + } + + switch (resp_type->type) { + case CEXXC_RESPONSE_TYPE_ICA: + len = sizeof(struct type86x_reply) + (size_t)t86r->length; + break; + case CEXXC_RESPONSE_TYPE_XCRB: + len1 = (size_t)t86r->fmt2.offset1 + (size_t)t86r->fmt2.count1; + if (t86r->fmt2.offset1 > reply->len || + t86r->fmt2.count1 > reply->len) { + pr_warn_ratelimited( + "offset1 %u count1 %u rpl.len %zu mismatch => EMSGSIZE\n", + t86r->fmt2.offset1, t86r->fmt2.count1, + reply->len); + msg->rc = -EMSGSIZE; + goto out; + } + if (t86r->fmt2.count2) { + len2 = (size_t)t86r->fmt2.offset2 + + (size_t)t86r->fmt2.count2; + if (t86r->fmt2.offset2 > reply->len || + t86r->fmt2.count2 > reply->len) { + pr_warn_ratelimited( + "offset2 %u count2 %u rpl.len %zu mismatch => EMSGSIZE\n", + t86r->fmt2.offset2, t86r->fmt2.count2, + reply->len); msg->rc = -EMSGSIZE; goto out; } - memcpy(msg->msg, reply->msg, len); - msg->len = len; - break; - default: - memcpy(msg->msg, &error_reply, sizeof(error_reply)); - msg->len = sizeof(error_reply); } - } else { - memcpy(msg->msg, reply->msg, sizeof(error_reply)); + len = max_t(size_t, len1, len2); + break; + default: + memcpy(msg->msg, &error_reply, sizeof(error_reply)); msg->len = sizeof(error_reply); + goto out; + } + + if (len != reply->len) { + pr_warn_ratelimited("len %zu rpl.len %zu mismatch => EMSGSIZE\n", + len, reply->len); + msg->rc = -EMSGSIZE; + goto out; } + if (len > reply->bufsize || len > msg->bufsize) { + pr_warn_ratelimited("len %zu exceeds buf %zu/%zu => EMSGSIZE\n", + len, reply->bufsize, msg->bufsize); + msg->rc = -EMSGSIZE; + goto out; + } + memcpy(msg->msg, reply->msg, len); + msg->len = len; + out: complete(&resp_type->work); } @@ -992,34 +1026,49 @@ 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) goto out; /* ap_msg->rc indicates the error */ + t86r = reply->msg; - if (t86r->hdr.type == TYPE86_RSP_CODE && - t86r->cprbx.cprb_ver_id == 0x04) { - switch (resp_type->type) { - case CEXXC_RESPONSE_TYPE_EP11: - len = t86r->fmt2.offset1 + t86r->fmt2.count1; - if (len > reply->bufsize || len > msg->bufsize || - len != reply->len) { - pr_debug("len mismatch => EMSGSIZE\n"); - msg->rc = -EMSGSIZE; - goto out; - } - memcpy(msg->msg, reply->msg, len); - msg->len = len; - break; - default: + + if (t86r->hdr.type != TYPE86_RSP_CODE || + t86r->cprbx.cprb_ver_id != 0x04) { + if (reply->len < sizeof(error_reply)) memcpy(msg->msg, &error_reply, sizeof(error_reply)); - msg->len = sizeof(error_reply); - } - } else { - memcpy(msg->msg, reply->msg, sizeof(error_reply)); + else + memcpy(msg->msg, reply->msg, sizeof(error_reply)); msg->len = sizeof(error_reply); + goto out; } + + switch (resp_type->type) { + case CEXXC_RESPONSE_TYPE_EP11: + len = t86r->fmt2.offset1 + (size_t)t86r->fmt2.count1; + break; + default: + memcpy(msg->msg, &error_reply, sizeof(error_reply)); + msg->len = sizeof(error_reply); + goto out; + } + + if (len != reply->len) { + pr_warn_ratelimited("len %zu rpl.len %zu mismatch => EMSGSIZE\n", + len, reply->len); + msg->rc = -EMSGSIZE; + goto out; + } + if (len > reply->bufsize || len > msg->bufsize) { + pr_warn_ratelimited("len %zu exceeds buf %zu/%zu => EMSGSIZE\n", + len, reply->bufsize, msg->bufsize); + msg->rc = -EMSGSIZE; + goto out; + } + memcpy(msg->msg, reply->msg, len); + msg->len = len; + out: complete(&resp_type->work); } -- 2.43.0