ACK: [SRU][Bionic][PATCH 1/1] s390/zcrypt: Fix CCA and EP11 CPRB processing failure memory leak.
Stefan Bader
stefan.bader at canonical.com
Fri Jun 15 14:51:24 UTC 2018
On 13.06.2018 18:12, Joseph Salisbury wrote:
> From: Harald Freudenberger <freude at de.ibm.com>
>
> BugLink: http://bugs.launchpad.net/bugs/1775390
>
> Tests showed, that the zcrypt device driver produces memory
> leaks when a valid CCA or EP11 CPRB can't get delivered or has
> a failure during processing within the zcrypt device driver.
>
> This happens when a invalid domain or adapter number is used
> or the lower level software or hardware layers produce any
> kind of failure during processing of the request.
>
> Only CPRBs send to CCA or EP11 cards can produce this memory
> leak. The accelerator and the CPRBs processed by this type
> of crypto card is not affected.
>
> The two fields message and private within the ap_message struct
> are allocated with pulling the function code for the CPRB but
> only freed when processing of the CPRB succeeds. So for example
> an invalid domain or adapter field causes the processing to
> fail, leaving these two memory areas allocated forever.
>
> Signed-off-by: Harald Freudenberger <freude at de.ibm.com>
> Reviewed-by: Ingo Franzki <ifranzki at de.ibm.com>
> Signed-off-by: Martin Schwidefsky <schwidefsky at de.ibm.com>
> (cherry picked from commit 89a0c0ec0d2e3ce0ee9caa00f60c0c26ccf11c21)
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
> drivers/s390/crypto/ap_bus.h | 17 ++++++++----
> drivers/s390/crypto/zcrypt_api.c | 15 ++++++++---
> drivers/s390/crypto/zcrypt_msgtype6.c | 51 +++++++++++++----------------------
> 3 files changed, 43 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h
> index e0827ea..4c82946 100644
> --- a/drivers/s390/crypto/ap_bus.h
> +++ b/drivers/s390/crypto/ap_bus.h
> @@ -198,11 +198,18 @@ struct ap_message {
> */
> static inline void ap_init_message(struct ap_message *ap_msg)
> {
> - ap_msg->psmid = 0;
> - ap_msg->length = 0;
> - ap_msg->rc = 0;
> - ap_msg->special = 0;
> - ap_msg->receive = NULL;
> + memset(ap_msg, 0, sizeof(*ap_msg));
> +}
> +
> +/**
> + * ap_release_message() - Release ap_message.
> + * Releases all memory used internal within the ap_message struct
> + * Currently this is the message and private field.
> + */
> +static inline void ap_release_message(struct ap_message *ap_msg)
> +{
> + kzfree(ap_msg->message);
> + kzfree(ap_msg->private);
> }
>
> #define for_each_ap_card(_ac) \
> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index ce15f10..9a8250d 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -373,6 +373,7 @@ long zcrypt_send_cprb(struct ica_xcRB *xcRB)
>
> trace_s390_zcrypt_req(xcRB, TB_ZSECSENDCPRB);
>
> + ap_init_message(&ap_msg);
> rc = get_cprb_fc(xcRB, &ap_msg, &func_code, &domain);
> if (rc)
> goto out;
> @@ -427,6 +428,7 @@ long zcrypt_send_cprb(struct ica_xcRB *xcRB)
> spin_unlock(&zcrypt_list_lock);
>
> out:
> + ap_release_message(&ap_msg);
> trace_s390_zcrypt_rep(xcRB, func_code, rc,
> AP_QID_CARD(qid), AP_QID_QUEUE(qid));
> return rc;
> @@ -470,6 +472,8 @@ static long zcrypt_send_ep11_cprb(struct ep11_urb *xcrb)
>
> trace_s390_zcrypt_req(xcrb, TP_ZSENDEP11CPRB);
>
> + ap_init_message(&ap_msg);
> +
> target_num = (unsigned short) xcrb->targets_num;
>
> /* empty list indicates autoselect (all available targets) */
> @@ -487,7 +491,7 @@ static long zcrypt_send_ep11_cprb(struct ep11_urb *xcrb)
> if (copy_from_user(targets, uptr,
> target_num * sizeof(*targets))) {
> rc = -EFAULT;
> - goto out;
> + goto out_free;
> }
> }
>
> @@ -544,6 +548,7 @@ static long zcrypt_send_ep11_cprb(struct ep11_urb *xcrb)
> out_free:
> kfree(targets);
> out:
> + ap_release_message(&ap_msg);
> trace_s390_zcrypt_rep(xcrb, func_code, rc,
> AP_QID_CARD(qid), AP_QID_QUEUE(qid));
> return rc;
> @@ -561,6 +566,7 @@ static long zcrypt_rng(char *buffer)
>
> trace_s390_zcrypt_req(buffer, TP_HWRNGCPRB);
>
> + ap_init_message(&ap_msg);
> rc = get_rng_fc(&ap_msg, &func_code, &domain);
> if (rc)
> goto out;
> @@ -591,8 +597,10 @@ static long zcrypt_rng(char *buffer)
> pref_zq = zcrypt_pick_queue(pref_zc, pref_zq, weight);
> spin_unlock(&zcrypt_list_lock);
>
> - if (!pref_zq)
> - return -ENODEV;
> + if (!pref_zq) {
> + rc = -ENODEV;
> + goto out;
> + }
>
> qid = pref_zq->queue->qid;
> rc = pref_zq->ops->rng(pref_zq, buffer, &ap_msg);
> @@ -602,6 +610,7 @@ static long zcrypt_rng(char *buffer)
> spin_unlock(&zcrypt_list_lock);
>
> out:
> + ap_release_message(&ap_msg);
> trace_s390_zcrypt_rep(buffer, func_code, rc,
> AP_QID_CARD(qid), AP_QID_QUEUE(qid));
> return rc;
> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index f54bef4..97d4bac 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> @@ -1084,6 +1084,13 @@ static long zcrypt_msgtype6_modexpo_crt(struct zcrypt_queue *zq,
> return rc;
> }
>
> +/**
> + * Fetch function code from cprb.
> + * Extracting the fc requires to copy the cprb from userspace.
> + * So this function allocates memory and needs an ap_msg prepared
> + * by the caller with ap_init_message(). Also the caller has to
> + * make sure ap_release_message() is always called even on failure.
> + */
> unsigned int get_cprb_fc(struct ica_xcRB *xcRB,
> struct ap_message *ap_msg,
> unsigned int *func_code, unsigned short **dom)
> @@ -1091,9 +1098,7 @@ unsigned int get_cprb_fc(struct ica_xcRB *xcRB,
> struct response_type resp_type = {
> .type = PCIXCC_RESPONSE_TYPE_XCRB,
> };
> - int rc;
>
> - ap_init_message(ap_msg);
> ap_msg->message = kmalloc(MSGTYPE06_MAX_MSG_SIZE, GFP_KERNEL);
> if (!ap_msg->message)
> return -ENOMEM;
> @@ -1101,17 +1106,10 @@ unsigned int get_cprb_fc(struct ica_xcRB *xcRB,
> ap_msg->psmid = (((unsigned long long) current->pid) << 32) +
> atomic_inc_return(&zcrypt_step);
> ap_msg->private = kmalloc(sizeof(resp_type), GFP_KERNEL);
> - if (!ap_msg->private) {
> - kzfree(ap_msg->message);
> + if (!ap_msg->private)
> return -ENOMEM;
> - }
> memcpy(ap_msg->private, &resp_type, sizeof(resp_type));
> - rc = XCRB_msg_to_type6CPRB_msgX(ap_msg, xcRB, func_code, dom);
> - if (rc) {
> - kzfree(ap_msg->message);
> - kzfree(ap_msg->private);
> - }
> - return rc;
> + return XCRB_msg_to_type6CPRB_msgX(ap_msg, xcRB, func_code, dom);
> }
>
> /**
> @@ -1139,11 +1137,16 @@ static long zcrypt_msgtype6_send_cprb(struct zcrypt_queue *zq,
> /* Signal pending. */
> ap_cancel_message(zq->queue, ap_msg);
>
> - kzfree(ap_msg->message);
> - kzfree(ap_msg->private);
> return rc;
> }
>
> +/**
> + * Fetch function code from ep11 cprb.
> + * Extracting the fc requires to copy the ep11 cprb from userspace.
> + * So this function allocates memory and needs an ap_msg prepared
> + * by the caller with ap_init_message(). Also the caller has to
> + * make sure ap_release_message() is always called even on failure.
> + */
> unsigned int get_ep11cprb_fc(struct ep11_urb *xcrb,
> struct ap_message *ap_msg,
> unsigned int *func_code)
> @@ -1151,9 +1154,7 @@ unsigned int get_ep11cprb_fc(struct ep11_urb *xcrb,
> struct response_type resp_type = {
> .type = PCIXCC_RESPONSE_TYPE_EP11,
> };
> - int rc;
>
> - ap_init_message(ap_msg);
> ap_msg->message = kmalloc(MSGTYPE06_MAX_MSG_SIZE, GFP_KERNEL);
> if (!ap_msg->message)
> return -ENOMEM;
> @@ -1161,17 +1162,10 @@ unsigned int get_ep11cprb_fc(struct ep11_urb *xcrb,
> ap_msg->psmid = (((unsigned long long) current->pid) << 32) +
> atomic_inc_return(&zcrypt_step);
> ap_msg->private = kmalloc(sizeof(resp_type), GFP_KERNEL);
> - if (!ap_msg->private) {
> - kzfree(ap_msg->message);
> + if (!ap_msg->private)
> return -ENOMEM;
> - }
> memcpy(ap_msg->private, &resp_type, sizeof(resp_type));
> - rc = xcrb_msg_to_type6_ep11cprb_msgx(ap_msg, xcrb, func_code);
> - if (rc) {
> - kzfree(ap_msg->message);
> - kzfree(ap_msg->private);
> - }
> - return rc;
> + return xcrb_msg_to_type6_ep11cprb_msgx(ap_msg, xcrb, func_code);
> }
>
> /**
> @@ -1246,8 +1240,6 @@ static long zcrypt_msgtype6_send_ep11_cprb(struct zcrypt_queue *zq,
> /* Signal pending. */
> ap_cancel_message(zq->queue, ap_msg);
>
> - kzfree(ap_msg->message);
> - kzfree(ap_msg->private);
> return rc;
> }
>
> @@ -1258,7 +1250,6 @@ unsigned int get_rng_fc(struct ap_message *ap_msg, int *func_code,
> .type = PCIXCC_RESPONSE_TYPE_XCRB,
> };
>
> - ap_init_message(ap_msg);
> ap_msg->message = kmalloc(MSGTYPE06_MAX_MSG_SIZE, GFP_KERNEL);
> if (!ap_msg->message)
> return -ENOMEM;
> @@ -1266,10 +1257,8 @@ unsigned int get_rng_fc(struct ap_message *ap_msg, int *func_code,
> ap_msg->psmid = (((unsigned long long) current->pid) << 32) +
> atomic_inc_return(&zcrypt_step);
> ap_msg->private = kmalloc(sizeof(resp_type), GFP_KERNEL);
> - if (!ap_msg->private) {
> - kzfree(ap_msg->message);
> + if (!ap_msg->private)
> return -ENOMEM;
> - }
> memcpy(ap_msg->private, &resp_type, sizeof(resp_type));
>
> rng_type6CPRB_msgX(ap_msg, ZCRYPT_RNG_BUFFER_SIZE, domain);
> @@ -1313,8 +1302,6 @@ static long zcrypt_msgtype6_rng(struct zcrypt_queue *zq,
> /* Signal pending. */
> ap_cancel_message(zq->queue, ap_msg);
>
> - kzfree(ap_msg->message);
> - kzfree(ap_msg->private);
> return rc;
> }
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180615/22a7a33a/attachment.sig>
More information about the kernel-team
mailing list