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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 3 Jul 2019 14:51:41 -0400
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Fuqian Huang <huangfq.daxian@...il.com>
Cc:     Horia Geantă <horia.geanta@....com>,
        Aymen Sghaier <aymen.sghaier@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Gonglei <arei.gonglei@...wei.com>,
        Jason Wang <jasowang@...hat.com>, linux-crypto@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v2 06/35] crypto: Use kmemdup rather than duplicating its
 implementation

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@...il.com>

virtio part:

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


> ---
> 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