Re: [PATCH v4] migration: Plug memory leak with migration URIs

[Peter Xu]

On Thu, Nov 30, 2023 at 07:35:43PM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Wed, Nov 29, 2023 at 08:43:01PM +0000, Het Gala wrote:
> >> migrate_uri_parse() allocates memory to 'channel' if the user
> >> opts for old syntax - uri, which is leaked because there is no
> >> code for freeing 'channel'.
> >> So, free channel to avoid memory leak in case where 'channels'
> >> is empty and uri parsing is required.
> >> 
> >> Fixes: 5994024f ("migration: Implement MigrateChannelList to qmp migration 
> >> flow")
> >> Signed-off-by: Het Gala <het.gala@nutanix.com>
> >> Suggested-by: Markus Armbruster <armbru@redhat.com>
> >
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> >
> >> @@ -533,18 +533,18 @@ static void qemu_start_incoming_migration(const char 
> >> *uri, bool has_channels,
> >>              error_setg(errp, "Channel list has more than one entries");
> >>              return;
> >>          }
> >> -        channel = channels->value;
> >> +        addr = channels->value->addr;
> >>      } else if (uri) {
> >>          /* caller uses the old URI syntax */
> >>          if (!migrate_uri_parse(uri, &channel, errp)) {
> >>              return;
> >>          }
> >> +        addr = channel->addr;
> >>      } else {
> >>          error_setg(errp, "neither 'uri' or 'channels' argument are "
> >>                     "specified in 'migrate-incoming' qmp command ");
> >>          return;
> >>      }
> >> -    addr = channel->addr;
> >
> > Why these "addr" lines need change?  Won't that behave the same as before?
> 
> In the first case, @channel is now null.  If we left the assignment to
> @addr alone, it would crash.  Clearer now?

Is it this one?

    if (uri && has_channels) {
        error_setg(errp, "'uri' and 'channels' arguments are mutually "
                   "exclusive; exactly one of the two should be present in "
                   "'migrate-incoming' qmp command ");
        return;
    }

It returns already?

Thanks,

-- 
Peter Xu