On Fri, Mar 20, 2020 at 05:31:17PM +0000, Dr. David Alan Gilbert wrote:
> (Rearranging the text a bit)
>
> * Markus Armbruster (arm...@redhat.com) wrote:
>
> > David (cc'ed) should be able to tell us which fix is right.
> >
> > @tls_creds and @tls_hostname look like they could have the same issue.
>
> A certain Markus removed the Null checks in 8cc99dc because 4af245d
> guaranteed they would be None-Null for tls-creds/hostname - so we
> should be OK for those.
>
> But tls-authz came along a lot later in d2f1d29 and doesn't
> seem to have the initialisation, which is now in
> migration_instance_init.
>
> So I *think* the fix for this is to do the modern equivalent of 4af245d
> :
>
> diff --git a/migration/migration.c b/migration/migration.c
> index c1d88ace7f..0bc1b93277 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3686,6 +3686,7 @@ static void migration_instance_init(Object *obj)
>
> params->tls_hostname = g_strdup("");
> params->tls_creds = g_strdup("");
> + params->tls_authz = g_strdup("");
>
> /* Set has_* up only for parameter checks */
> params->has_compress_level = true;
>
> Copying in Dan to check that wouldn't break tls.
It *will* break TLS, because it will cause the TLS code to lookup
an object with the ID of "". NULL must be preserved when calling
the TLS APIs.
The assignment of "" to tls_hostname would also have broken TLS,
so the migration_tls_channel_connect method had to turn it back
into a real NULL.
The use of "" for tls_creds will similarly cause it to try and
lookup an object with ID of "", and fail. That one's harmless
though, because it would also fail if it were NULL.
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 :|
