Re: [Qemu-devel] [PATCH] Migration+TLS: Fix crash due to double cleanup

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Apr 30, 2018 at 07:59:43PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> During a TLS connect we see:
>   migration_channel_connect calls
>   migration_tls_channel_connect
>   (calls after TLS setup)
>   migration_channel_connect
> 
> My previous error handling fix made migration_channel_connect
> call migrate_fd_connect in all cases; unfortunately the above
> means it gets called twice and crashes doing double cleanup.
> 
> Fixes: 688a3dcba98
> 
> Reported-by: Peter Krempa <pkrempa@redhat.com>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/channel.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

I've satisfied myself that the double error reporting is not going to
cause any functional harm, so that's less urgent to fix.

> 
> diff --git a/migration/channel.c b/migration/channel.c
> index c5eaf0fa0e..7a32b5aca4 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -71,6 +71,15 @@ void migration_channel_connect(MigrationState *s,
>              !object_dynamic_cast(OBJECT(ioc),
>                                   TYPE_QIO_CHANNEL_TLS)) {
>              migration_tls_channel_connect(s, ioc, hostname, &error);
> +
> +            if (!error) {
> +                /* tls_channel_connect will call back to this
> +                 * function after the TLS handshake,
> +                 * so we mustn't call migrate_fd_connect until then
> +                 */
> +
> +                return;
> +            }
>          } else {
>              QEMUFile *f = qemu_fopen_channel_output(ioc);
>  
> -- 
> 2.17.0
> 

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