| Message ID | 20200717091943.1942-1-yezhenyu2@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] migration: tls: unref creds after used | expand |
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
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
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)); }
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(-)