Re: [PATCH v2 6/9] crypto: virtio: Add req_data with structure virtio_crypto_sym_request

["Michael S. Tsirkin" <mst@redhat.com>]

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@loongson.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