linux-kernel - Re: [PATCH v2 6/9] crypto: virtio: Add req_data with structure virtio_crypto_sym

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]

Message-ID: <20251204074310-mutt-send-email-mst@kernel.org>
Date: Thu, 4 Dec 2025 07:46:11 -0500
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Bibo Mao <maobibo@...ngson.cn>
Cc: Gonglei <arei.gonglei@...wei.com>, Jason Wang <jasowang@...hat.com>,
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
	Eugenio Pérez <eperezma@...hat.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	"David S. Miller" <davem@...emloft.net>,
	virtualization@...ts.linux.dev, linux-crypto@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/9] crypto: virtio: Add req_data with structure
 virtio_crypto_sym_request

On Thu, Dec 04, 2025 at 07:22:23PM +0800, Bibo Mao wrote:
> With normal encrypt/decrypt workflow, req_data with struct type
> virtio_crypto_op_data_req will be allocated. Here put req_data in
> virtio_crypto_sym_request, it is pre-allocated when encrypt/decrypt
> interface is called.
> 
> Signed-off-by: Bibo Mao <maobibo@...ngson.cn>
> ---
>  drivers/crypto/virtio/virtio_crypto_core.c          |  3 ++-
>  drivers/crypto/virtio/virtio_crypto_skcipher_algs.c | 12 +++---------
>  2 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
> index ccc6b5c1b24b..e60ad1d94e7f 100644
> --- a/drivers/crypto/virtio/virtio_crypto_core.c
> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> @@ -17,7 +17,8 @@ void
>  virtcrypto_clear_request(struct virtio_crypto_request *vc_req)
>  {
>  	if (vc_req) {
> -		kfree_sensitive(vc_req->req_data);
> +		if (vc_req->req_data)
> +			kfree_sensitive(vc_req->req_data);

kfree of NULL is a nop, why make this change?

>  		kfree(vc_req->sgs);
>  	}
>  }
> diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
> index 7b3f21a40d78..a7c7c726e6d9 100644
> --- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
> @@ -26,6 +26,7 @@ struct virtio_crypto_skcipher_ctx {
>  
>  struct virtio_crypto_sym_request {
>  	struct virtio_crypto_request base;
> +	struct virtio_crypto_op_data_req req_data;
>  
>  	/* Cipher or aead */
>  	uint32_t type;
> @@ -350,14 +351,8 @@ __virtio_crypto_skcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req,
>  	if (!sgs)
>  		return -ENOMEM;
>  
> -	req_data = kzalloc_node(sizeof(*req_data), GFP_KERNEL,
> -				dev_to_node(&vcrypto->vdev->dev));
> -	if (!req_data) {
> -		kfree(sgs);
> -		return -ENOMEM;
> -	}
> -
> -	vc_req->req_data = req_data;
> +	req_data = &vc_sym_req->req_data;
> +	vc_req->req_data = NULL;
>  	vc_sym_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER;
>  	/* Head of operation */
>  	if (vc_sym_req->encrypt) {
> @@ -450,7 +445,6 @@ __virtio_crypto_skcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req,
>  free_iv:
>  	kfree_sensitive(iv);
>  free:
> -	kfree_sensitive(req_data);


So the request is no longer erased with memset on error. Is that not
a problem?

>  	kfree(sgs);
>  	return err;
>  }
> -- 
> 2.39.3