[v1] multifd/tls: fix memoryleak of the QIOChannelSocket object when canceling migration

[RESEND][PATCH] multifd/tls: fix memoryleak of the QIOChannelSocket object when canceling migration

Chuan Zheng posted 1 patch 5 years ago

Diff against v1
Download mbox

Test checkpatch passed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1605078858-114607-1-git-send-email-zhengchuan@huawei.com

Maintainers: Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>

migration/multifd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

Expand all Fold all

[RESEND][PATCH] multifd/tls: fix memoryleak of the QIOChannelSocket object when canceling migration

Posted by Chuan Zheng 5 years ago

When creating new tls client, the tioc->master will be referenced, we need dereferenced
it after tls handshake.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 68b171f..df76a8e 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -728,7 +728,8 @@ static void multifd_tls_outgoing_handshake(QIOTask *task,
                                            gpointer opaque)
 {
     MultiFDSendParams *p = opaque;
-    QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
+    QIOChannelTLS *tioc = QIO_CHANNEL_TLS(qio_task_get_source(task));
+    QIOChannel *ioc = QIO_CHANNEL(tioc);
     Error *err = NULL;
 
     if (qio_task_propagate_error(task, &err)) {
@@ -737,6 +738,7 @@ static void multifd_tls_outgoing_handshake(QIOTask *task,
         trace_multifd_tls_outgoing_handshake_complete(ioc);
     }
     multifd_channel_connect(p, ioc, err);
+    object_unref(OBJECT(tioc->master));
 }
 
 static void multifd_tls_channel_connect(MultiFDSendParams *p,
-- 
1.8.3.1

Re: [RESEND][PATCH] multifd/tls: fix memoryleak of the QIOChannelSocket object when canceling migration

Posted by Daniel P. Berrangé 5 years ago

On Wed, Nov 11, 2020 at 03:14:18PM +0800, Chuan Zheng wrote:
> When creating new tls client, the tioc->master will be referenced, we need dereferenced
> it after tls handshake.
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/multifd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 68b171f..df76a8e 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -728,7 +728,8 @@ static void multifd_tls_outgoing_handshake(QIOTask *task,
>                                             gpointer opaque)
>  {
>      MultiFDSendParams *p = opaque;
> -    QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
> +    QIOChannelTLS *tioc = QIO_CHANNEL_TLS(qio_task_get_source(task));
> +    QIOChannel *ioc = QIO_CHANNEL(tioc);
>      Error *err = NULL;
>  
>      if (qio_task_propagate_error(task, &err)) {
> @@ -737,6 +738,7 @@ static void multifd_tls_outgoing_handshake(QIOTask *task,
>          trace_multifd_tls_outgoing_handshake_complete(ioc);
>      }
>      multifd_channel_connect(p, ioc, err);
> +    object_unref(OBJECT(tioc->master));
>  }

I think this should be done earlier, at the time where we wrap the channel.
eg in multifd_tls_channel_connect(), immediately after the call to
migration_tls_client_create, as that's where the double reference starts
from.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|