[PATCH] hw/virtio/virtio-crypto: Fix op_code assignment in virtio_crypto_create_asym_session

[PATCH] hw/virtio/virtio-crypto: Fix op_code assignment in virtio_crypto_create_asym_session

[Zheyu Ma]

The assignment of the op_code in the virtio_crypto_create_asym_session
function was moved before its usage to ensure it is correctly set.
Previously, if the function failed during the key_len check, the op_code
did not have a proper value, causing virtio_crypto_free_create_session_req
to not free the memory correctly, leading to a memory leak.

By setting the op_code before performing any checks, we ensure that
virtio_crypto_free_create_session_req has the correct context to
perform cleanup operations properly, thus preventing memory leaks.

ASAN log:
==3055068==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 512 byte(s) in 1 object(s) allocated from:
    #0 0x5586a75e6ddd in malloc llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
    #1 0x7fb6b63b6738 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738)
    #2 0x5586a864bbde in virtio_crypto_handle_ctrl hw/virtio/virtio-crypto.c:407:19
    #3 0x5586a94fc84c in virtio_queue_notify_vq hw/virtio/virtio.c:2277:9
    #4 0x5586a94fc0a2 in virtio_queue_host_notifier_read hw/virtio/virtio.c:3641:9

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 hw/virtio/virtio-crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index bbe8aa4b99..5034768bff 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -205,6 +205,7 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
     int queue_index;
     uint32_t algo, keytype, keylen;
 
+    sreq->info.op_code = opcode;
     algo = ldl_le_p(&sess_req->para.algo);
     keytype = ldl_le_p(&sess_req->para.keytype);
     keylen = ldl_le_p(&sess_req->para.keylen);
@@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
         iov_discard_front(&iov, &out_num, keylen);
     }
 
-    sreq->info.op_code = opcode;
     asym_info = &sreq->info.u.asym_sess_info;
     asym_info->algo = algo;
     asym_info->keytype = keytype;
-- 
2.34.1

Re: [PATCH] hw/virtio/virtio-crypto: Fix op_code assignment in virtio_crypto_create_asym_session

[Michael S. Tsirkin]

On Tue, Jul 02, 2024 at 11:02:27PM +0200, Zheyu Ma wrote:
> The assignment of the op_code in the virtio_crypto_create_asym_session
> function was moved before its usage to ensure it is correctly set.
> Previously, if the function failed during the key_len check, the op_code

when you say Previously, you mean "currently"?

> did not have a proper value, causing virtio_crypto_free_create_session_req

and here did not -> does not.

> to not free the memory correctly, leading to a memory leak.
> 
> By setting the op_code before performing any checks, we ensure that
> virtio_crypto_free_create_session_req has the correct context to
> perform cleanup operations properly, thus preventing memory leaks.
> 
> ASAN log:
> ==3055068==ERROR: LeakSanitizer: detected memory leaks
> Direct leak of 512 byte(s) in 1 object(s) allocated from:
>     #0 0x5586a75e6ddd in malloc llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
>     #1 0x7fb6b63b6738 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738)
>     #2 0x5586a864bbde in virtio_crypto_handle_ctrl hw/virtio/virtio-crypto.c:407:19
>     #3 0x5586a94fc84c in virtio_queue_notify_vq hw/virtio/virtio.c:2277:9
>     #4 0x5586a94fc0a2 in virtio_queue_host_notifier_read hw/virtio/virtio.c:3641:9
> 
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
> ---
>  hw/virtio/virtio-crypto.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> index bbe8aa4b99..5034768bff 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -205,6 +205,7 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
>      int queue_index;
>      uint32_t algo, keytype, keylen;
>  
> +    sreq->info.op_code = opcode;
>      algo = ldl_le_p(&sess_req->para.algo);
>      keytype = ldl_le_p(&sess_req->para.keytype);
>      keylen = ldl_le_p(&sess_req->para.keylen);
> @@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
>          iov_discard_front(&iov, &out_num, keylen);
>      }
>  
> -    sreq->info.op_code = opcode;
>      asym_info = &sreq->info.u.asym_sess_info;
>      asym_info->algo = algo;
>      asym_info->keytype = keytype;
> -- 
> 2.34.1

[PATCH] hw/virtio/virtio-crypto: Fix op_code assignment in virtio_crypto_create_asym_session

[Zheyu Ma]

The assignment of the op_code in the virtio_crypto_create_asym_session
function was moved before its usage to ensure it is correctly set.
Previously, if the function failed during the key_len check, the op_code
did not have a proper value, causing virtio_crypto_free_create_session_req
to not free the memory correctly, leading to a memory leak.

By setting the op_code before performing any checks, we ensure that
virtio_crypto_free_create_session_req has the correct context to
perform cleanup operations properly, thus preventing memory leaks.

ASAN log:
==3055068==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 512 byte(s) in 1 object(s) allocated from:
    #0 0x5586a75e6ddd in malloc llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
    #1 0x7fb6b63b6738 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738)
    #2 0x5586a864bbde in virtio_crypto_handle_ctrl hw/virtio/virtio-crypto.c:407:19
    #3 0x5586a94fc84c in virtio_queue_notify_vq hw/virtio/virtio.c:2277:9
    #4 0x5586a94fc0a2 in virtio_queue_host_notifier_read hw/virtio/virtio.c:3641:9

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 hw/virtio/virtio-crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index bbe8aa4b99..5034768bff 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -205,6 +205,7 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
     int queue_index;
     uint32_t algo, keytype, keylen;
 
+    sreq->info.op_code = opcode;
     algo = ldl_le_p(&sess_req->para.algo);
     keytype = ldl_le_p(&sess_req->para.keytype);
     keylen = ldl_le_p(&sess_req->para.keylen);
@@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
         iov_discard_front(&iov, &out_num, keylen);
     }
 
-    sreq->info.op_code = opcode;
     asym_info = &sreq->info.u.asym_sess_info;
     asym_info->algo = algo;
     asym_info->keytype = keytype;
-- 
2.34.1

Re: [PATCH] hw/virtio/virtio-crypto: Fix op_code assignment in virtio_crypto_create_asym_session

[Michael S. Tsirkin]

On Tue, Jul 02, 2024 at 11:04:43PM +0200, Zheyu Ma wrote:
> The assignment of the op_code in the virtio_crypto_create_asym_session
> function was moved before its usage to ensure it is correctly set.
> Previously, if the function failed during the key_len check, the op_code
> did not have a proper value, causing virtio_crypto_free_create_session_req
> to not free the memory correctly, leading to a memory leak.
> 
> By setting the op_code before performing any checks, we ensure that
> virtio_crypto_free_create_session_req has the correct context to
> perform cleanup operations properly, thus preventing memory leaks.
> 
> ASAN log:
> ==3055068==ERROR: LeakSanitizer: detected memory leaks
> Direct leak of 512 byte(s) in 1 object(s) allocated from:
>     #0 0x5586a75e6ddd in malloc llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
>     #1 0x7fb6b63b6738 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738)
>     #2 0x5586a864bbde in virtio_crypto_handle_ctrl hw/virtio/virtio-crypto.c:407:19
>     #3 0x5586a94fc84c in virtio_queue_notify_vq hw/virtio/virtio.c:2277:9
>     #4 0x5586a94fc0a2 in virtio_queue_host_notifier_read hw/virtio/virtio.c:3641:9
> 
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>


why did you send 2 copies?

> ---
>  hw/virtio/virtio-crypto.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> index bbe8aa4b99..5034768bff 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -205,6 +205,7 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
>      int queue_index;
>      uint32_t algo, keytype, keylen;
>  
> +    sreq->info.op_code = opcode;
>      algo = ldl_le_p(&sess_req->para.algo);
>      keytype = ldl_le_p(&sess_req->para.keytype);
>      keylen = ldl_le_p(&sess_req->para.keylen);
> @@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
>          iov_discard_front(&iov, &out_num, keylen);
>      }
>  
> -    sreq->info.op_code = opcode;
>      asym_info = &sreq->info.u.asym_sess_info;
>      asym_info->algo = algo;
>      asym_info->keytype = keytype;
> -- 
> 2.34.1

Re: [PATCH] hw/virtio/virtio-crypto: Fix op_code assignment in virtio_crypto_create_asym_session

[Zheyu Ma]

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

On Tue, Jul 2, 2024 at 11:05 PM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Tue, Jul 02, 2024 at 11:04:43PM +0200, Zheyu Ma wrote:
> > The assignment of the op_code in the virtio_crypto_create_asym_session
> > function was moved before its usage to ensure it is correctly set.
> > Previously, if the function failed during the key_len check, the op_code
> > did not have a proper value, causing
> virtio_crypto_free_create_session_req
> > to not free the memory correctly, leading to a memory leak.
> >
> > By setting the op_code before performing any checks, we ensure that
> > virtio_crypto_free_create_session_req has the correct context to
> > perform cleanup operations properly, thus preventing memory leaks.
> >
> > ASAN log:
> > ==3055068==ERROR: LeakSanitizer: detected memory leaks
> > Direct leak of 512 byte(s) in 1 object(s) allocated from:
> >     #0 0x5586a75e6ddd in malloc
> llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
> >     #1 0x7fb6b63b6738 in g_malloc
> (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738)
> >     #2 0x5586a864bbde in virtio_crypto_handle_ctrl
> hw/virtio/virtio-crypto.c:407:19
> >     #3 0x5586a94fc84c in virtio_queue_notify_vq hw/virtio/virtio.c:2277:9
> >     #4 0x5586a94fc0a2 in virtio_queue_host_notifier_read
> hw/virtio/virtio.c:3641:9
> >
> > Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
>
>
> why did you send 2 copies?
>
> Sorry for disturbing you; I sent the v2 version wrongly, and now I've sent
a v3 version.

Regards,
Zheyu


> > ---
> >  hw/virtio/virtio-crypto.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> > index bbe8aa4b99..5034768bff 100644
> > --- a/hw/virtio/virtio-crypto.c
> > +++ b/hw/virtio/virtio-crypto.c
> > @@ -205,6 +205,7 @@ virtio_crypto_create_asym_session(VirtIOCrypto
> *vcrypto,
> >      int queue_index;
> >      uint32_t algo, keytype, keylen;
> >
> > +    sreq->info.op_code = opcode;
> >      algo = ldl_le_p(&sess_req->para.algo);
> >      keytype = ldl_le_p(&sess_req->para.keytype);
> >      keylen = ldl_le_p(&sess_req->para.keylen);
> > @@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto
> *vcrypto,
> >          iov_discard_front(&iov, &out_num, keylen);
> >      }
> >
> > -    sreq->info.op_code = opcode;
> >      asym_info = &sreq->info.u.asym_sess_info;
> >      asym_info->algo = algo;
> >      asym_info->keytype = keytype;
> > --
> > 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 3395 bytes --]