Message ID | 20180925092245.29565-1-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | migration: fix QEMUFile leak | expand |
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote: > Spotted by ASAN while running: > > $ tests/migration-test -p /x86_64/migration/postcopy/recovery > > ================================================================= > ==18034==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 33864 byte(s) in 1 object(s) allocated from: > #0 0x7f3da7f31e50 in calloc (/lib64/libasan.so.5+0xeee50) > #1 0x7f3da644441d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d) > #2 0x55af9db15440 in qemu_fopen_channel_input /home/elmarco/src/qemu/migration/qemu-file-channel.c:183 > #3 0x55af9db15413 in channel_get_output_return_path /home/elmarco/src/qemu/migration/qemu-file-channel.c:159 > #4 0x55af9db0d4ac in qemu_file_get_return_path /home/elmarco/src/qemu/migration/qemu-file.c:78 > #5 0x55af9dad5e4f in open_return_path_on_source /home/elmarco/src/qemu/migration/migration.c:2295 > #6 0x55af9dadb3bf in migrate_fd_connect /home/elmarco/src/qemu/migration/migration.c:3111 > #7 0x55af9dae1bf3 in migration_channel_connect /home/elmarco/src/qemu/migration/channel.c:91 > #8 0x55af9daddeca in socket_outgoing_migration /home/elmarco/src/qemu/migration/socket.c:108 > #9 0x55af9e13d3db in qio_task_complete /home/elmarco/src/qemu/io/task.c:158 > #10 0x55af9e13ca03 in qio_task_thread_result /home/elmarco/src/qemu/io/task.c:89 > #11 0x7f3da643b1ca in g_idle_dispatch gmain.c:5535 > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > migration/migration.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 05d0a7296a..d214ea12c8 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2268,7 +2268,10 @@ out: > */ > if (postcopy_pause_return_path_thread(ms)) { > /* Reload rp, reset the rest */ > - rp = ms->rp_state.from_dst_file; > + if (rp != ms->rp_state.from_dst_file) { > + qemu_fclose(rp); > + rp = ms->rp_state.from_dst_file; > + } > ms->rp_state.error = false; > goto retry; > } Yep, I think that's right (cc'ing in Peter); Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > -- > 2.19.0 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Tue, Sep 25, 2018 at 10:37:38AM +0100, Dr. David Alan Gilbert wrote: > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote: > > Spotted by ASAN while running: > > > > $ tests/migration-test -p /x86_64/migration/postcopy/recovery > > > > ================================================================= > > ==18034==ERROR: LeakSanitizer: detected memory leaks > > > > Direct leak of 33864 byte(s) in 1 object(s) allocated from: > > #0 0x7f3da7f31e50 in calloc (/lib64/libasan.so.5+0xeee50) > > #1 0x7f3da644441d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d) > > #2 0x55af9db15440 in qemu_fopen_channel_input /home/elmarco/src/qemu/migration/qemu-file-channel.c:183 > > #3 0x55af9db15413 in channel_get_output_return_path /home/elmarco/src/qemu/migration/qemu-file-channel.c:159 > > #4 0x55af9db0d4ac in qemu_file_get_return_path /home/elmarco/src/qemu/migration/qemu-file.c:78 > > #5 0x55af9dad5e4f in open_return_path_on_source /home/elmarco/src/qemu/migration/migration.c:2295 > > #6 0x55af9dadb3bf in migrate_fd_connect /home/elmarco/src/qemu/migration/migration.c:3111 > > #7 0x55af9dae1bf3 in migration_channel_connect /home/elmarco/src/qemu/migration/channel.c:91 > > #8 0x55af9daddeca in socket_outgoing_migration /home/elmarco/src/qemu/migration/socket.c:108 > > #9 0x55af9e13d3db in qio_task_complete /home/elmarco/src/qemu/io/task.c:158 > > #10 0x55af9e13ca03 in qio_task_thread_result /home/elmarco/src/qemu/io/task.c:89 > > #11 0x7f3da643b1ca in g_idle_dispatch gmain.c:5535 > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > migration/migration.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/migration/migration.c b/migration/migration.c > > index 05d0a7296a..d214ea12c8 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -2268,7 +2268,10 @@ out: > > */ > > if (postcopy_pause_return_path_thread(ms)) { > > /* Reload rp, reset the rest */ > > - rp = ms->rp_state.from_dst_file; > > + if (rp != ms->rp_state.from_dst_file) { > > + qemu_fclose(rp); > > + rp = ms->rp_state.from_dst_file; > > + } > > ms->rp_state.error = false; > > goto retry; > > } > > Yep, I think that's right (cc'ing in Peter); > > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Thanks for the report and the fix. I think the "if" will always be true, so imho we can even drop that check. Anyway: Reviewed-by: Peter Xu <peterx@redhat.com> Regards,
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote: > Spotted by ASAN while running: > > $ tests/migration-test -p /x86_64/migration/postcopy/recovery > > ================================================================= > ==18034==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 33864 byte(s) in 1 object(s) allocated from: > #0 0x7f3da7f31e50 in calloc (/lib64/libasan.so.5+0xeee50) > #1 0x7f3da644441d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d) > #2 0x55af9db15440 in qemu_fopen_channel_input /home/elmarco/src/qemu/migration/qemu-file-channel.c:183 > #3 0x55af9db15413 in channel_get_output_return_path /home/elmarco/src/qemu/migration/qemu-file-channel.c:159 > #4 0x55af9db0d4ac in qemu_file_get_return_path /home/elmarco/src/qemu/migration/qemu-file.c:78 > #5 0x55af9dad5e4f in open_return_path_on_source /home/elmarco/src/qemu/migration/migration.c:2295 > #6 0x55af9dadb3bf in migrate_fd_connect /home/elmarco/src/qemu/migration/migration.c:3111 > #7 0x55af9dae1bf3 in migration_channel_connect /home/elmarco/src/qemu/migration/channel.c:91 > #8 0x55af9daddeca in socket_outgoing_migration /home/elmarco/src/qemu/migration/socket.c:108 > #9 0x55af9e13d3db in qio_task_complete /home/elmarco/src/qemu/io/task.c:158 > #10 0x55af9e13ca03 in qio_task_thread_result /home/elmarco/src/qemu/io/task.c:89 > #11 0x7f3da643b1ca in g_idle_dispatch gmain.c:5535 > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Queued > --- > migration/migration.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 05d0a7296a..d214ea12c8 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2268,7 +2268,10 @@ out: > */ > if (postcopy_pause_return_path_thread(ms)) { > /* Reload rp, reset the rest */ > - rp = ms->rp_state.from_dst_file; > + if (rp != ms->rp_state.from_dst_file) { > + qemu_fclose(rp); > + rp = ms->rp_state.from_dst_file; > + } > ms->rp_state.error = false; > goto retry; > } > -- > 2.19.0 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
================================================================= ==18034==ERROR: LeakSanitizer: detected memory leaks Direct leak of 33864 byte(s) in 1 object(s) allocated from: #0 0x7f3da7f31e50 in calloc (/lib64/libasan.so.5+0xeee50) #1 0x7f3da644441d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d) #2 0x55af9db15440 in qemu_fopen_channel_input /home/elmarco/src/qemu/migration/qemu-file-channel.c:183 #3 0x55af9db15413 in channel_get_output_return_path /home/elmarco/src/qemu/migration/qemu-file-channel.c:159 #4 0x55af9db0d4ac in qemu_file_get_return_path /home/elmarco/src/qemu/migration/qemu-file.c:78 #5 0x55af9dad5e4f in open_return_path_on_source /home/elmarco/src/qemu/migration/migration.c:2295 #6 0x55af9dadb3bf in migrate_fd_connect /home/elmarco/src/qemu/migration/migration.c:3111 #7 0x55af9dae1bf3 in migration_channel_connect /home/elmarco/src/qemu/migration/channel.c:91 #8 0x55af9daddeca in socket_outgoing_migration /home/elmarco/src/qemu/migration/socket.c:108 #9 0x55af9e13d3db in qio_task_complete /home/elmarco/src/qemu/io/task.c:158 #10 0x55af9e13ca03 in qio_task_thread_result /home/elmarco/src/qemu/io/task.c:89 #11 0x7f3da643b1ca in g_idle_dispatch gmain.c:5535 Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- migration/migration.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index 05d0a7296a..d214ea12c8 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2268,7 +2268,10 @@ out: */ if (postcopy_pause_return_path_thread(ms)) { /* Reload rp, reset the rest */ - rp = ms->rp_state.from_dst_file; + if (rp != ms->rp_state.from_dst_file) { + qemu_fclose(rp); + rp = ms->rp_state.from_dst_file; + } ms->rp_state.error = false; goto retry; }