[PATCH v1] migration: tls: unref creds after used

[PATCH v1] migration: tls: unref creds after used

[Zhenyu Ye]

We add the reference of creds in migration_tls_get_creds(),
but there was no place to unref it.  So the OBJECT(creds) will
never be freed and result in memory leak.

Unref the creds after creating the tls-channel server/client.

Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
---
 migration/tls.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/migration/tls.c b/migration/tls.c
index 5171afc6c4..0740d02976 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -97,7 +97,7 @@ void migration_tls_channel_process_incoming(MigrationState *s,
         s->parameters.tls_authz,
         errp);
     if (!tioc) {
-        return;
+        goto cleanup;
     }
 
     trace_migration_tls_incoming_handshake_start();
@@ -107,6 +107,9 @@ void migration_tls_channel_process_incoming(MigrationState *s,
                               NULL,
                               NULL,
                               NULL);
+
+cleanup:
+    object_unref(OBJECT(creds));
 }
 
 
@@ -146,13 +149,13 @@ void migration_tls_channel_connect(MigrationState *s,
     }
     if (!hostname) {
         error_setg(errp, "No hostname available for TLS");
-        return;
+        goto cleanup;
     }
 
     tioc = qio_channel_tls_new_client(
         ioc, creds, hostname, errp);
     if (!tioc) {
-        return;
+        goto cleanup;
     }
 
     trace_migration_tls_outgoing_handshake_start(hostname);
@@ -162,4 +165,7 @@ void migration_tls_channel_connect(MigrationState *s,
                               s,
                               NULL,
                               NULL);
+
+cleanup:
+    object_unref(OBJECT(creds));
 }
-- 
2.19.1

Re: [PATCH v1] migration: tls: unref creds after used

[Daniel P. Berrangé]

On Fri, Jul 17, 2020 at 05:19:43PM +0800, Zhenyu Ye wrote:
> We add the reference of creds in migration_tls_get_creds(),
> but there was no place to unref it.  So the OBJECT(creds) will
> never be freed and result in memory leak.
> 
> Unref the creds after creating the tls-channel server/client.
> 
> Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
> ---
>  migration/tls.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/tls.c b/migration/tls.c
> index 5171afc6c4..0740d02976 100644
> --- a/migration/tls.c
> +++ b/migration/tls.c
> @@ -97,7 +97,7 @@ void migration_tls_channel_process_incoming(MigrationState *s,
>          s->parameters.tls_authz,
>          errp);
>      if (!tioc) {
> -        return;
> +        goto cleanup;
>      }
>  
>      trace_migration_tls_incoming_handshake_start();
> @@ -107,6 +107,9 @@ void migration_tls_channel_process_incoming(MigrationState *s,
>                                NULL,
>                                NULL,
>                                NULL);
> +
> +cleanup:
> +    object_unref(OBJECT(creds));
>  }
>  
>  
> @@ -146,13 +149,13 @@ void migration_tls_channel_connect(MigrationState *s,
>      }
>      if (!hostname) {
>          error_setg(errp, "No hostname available for TLS");
> -        return;
> +        goto cleanup;
>      }
>  
>      tioc = qio_channel_tls_new_client(
>          ioc, creds, hostname, errp);
>      if (!tioc) {
> -        return;
> +        goto cleanup;
>      }
>  
>      trace_migration_tls_outgoing_handshake_start(hostname);
> @@ -162,4 +165,7 @@ void migration_tls_channel_connect(MigrationState *s,
>                                s,
>                                NULL,
>                                NULL);
> +
> +cleanup:
> +    object_unref(OBJECT(creds));
>  }

Simpler to just change  migration_tls_get_creds() to remove the
object_ref() call, since we're fine to use the borrowed reference
in these methods.


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 :|

Re: [PATCH v1] migration: tls: unref creds after used

[Zhenyu Ye]

On 2020/7/21 19:54, Daniel P. Berrangé wrote:
> On Fri, Jul 17, 2020 at 05:19:43PM +0800, Zhenyu Ye wrote:
>> We add the reference of creds in migration_tls_get_creds(),
>> but there was no place to unref it.  So the OBJECT(creds) will
>> never be freed and result in memory leak.
>>
>> Unref the creds after creating the tls-channel server/client.
>> ...
>>
>> +
>> +cleanup:
>> +    object_unref(OBJECT(creds));
>>  }
> 
> Simpler to just change  migration_tls_get_creds() to remove the
> object_ref() call, since we're fine to use the borrowed reference
> in these methods.
> 

Thanks for your review.  I have sent a new series since the current
title "unref creds after used" is no longer appropriate:

https://lore.kernel.org/qemu-devel/20200722033228.71-1-yezhenyu2@huawei.com/

Thanks,
Zhenyu