Re: [PATCH] hw/virtio/virtio-crypto: Fix op_code assignment in virtio_crypto_create_asym_session
On Tue, Jul 2, 2024 at 11:05 PM Michael S. Tsirkin 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 > > > 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(_req->para.algo); > > keytype = ldl_le_p(_req->para.keytype); > > keylen = ldl_le_p(_req->para.keylen); > > @@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto > *vcrypto, > > iov_discard_front(, _num, keylen); > > } > > > > -sreq->info.op_code = opcode; > > asym_info = >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
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 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(_req->para.algo); > keytype = ldl_le_p(_req->para.keytype); > keylen = ldl_le_p(_req->para.keylen); > @@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto, > iov_discard_front(, _num, keylen); > } > > -sreq->info.op_code = opcode; > asym_info = >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
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 > --- > 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(_req->para.algo); > keytype = ldl_le_p(_req->para.keytype); > keylen = ldl_le_p(_req->para.keylen); > @@ -224,7 +225,6 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto, > iov_discard_front(, _num, keylen); > } > > -sreq->info.op_code = opcode; > asym_info = >info.u.asym_sess_info; > asym_info->algo = algo; > asym_info->keytype = keytype; > -- > 2.34.1
