[v2] virtio-crypto: fix NULL pointer dereference in virtio_crypto_free_request

[PATCH v2] virtio-crypto: fix NULL pointer dereference in virtio_crypto_free_request

Posted by Mauro Matteo Cascella 11 months, 3 weeks ago

Ensure op_info is not NULL in case of QCRYPTODEV_BACKEND_ALG_SYM algtype.

Fixes: 0e660a6f90a ("crypto: Introduce RSA algorithm")
Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
Reported-by: Yiming Tao <taoym@zju.edu.cn>
---
v2:
- updated 'Fixes:' tag

 hw/virtio/virtio-crypto.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 2fe804510f..c729a1f79e 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -476,15 +476,17 @@ static void virtio_crypto_free_request(VirtIOCryptoReq *req)
         size_t max_len;
         CryptoDevBackendSymOpInfo *op_info = req->op_info.u.sym_op_info;
 
-        max_len = op_info->iv_len +
-                  op_info->aad_len +
-                  op_info->src_len +
-                  op_info->dst_len +
-                  op_info->digest_result_len;
-
-        /* Zeroize and free request data structure */
-        memset(op_info, 0, sizeof(*op_info) + max_len);
-        g_free(op_info);
+        if (op_info) {
+            max_len = op_info->iv_len +
+                      op_info->aad_len +
+                      op_info->src_len +
+                      op_info->dst_len +
+                      op_info->digest_result_len;
+
+            /* Zeroize and free request data structure */
+            memset(op_info, 0, sizeof(*op_info) + max_len);
+            g_free(op_info);
+        }
     } else if (req->flags == QCRYPTODEV_BACKEND_ALG_ASYM) {
         CryptoDevBackendAsymOpInfo *op_info = req->op_info.u.asym_op_info;
         if (op_info) {
-- 
2.40.1

Re: [PATCH v2] virtio-crypto: fix NULL pointer dereference in virtio_crypto_free_request

Posted by zhenwei pi 11 months, 3 weeks ago

LGTM. Thanks!

Reviewed-by: zhenwei pi<pizhenwei@bytedance.com>

On 5/9/23 15:53, Mauro Matteo Cascella wrote:
> Ensure op_info is not NULL in case of QCRYPTODEV_BACKEND_ALG_SYM algtype.
> 
> Fixes: 0e660a6f90a ("crypto: Introduce RSA algorithm")
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Yiming Tao <taoym@zju.edu.cn>
> ---
> v2:
> - updated 'Fixes:' tag
> 
>   hw/virtio/virtio-crypto.c | 20 +++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> index 2fe804510f..c729a1f79e 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -476,15 +476,17 @@ static void virtio_crypto_free_request(VirtIOCryptoReq *req)
>           size_t max_len;
>           CryptoDevBackendSymOpInfo *op_info = req->op_info.u.sym_op_info;
>   
> -        max_len = op_info->iv_len +
> -                  op_info->aad_len +
> -                  op_info->src_len +
> -                  op_info->dst_len +
> -                  op_info->digest_result_len;
> -
> -        /* Zeroize and free request data structure */
> -        memset(op_info, 0, sizeof(*op_info) + max_len);
> -        g_free(op_info);
> +        if (op_info) {
> +            max_len = op_info->iv_len +
> +                      op_info->aad_len +
> +                      op_info->src_len +
> +                      op_info->dst_len +
> +                      op_info->digest_result_len;
> +
> +            /* Zeroize and free request data structure */
> +            memset(op_info, 0, sizeof(*op_info) + max_len);
> +            g_free(op_info);
> +        }
>       } else if (req->flags == QCRYPTODEV_BACKEND_ALG_ASYM) {
>           CryptoDevBackendAsymOpInfo *op_info = req->op_info.u.asym_op_info;
>           if (op_info) {

-- 
zhenwei pi

RE: [PATCH v2] virtio-crypto: fix NULL pointer dereference in virtio_crypto_free_request

Posted by Gonglei (Arei) via 11 months, 3 weeks ago


> -----Original Message-----
> From: Mauro Matteo Cascella [mailto:mcascell@redhat.com]
> Sent: Tuesday, May 9, 2023 3:53 PM
> To: qemu-devel@nongnu.org
> Cc: mst@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>;
> pizhenwei@bytedance.com; taoym@zju.edu.cn; mcascell@redhat.com
> Subject: [PATCH v2] virtio-crypto: fix NULL pointer dereference in
> virtio_crypto_free_request
> 
> Ensure op_info is not NULL in case of QCRYPTODEV_BACKEND_ALG_SYM
> algtype.
> 
> Fixes: 0e660a6f90a ("crypto: Introduce RSA algorithm")
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Yiming Tao <taoym@zju.edu.cn>
> ---
> v2:
> - updated 'Fixes:' tag
> 
>  hw/virtio/virtio-crypto.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


Regards,
-Gonglei

> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c index
> 2fe804510f..c729a1f79e 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -476,15 +476,17 @@ static void
> virtio_crypto_free_request(VirtIOCryptoReq *req)
>          size_t max_len;
>          CryptoDevBackendSymOpInfo *op_info =
> req->op_info.u.sym_op_info;
> 
> -        max_len = op_info->iv_len +
> -                  op_info->aad_len +
> -                  op_info->src_len +
> -                  op_info->dst_len +
> -                  op_info->digest_result_len;
> -
> -        /* Zeroize and free request data structure */
> -        memset(op_info, 0, sizeof(*op_info) + max_len);
> -        g_free(op_info);
> +        if (op_info) {
> +            max_len = op_info->iv_len +
> +                      op_info->aad_len +
> +                      op_info->src_len +
> +                      op_info->dst_len +
> +                      op_info->digest_result_len;
> +
> +            /* Zeroize and free request data structure */
> +            memset(op_info, 0, sizeof(*op_info) + max_len);
> +            g_free(op_info);
> +        }
>      } else if (req->flags == QCRYPTODEV_BACKEND_ALG_ASYM) {
>          CryptoDevBackendAsymOpInfo *op_info =
> req->op_info.u.asym_op_info;
>          if (op_info) {
> --
> 2.40.1