diff mbox

[v2,for-2.10,08/18] crypto: hmac: introduce qcrypto_hmac_ctx_new for gcrypt-backend

Message ID 1492392806-53720-9-git-send-email-longpeng2@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Longpeng(Mike) April 17, 2017, 1:33 a.m. UTC
1) Fix a handle-leak problem in qcrypto_hmac_new(), doesn't free
   ctx->handle if gcry_mac_setkey fails.

2) Extracts qcrypto_hmac_ctx_new() from qcrypto_hmac_new() for
   gcrypt-backend impls.

Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
---
 crypto/hmac-gcrypt.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

Comments

Gonglei (Arei) April 21, 2017, 12:15 p.m. UTC | #1
> -----Original Message-----
> From: longpeng
> Sent: Monday, April 17, 2017 9:33 AM
> To: berrange@redhat.com
> Cc: Gonglei (Arei); Huangweidong (C); armbru@redhat.com;
> eblake@redhat.com; mst@redhat.com; qemu-devel@nongnu.org; longpeng
> Subject: [PATCH v2 for-2.10 08/18] crypto: hmac: introduce
> qcrypto_hmac_ctx_new for gcrypt-backend
> 
> 1) Fix a handle-leak problem in qcrypto_hmac_new(), doesn't free
>    ctx->handle if gcry_mac_setkey fails.
> 
> 2) Extracts qcrypto_hmac_ctx_new() from qcrypto_hmac_new() for
>    gcrypt-backend impls.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---
>  crypto/hmac-gcrypt.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


> diff --git a/crypto/hmac-gcrypt.c b/crypto/hmac-gcrypt.c
> index 21189e6..42489f3 100644
> --- a/crypto/hmac-gcrypt.c
> +++ b/crypto/hmac-gcrypt.c
> @@ -42,11 +42,11 @@ bool qcrypto_hmac_supports(QCryptoHashAlgorithm
> alg)
>      return false;
>  }
> 
> -QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
> -                              const uint8_t *key, size_t nkey,
> -                              Error **errp)
> +static QCryptoHmacGcrypt *
> +qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
> +                     const uint8_t *key, size_t nkey,
> +                     Error **errp)
>  {
> -    QCryptoHmac *hmac;
>      QCryptoHmacGcrypt *ctx;
>      gcry_error_t err;
> 
> @@ -56,9 +56,6 @@ QCryptoHmac
> *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
>          return NULL;
>      }
> 
> -    hmac = g_new0(QCryptoHmac, 1);
> -    hmac->alg = alg;
> -
>      ctx = g_new0(QCryptoHmacGcrypt, 1);
> 
>      err = gcry_mac_open(&ctx->handle, qcrypto_hmac_alg_map[alg],
> @@ -73,15 +70,14 @@ QCryptoHmac
> *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
>      if (err != 0) {
>          error_setg(errp, "Cannot set key: %s",
>                     gcry_strerror(err));
> +        gcry_mac_close(ctx->handle);
>          goto error;
>      }
> 
> -    hmac->opaque = ctx;
> -    return hmac;
> +    return ctx;
> 
>  error:
>      g_free(ctx);
> -    g_free(hmac);
>      return NULL;
>  }
> 
> @@ -150,3 +146,22 @@ int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
> 
>      return 0;
>  }
> +
> +QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
> +                              const uint8_t *key, size_t nkey,
> +                              Error **errp)
> +{
> +    QCryptoHmac *hmac;
> +    QCryptoHmacGcrypt *ctx;
> +
> +    ctx = qcrypto_hmac_ctx_new(alg, key, nkey, errp);
> +    if (ctx == NULL) {
> +        return NULL;
> +    }
> +
> +    hmac = g_new0(QCryptoHmac, 1);
> +    hmac->alg = alg;
> +    hmac->opaque = ctx;
> +
> +    return hmac;
> +}
> --
> 1.8.3.1
>
diff mbox

Patch

diff --git a/crypto/hmac-gcrypt.c b/crypto/hmac-gcrypt.c
index 21189e6..42489f3 100644
--- a/crypto/hmac-gcrypt.c
+++ b/crypto/hmac-gcrypt.c
@@ -42,11 +42,11 @@  bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
     return false;
 }
 
-QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
-                              const uint8_t *key, size_t nkey,
-                              Error **errp)
+static QCryptoHmacGcrypt *
+qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
+                     const uint8_t *key, size_t nkey,
+                     Error **errp)
 {
-    QCryptoHmac *hmac;
     QCryptoHmacGcrypt *ctx;
     gcry_error_t err;
 
@@ -56,9 +56,6 @@  QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
         return NULL;
     }
 
-    hmac = g_new0(QCryptoHmac, 1);
-    hmac->alg = alg;
-
     ctx = g_new0(QCryptoHmacGcrypt, 1);
 
     err = gcry_mac_open(&ctx->handle, qcrypto_hmac_alg_map[alg],
@@ -73,15 +70,14 @@  QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
     if (err != 0) {
         error_setg(errp, "Cannot set key: %s",
                    gcry_strerror(err));
+        gcry_mac_close(ctx->handle);
         goto error;
     }
 
-    hmac->opaque = ctx;
-    return hmac;
+    return ctx;
 
 error:
     g_free(ctx);
-    g_free(hmac);
     return NULL;
 }
 
@@ -150,3 +146,22 @@  int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
 
     return 0;
 }
+
+QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
+                              const uint8_t *key, size_t nkey,
+                              Error **errp)
+{
+    QCryptoHmac *hmac;
+    QCryptoHmacGcrypt *ctx;
+
+    ctx = qcrypto_hmac_ctx_new(alg, key, nkey, errp);
+    if (ctx == NULL) {
+        return NULL;
+    }
+
+    hmac = g_new0(QCryptoHmac, 1);
+    hmac->alg = alg;
+    hmac->opaque = ctx;
+
+    return hmac;
+}