[PATCH] cryptodev: Handle unexpected request to avoid crash

zhenwei pi posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230427080509.172477-1-pizhenwei@bytedance.com
Maintainers: "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>
backends/cryptodev.c | 10 ++++++++++
1 file changed, 10 insertions(+)
[PATCH] cryptodev: Handle unexpected request to avoid crash
Posted by zhenwei pi 1 year ago
Generally guest side should discover which services the device is
able to offer, then do requests on device.

However it's also possible to break this rule in a guest. Handle
unexpected request here to avoid NULL pointer dereference.

Fixes: e7a775fd ('cryptodev: Account statistics')
Cc: Gonglei <arei.gonglei@huawei.com>
Cc: Mauro Matteo Cascella <mcascell@redhat.com>
Cc: Xiao Lei <nop.leixiao@gmail.com>
Cc: Yongkang Jia <kangel@zju.edu.cn>
Reported-by: Yiming Tao <taoym@zju.edu.cn>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 backends/cryptodev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 94ca393cee..d3fe92d8c0 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -191,6 +191,11 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
     if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
         CryptoDevBackendAsymOpInfo *asym_op_info = op_info->u.asym_op_info;
         len = asym_op_info->src_len;
+
+        if (unlikely(!backend->asym_stat)) {
+            error_report("cryptodev: Unexpected asym operation");
+            return -VIRTIO_CRYPTO_NOTSUPP;
+        }
         switch (op_info->op_code) {
         case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
             CryptodevAsymStatIncEncrypt(backend, len);
@@ -210,6 +215,11 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
     } else if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
         CryptoDevBackendSymOpInfo *sym_op_info = op_info->u.sym_op_info;
         len = sym_op_info->src_len;
+
+        if (unlikely(!backend->sym_stat)) {
+            error_report("cryptodev: Unexpected sym operation");
+            return -VIRTIO_CRYPTO_NOTSUPP;
+        }
         switch (op_info->op_code) {
         case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
             CryptodevSymStatIncEncrypt(backend, len);
-- 
2.34.1
Re: [PATCH] cryptodev: Handle unexpected request to avoid crash
Posted by Lei He 11 months, 1 week ago
> On Apr 27, 2023, at 16:05, zhenwei pi <pizhenwei@bytedance.com> wrote:
> 
> Generally guest side should discover which services the device is
> able to offer, then do requests on device.
> 
> However it's also possible to break this rule in a guest. Handle
> unexpected request here to avoid NULL pointer dereference.
> 
> Fixes: e7a775fd ('cryptodev: Account statistics')
> Cc: Gonglei <arei.gonglei@huawei.com>
> Cc: Mauro Matteo Cascella <mcascell@redhat.com>
> Cc: Xiao Lei <nop.leixiao@gmail.com>
> Cc: Yongkang Jia <kangel@zju.edu.cn>
> Reported-by: Yiming Tao <taoym@zju.edu.cn>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> backends/cryptodev.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index 94ca393cee..d3fe92d8c0 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -191,6 +191,11 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
>     if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
>         CryptoDevBackendAsymOpInfo *asym_op_info = op_info->u.asym_op_info;
>         len = asym_op_info->src_len;
> +
> +        if (unlikely(!backend->asym_stat)) {
> +            error_report("cryptodev: Unexpected asym operation");
> +            return -VIRTIO_CRYPTO_NOTSUPP;
> +        }
>         switch (op_info->op_code) {
>         case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
>             CryptodevAsymStatIncEncrypt(backend, len);
> @@ -210,6 +215,11 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
>     } else if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
>         CryptoDevBackendSymOpInfo *sym_op_info = op_info->u.sym_op_info;
>         len = sym_op_info->src_len;
> +
> +        if (unlikely(!backend->sym_stat)) {
> +            error_report("cryptodev: Unexpected sym operation");
> +            return -VIRTIO_CRYPTO_NOTSUPP;
> +        }
>         switch (op_info->op_code) {
>         case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
>             CryptodevSymStatIncEncrypt(backend, len);
> -- 
> 2.34.1
> 

Reviewed-by: Lei He <helei.sig11@bytedance.com>


Best regards,
Lei He
--
helei.sig11@bytedance.com
Re: [PATCH] cryptodev: Handle unexpected request to avoid crash
Posted by zhenwei pi 10 months, 1 week ago
Hi Michael

Could you please apply this patch?

On 5/26/23 11:38, Lei He wrote:
> 
>> On Apr 27, 2023, at 16:05, zhenwei pi <pizhenwei@bytedance.com> wrote:
>>
>> Generally guest side should discover which services the device is
>> able to offer, then do requests on device.
>>
>> However it's also possible to break this rule in a guest. Handle
>> unexpected request here to avoid NULL pointer dereference.
>>
>> Fixes: e7a775fd ('cryptodev: Account statistics')
>> Cc: Gonglei <arei.gonglei@huawei.com>
>> Cc: Mauro Matteo Cascella <mcascell@redhat.com>
>> Cc: Xiao Lei <nop.leixiao@gmail.com>
>> Cc: Yongkang Jia <kangel@zju.edu.cn>
>> Reported-by: Yiming Tao <taoym@zju.edu.cn>
>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>> ---
>> backends/cryptodev.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
>> index 94ca393cee..d3fe92d8c0 100644
>> --- a/backends/cryptodev.c
>> +++ b/backends/cryptodev.c
>> @@ -191,6 +191,11 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
>>      if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
>>          CryptoDevBackendAsymOpInfo *asym_op_info = op_info->u.asym_op_info;
>>          len = asym_op_info->src_len;
>> +
>> +        if (unlikely(!backend->asym_stat)) {
>> +            error_report("cryptodev: Unexpected asym operation");
>> +            return -VIRTIO_CRYPTO_NOTSUPP;
>> +        }
>>          switch (op_info->op_code) {
>>          case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
>>              CryptodevAsymStatIncEncrypt(backend, len);
>> @@ -210,6 +215,11 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
>>      } else if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
>>          CryptoDevBackendSymOpInfo *sym_op_info = op_info->u.sym_op_info;
>>          len = sym_op_info->src_len;
>> +
>> +        if (unlikely(!backend->sym_stat)) {
>> +            error_report("cryptodev: Unexpected sym operation");
>> +            return -VIRTIO_CRYPTO_NOTSUPP;
>> +        }
>>          switch (op_info->op_code) {
>>          case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
>>              CryptodevSymStatIncEncrypt(backend, len);
>> -- 
>> 2.34.1
>>
> 
> Reviewed-by: Lei He <helei.sig11@bytedance.com>
> 
> 
> Best regards,
> Lei He
> --
> helei.sig11@bytedance.com
> 
> 
> 

-- 
zhenwei pi