[PATCH v2 1/9] crypto: virtio: Add spinlock protection with virtqueue notification

[PATCH v2 1/9] crypto: virtio: Add spinlock protection with virtqueue notification

[Bibo Mao]

When VM boots with one virtio-crypto PCI device and builtin backend,
run openssl benchmark command with multiple processes, such as
  openssl speed -evp aes-128-cbc -engine afalg  -seconds 10 -multi 32

openssl processes will hangup and there is error reported like this:
 virtio_crypto virtio0: dataq.0:id 3 is not a head!

It seems that the data virtqueue need protection when it is handled
for virtio done notification. If the spinlock protection is added
in virtcrypto_done_task(), openssl benchmark with multiple processes
works well.

Fixes: fed93fb62e05 ("crypto: virtio - Handle dataq logic with tasklet")
Cc: stable@vger.kernel.org
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 drivers/crypto/virtio/virtio_crypto_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index 3d241446099c..ccc6b5c1b24b 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -75,15 +75,20 @@ static void virtcrypto_done_task(unsigned long data)
 	struct data_queue *data_vq = (struct data_queue *)data;
 	struct virtqueue *vq = data_vq->vq;
 	struct virtio_crypto_request *vc_req;
+	unsigned long flags;
 	unsigned int len;
 
+	spin_lock_irqsave(&data_vq->lock, flags);
 	do {
 		virtqueue_disable_cb(vq);
 		while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
+			spin_unlock_irqrestore(&data_vq->lock, flags);
 			if (vc_req->alg_cb)
 				vc_req->alg_cb(vc_req, len);
+			spin_lock_irqsave(&data_vq->lock, flags);
 		}
 	} while (!virtqueue_enable_cb(vq));
+	spin_unlock_irqrestore(&data_vq->lock, flags);
 }
 
 static void virtcrypto_dataq_callback(struct virtqueue *vq)
-- 
2.39.3

Re: [PATCH v2 1/9] crypto: virtio: Add spinlock protection with virtqueue notification

[Jason Wang]

On Thu, Dec 4, 2025 at 7:22 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> When VM boots with one virtio-crypto PCI device and builtin backend,
> run openssl benchmark command with multiple processes, such as
>   openssl speed -evp aes-128-cbc -engine afalg  -seconds 10 -multi 32
>
> openssl processes will hangup and there is error reported like this:
>  virtio_crypto virtio0: dataq.0:id 3 is not a head!
>
> It seems that the data virtqueue need protection when it is handled
> for virtio done notification. If the spinlock protection is added
> in virtcrypto_done_task(), openssl benchmark with multiple processes
> works well.
>
> Fixes: fed93fb62e05 ("crypto: virtio - Handle dataq logic with tasklet")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  drivers/crypto/virtio/virtio_crypto_core.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
> index 3d241446099c..ccc6b5c1b24b 100644
> --- a/drivers/crypto/virtio/virtio_crypto_core.c
> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> @@ -75,15 +75,20 @@ static void virtcrypto_done_task(unsigned long data)
>         struct data_queue *data_vq = (struct data_queue *)data;
>         struct virtqueue *vq = data_vq->vq;
>         struct virtio_crypto_request *vc_req;
> +       unsigned long flags;
>         unsigned int len;
>
> +       spin_lock_irqsave(&data_vq->lock, flags);
>         do {
>                 virtqueue_disable_cb(vq);
>                 while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
> +                       spin_unlock_irqrestore(&data_vq->lock, flags);
>                         if (vc_req->alg_cb)
>                                 vc_req->alg_cb(vc_req, len);
> +                       spin_lock_irqsave(&data_vq->lock, flags);
>                 }
>         } while (!virtqueue_enable_cb(vq));
> +       spin_unlock_irqrestore(&data_vq->lock, flags);
>  }

Another thing that needs to care:

There seems to be a redundant virtqueue_kick() in
virtio_crypto_skcipher_crypt_req() which is out of the protection of
the spinlock.

I think we can simply remote that?

Thanks

>
>  static void virtcrypto_dataq_callback(struct virtqueue *vq)
> --
> 2.39.3
>

Re: [PATCH v2 1/9] crypto: virtio: Add spinlock protection with virtqueue notification

[Bibo Mao]

On 2025/12/5 上午9:21, Jason Wang wrote:
> On Thu, Dec 4, 2025 at 7:22 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> When VM boots with one virtio-crypto PCI device and builtin backend,
>> run openssl benchmark command with multiple processes, such as
>>    openssl speed -evp aes-128-cbc -engine afalg  -seconds 10 -multi 32
>>
>> openssl processes will hangup and there is error reported like this:
>>   virtio_crypto virtio0: dataq.0:id 3 is not a head!
>>
>> It seems that the data virtqueue need protection when it is handled
>> for virtio done notification. If the spinlock protection is added
>> in virtcrypto_done_task(), openssl benchmark with multiple processes
>> works well.
>>
>> Fixes: fed93fb62e05 ("crypto: virtio - Handle dataq logic with tasklet")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   drivers/crypto/virtio/virtio_crypto_core.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
>> index 3d241446099c..ccc6b5c1b24b 100644
>> --- a/drivers/crypto/virtio/virtio_crypto_core.c
>> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
>> @@ -75,15 +75,20 @@ static void virtcrypto_done_task(unsigned long data)
>>          struct data_queue *data_vq = (struct data_queue *)data;
>>          struct virtqueue *vq = data_vq->vq;
>>          struct virtio_crypto_request *vc_req;
>> +       unsigned long flags;
>>          unsigned int len;
>>
>> +       spin_lock_irqsave(&data_vq->lock, flags);
>>          do {
>>                  virtqueue_disable_cb(vq);
>>                  while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
>> +                       spin_unlock_irqrestore(&data_vq->lock, flags);
>>                          if (vc_req->alg_cb)
>>                                  vc_req->alg_cb(vc_req, len);
>> +                       spin_lock_irqsave(&data_vq->lock, flags);
>>                  }
>>          } while (!virtqueue_enable_cb(vq));
>> +       spin_unlock_irqrestore(&data_vq->lock, flags);
>>   }
> 
> Another thing that needs to care:
> 
> There seems to be a redundant virtqueue_kick() in
> virtio_crypto_skcipher_crypt_req() which is out of the protection of
> the spinlock.
> 
> I think we can simply remote that?
yes, there is redundant virtqueue_kick() in function 
virtio_crypto_skcipher_crypt_req().

Will remove one in next version.

Regards
Bibo Mao
> 
> Thanks
> 
>>
>>   static void virtcrypto_dataq_callback(struct virtqueue *vq)
>> --
>> 2.39.3
>>

[PATCH v2 2/9] crypto: virtio: Replace package id with numa node id

[Bibo Mao]

With multiple virtio crypto devices supported with different NUMA
nodes, when crypto session is created, it will search virtio crypto
device with the same numa node of current CPU.

Here API topology_physical_package_id() is replaced with cpu_to_node()
since package id is physical concept, and one package id have multiple
memory numa id.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 drivers/crypto/virtio/virtio_crypto_common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
index 19c934af3df6..e559bdadf4f9 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -135,7 +135,7 @@ static inline int virtio_crypto_get_current_node(void)
 	int cpu, node;
 
 	cpu = get_cpu();
-	node = topology_physical_package_id(cpu);
+	node = cpu_to_node(cpu);
 	put_cpu();
 
 	return node;
-- 
2.39.3

[PATCH v2 3/9] crypto: virtio: Add algo pointer in virtio_crypto_skcipher_ctx

[Bibo Mao]

Structure virtio_crypto_skcipher_ctx is allocated together with
crypto_skcipher, and skcipher_alg is set in strucutre crypto_skcipher
when it is created.

Here add virtio_crypto_algo pointer in virtio_crypto_skcipher_ctx and
set it in function virtio_crypto_skcipher_init(). So crypto service
and algo can be acquired from virtio_crypto_algo pointer.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 .../virtio/virtio_crypto_skcipher_algs.c      | 20 ++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index 1b3fb21a2a7d..d42c7a77cdbb 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -15,9 +15,11 @@
 #include "virtio_crypto_common.h"
 
 
+struct virtio_crypto_algo;
 struct virtio_crypto_skcipher_ctx {
 	struct virtio_crypto *vcrypto;
 
+	struct virtio_crypto_algo *alg;
 	struct virtio_crypto_sym_session_info enc_sess_info;
 	struct virtio_crypto_sym_session_info dec_sess_info;
 };
@@ -108,9 +110,7 @@ virtio_crypto_alg_validate_key(int key_len, uint32_t *alg)
 
 static int virtio_crypto_alg_skcipher_init_session(
 		struct virtio_crypto_skcipher_ctx *ctx,
-		uint32_t alg, const uint8_t *key,
-		unsigned int keylen,
-		int encrypt)
+		const uint8_t *key, unsigned int keylen, int encrypt)
 {
 	struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
 	struct virtio_crypto *vcrypto = ctx->vcrypto;
@@ -140,7 +140,7 @@ static int virtio_crypto_alg_skcipher_init_session(
 	/* Pad ctrl header */
 	ctrl = &vc_ctrl_req->ctrl;
 	ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
-	ctrl->header.algo = cpu_to_le32(alg);
+	ctrl->header.algo = cpu_to_le32(ctx->alg->algonum);
 	/* Set the default dataqueue id to 0 */
 	ctrl->header.queue_id = 0;
 
@@ -261,13 +261,11 @@ static int virtio_crypto_alg_skcipher_init_sessions(
 		return -EINVAL;
 
 	/* Create encryption session */
-	ret = virtio_crypto_alg_skcipher_init_session(ctx,
-			alg, key, keylen, 1);
+	ret = virtio_crypto_alg_skcipher_init_session(ctx, key, keylen, 1);
 	if (ret)
 		return ret;
 	/* Create decryption session */
-	ret = virtio_crypto_alg_skcipher_init_session(ctx,
-			alg, key, keylen, 0);
+	ret = virtio_crypto_alg_skcipher_init_session(ctx, key, keylen, 0);
 	if (ret) {
 		virtio_crypto_alg_skcipher_close_session(ctx, 1);
 		return ret;
@@ -293,7 +291,7 @@ static int virtio_crypto_skcipher_setkey(struct crypto_skcipher *tfm,
 		int node = virtio_crypto_get_current_node();
 		struct virtio_crypto *vcrypto =
 				      virtcrypto_get_dev_node(node,
-				      VIRTIO_CRYPTO_SERVICE_CIPHER, alg);
+				      ctx->alg->service, ctx->alg->algonum);
 		if (!vcrypto) {
 			pr_err("virtio_crypto: Could not find a virtio device in the system or unsupported algo\n");
 			return -ENODEV;
@@ -509,7 +507,11 @@ static int virtio_crypto_skcipher_decrypt(struct skcipher_request *req)
 
 static int virtio_crypto_skcipher_init(struct crypto_skcipher *tfm)
 {
+	struct virtio_crypto_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+	struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
+
 	crypto_skcipher_set_reqsize(tfm, sizeof(struct virtio_crypto_sym_request));
+	ctx->alg = container_of(alg, struct virtio_crypto_algo, algo.base);
 
 	return 0;
 }
-- 
2.39.3

[PATCH v2 4/9] crypto: virtio: Use generic API aes_check_keylen()

[Bibo Mao]

With AES algo, generic API aes_check_keylen() is used to check length
of key.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 .../virtio/virtio_crypto_skcipher_algs.c      | 20 ++++++++-----------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index d42c7a77cdbb..314ecda46311 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -94,18 +94,16 @@ static u64 virtio_crypto_alg_sg_nents_length(struct scatterlist *sg)
 }
 
 static int
-virtio_crypto_alg_validate_key(int key_len, uint32_t *alg)
+virtio_crypto_alg_validate_key(int key_len, uint32_t alg)
 {
-	switch (key_len) {
-	case AES_KEYSIZE_128:
-	case AES_KEYSIZE_192:
-	case AES_KEYSIZE_256:
-		*alg = VIRTIO_CRYPTO_CIPHER_AES_CBC;
-		break;
+	switch (alg) {
+	case VIRTIO_CRYPTO_CIPHER_AES_ECB:
+	case VIRTIO_CRYPTO_CIPHER_AES_CBC:
+	case VIRTIO_CRYPTO_CIPHER_AES_CTR:
+		return aes_check_keylen(key_len);
 	default:
 		return -EINVAL;
 	}
-	return 0;
 }
 
 static int virtio_crypto_alg_skcipher_init_session(
@@ -248,7 +246,6 @@ static int virtio_crypto_alg_skcipher_init_sessions(
 		struct virtio_crypto_skcipher_ctx *ctx,
 		const uint8_t *key, unsigned int keylen)
 {
-	uint32_t alg;
 	int ret;
 	struct virtio_crypto *vcrypto = ctx->vcrypto;
 
@@ -257,7 +254,7 @@ static int virtio_crypto_alg_skcipher_init_sessions(
 		return -EINVAL;
 	}
 
-	if (virtio_crypto_alg_validate_key(keylen, &alg))
+	if (virtio_crypto_alg_validate_key(keylen, ctx->alg->algonum))
 		return -EINVAL;
 
 	/* Create encryption session */
@@ -279,10 +276,9 @@ static int virtio_crypto_skcipher_setkey(struct crypto_skcipher *tfm,
 					 unsigned int keylen)
 {
 	struct virtio_crypto_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
-	uint32_t alg;
 	int ret;
 
-	ret = virtio_crypto_alg_validate_key(keylen, &alg);
+	ret = virtio_crypto_alg_validate_key(keylen, ctx->alg->algonum);
 	if (ret)
 		return ret;
 
-- 
2.39.3

[PATCH v2 5/9] crypto: virtio: Remove AES specified marcro AES_BLOCK_SIZE

[Bibo Mao]

Macro AES_BLOCK_SIZE is meaningful only for algo AES, replace it
with generic API crypto_skcipher_blocksize(), so that new algo can
be added in later.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 .../crypto/virtio/virtio_crypto_skcipher_algs.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index 314ecda46311..7b3f21a40d78 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -416,8 +416,8 @@ __virtio_crypto_skcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req,
 	memcpy(iv, req->iv, ivsize);
 	if (!vc_sym_req->encrypt)
 		scatterwalk_map_and_copy(req->iv, req->src,
-					 req->cryptlen - AES_BLOCK_SIZE,
-					 AES_BLOCK_SIZE, 0);
+					 req->cryptlen - ivsize,
+					 ivsize, 0);
 
 	sg_init_one(&iv_sg, iv, ivsize);
 	sgs[num_out++] = &iv_sg;
@@ -459,6 +459,7 @@ static int virtio_crypto_skcipher_encrypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *atfm = crypto_skcipher_reqtfm(req);
 	struct virtio_crypto_skcipher_ctx *ctx = crypto_skcipher_ctx(atfm);
+	unsigned int blocksize = crypto_skcipher_blocksize(atfm);
 	struct virtio_crypto_sym_request *vc_sym_req =
 				skcipher_request_ctx(req);
 	struct virtio_crypto_request *vc_req = &vc_sym_req->base;
@@ -468,7 +469,7 @@ static int virtio_crypto_skcipher_encrypt(struct skcipher_request *req)
 
 	if (!req->cryptlen)
 		return 0;
-	if (req->cryptlen % AES_BLOCK_SIZE)
+	if (req->cryptlen % blocksize)
 		return -EINVAL;
 
 	vc_req->dataq = data_vq;
@@ -482,6 +483,7 @@ static int virtio_crypto_skcipher_decrypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *atfm = crypto_skcipher_reqtfm(req);
 	struct virtio_crypto_skcipher_ctx *ctx = crypto_skcipher_ctx(atfm);
+	unsigned int blocksize = crypto_skcipher_blocksize(atfm);
 	struct virtio_crypto_sym_request *vc_sym_req =
 				skcipher_request_ctx(req);
 	struct virtio_crypto_request *vc_req = &vc_sym_req->base;
@@ -491,7 +493,7 @@ static int virtio_crypto_skcipher_decrypt(struct skcipher_request *req)
 
 	if (!req->cryptlen)
 		return 0;
-	if (req->cryptlen % AES_BLOCK_SIZE)
+	if (req->cryptlen % blocksize)
 		return -EINVAL;
 
 	vc_req->dataq = data_vq;
@@ -549,10 +551,13 @@ static void virtio_crypto_skcipher_finalize_req(
 	struct skcipher_request *req,
 	int err)
 {
+	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+
 	if (vc_sym_req->encrypt)
 		scatterwalk_map_and_copy(req->iv, req->dst,
-					 req->cryptlen - AES_BLOCK_SIZE,
-					 AES_BLOCK_SIZE, 0);
+					 req->cryptlen - ivsize,
+					 ivsize, 0);
 	kfree_sensitive(vc_sym_req->iv);
 	virtcrypto_clear_request(&vc_sym_req->base);
 
-- 
2.39.3

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

[Bibo Mao]

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(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);
 	kfree(sgs);
 	return err;
 }
-- 
2.39.3

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

[Michael S. Tsirkin]

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

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

[Bibo Mao]

On 2025/12/4 下午8:46, Michael S. Tsirkin wrote:
> 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?
Will keep it unchanged in next version.

> 
>>   		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?
I do not know why req_data is sensitive data here, it is only control 
command, key and IV data is not in req_data.

Regards
Bibo Mao
> 
>>   	kfree(sgs);
>>   	return err;
>>   }
>> -- 
>> 2.39.3
>