Re: [PATCH v2 06/35] crypto: Use kmemdup rather than duplicating its implementation

From: Michael S. Tsirkin
Date: Wed Jul 03 2019 - 14:51:56 EST


On Thu, Jul 04, 2019 at 12:27:08AM +0800, Fuqian Huang wrote:
> kmemdup is introduced to duplicate a region of memory in a neat way.
> Rather than kmalloc/kzalloc + memcpy, which the programmer needs to
> write the size twice (sometimes lead to mistakes), kmemdup improves
> readability, leads to smaller code and also reduce the chances of mistakes.
> Suggestion to use kmemdup rather than using kmalloc/kzalloc + memcpy.
>
> Signed-off-by: Fuqian Huang <huangfq.daxian@xxxxxxxxx>

virtio part:

Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx>


> ---
> Changes in v2:
> - Fix a typo in commit message (memset -> memcpy)
>
> drivers/crypto/caam/caampkc.c | 11 +++--------
> drivers/crypto/virtio/virtio_crypto_algs.c | 4 +---
> 2 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
> index fe24485274e1..a03464b4c019 100644
> --- a/drivers/crypto/caam/caampkc.c
> +++ b/drivers/crypto/caam/caampkc.c
> @@ -816,7 +816,7 @@ static int caam_rsa_set_pub_key(struct crypto_akcipher *tfm, const void *key,
> return ret;
>
> /* Copy key in DMA zone */
> - rsa_key->e = kzalloc(raw_key.e_sz, GFP_DMA | GFP_KERNEL);
> + rsa_key->e = kmemdup(raw_key.e, raw_key.e_sz, GFP_DMA | GFP_KERNEL);
> if (!rsa_key->e)
> goto err;
>
> @@ -838,8 +838,6 @@ static int caam_rsa_set_pub_key(struct crypto_akcipher *tfm, const void *key,
> rsa_key->e_sz = raw_key.e_sz;
> rsa_key->n_sz = raw_key.n_sz;
>
> - memcpy(rsa_key->e, raw_key.e, raw_key.e_sz);
> -
> return 0;
> err:
> caam_rsa_free_key(rsa_key);
> @@ -920,11 +918,11 @@ static int caam_rsa_set_priv_key(struct crypto_akcipher *tfm, const void *key,
> return ret;
>
> /* Copy key in DMA zone */
> - rsa_key->d = kzalloc(raw_key.d_sz, GFP_DMA | GFP_KERNEL);
> + rsa_key->d = kmemdup(raw_key.d, raw_key.d_sz, GFP_DMA | GFP_KERNEL);
> if (!rsa_key->d)
> goto err;
>
> - rsa_key->e = kzalloc(raw_key.e_sz, GFP_DMA | GFP_KERNEL);
> + rsa_key->e = kmemdup(raw_key.e, raw_key.e_sz, GFP_DMA | GFP_KERNEL);
> if (!rsa_key->e)
> goto err;
>
> @@ -947,9 +945,6 @@ static int caam_rsa_set_priv_key(struct crypto_akcipher *tfm, const void *key,
> rsa_key->e_sz = raw_key.e_sz;
> rsa_key->n_sz = raw_key.n_sz;
>
> - memcpy(rsa_key->d, raw_key.d, raw_key.d_sz);
> - memcpy(rsa_key->e, raw_key.e, raw_key.e_sz);
> -
> caam_rsa_set_priv_key_form(ctx, &raw_key);
>
> return 0;
> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c
> index 10f266d462d6..42d19205166b 100644
> --- a/drivers/crypto/virtio/virtio_crypto_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> @@ -129,13 +129,11 @@ static int virtio_crypto_alg_ablkcipher_init_session(
> * Avoid to do DMA from the stack, switch to using
> * dynamically-allocated for the key
> */
> - uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC);
> + uint8_t *cipher_key = kmemdup(key, keylen, GFP_ATOMIC);
>
> if (!cipher_key)
> return -ENOMEM;
>
> - memcpy(cipher_key, key, keylen);
> -
> spin_lock(&vcrypto->ctrl_lock);
> /* Pad ctrl header */
> vcrypto->ctrl.header.opcode =
> --
> 2.11.0