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] [day] [month] [year] [list]

Message-ID: <1d946b10-a1d3-afa0-8f33-029cb5a9828a@loongson.cn>
Date: Fri, 5 Dec 2025 09:23:53 +0800
From: Bibo Mao <maobibo@...ngson.cn>
To: "Michael S. Tsirkin" <mst@...hat.com>
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 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@...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?
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
>