[libvirt] [PATCH] qemu: fix libvirtd crash in migration after vm shutdown

[Jiri Denemark]

On Thu, Sep 22, 2016 at 11:56:38 +0200, Jiri Denemark wrote:
> On Tue, Aug 02, 2016 at 02:20:51 +0000, weifuqiang wrote:
> > [PATCH] qemu: fix libvirtd crash in migration after vm shutdown
> > 
> > 
> > 
> > 
> > 
> > If we shutdown a guest, then migrate it without the arg XML, libvirtd will get crashed.
> > 
> > 
> > 
> > The reason is that:
> > 
> > 1 during shutdown callback, qemuProcessStop() , it points vm->def  to vm->newDef
> > 
> > 2 during migration, it frees persistentDef, which points to vm->newDef when the arg XML is NULL.
> > 
> >    However, because vm->newDef is now vm->def, what we IN FACT freed is vm->def.
> > 
> > 3 it will refer to vm->def after step2, thus invalid read/write causes libvirtd crash
> > 
> > 
> > 
> > We needn't to free persistentDef if persist_xml is NULL, because no extra def was alloced if persistent_xml is NULL.
> > 
> > 
> > ---
> > src/qemu/qemu_migration.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 6a683f7..3636c93 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -4915,7 +4915,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
> >          VIR_WARN("Unable to encode migration cookie");
> >      }
> > -    if (persistDef != vm->newDef)
> > +    if (persist_xml && persistDef)
> >          virDomainDefFree(persistDef);
> >      qemuMigrationCookieFree(mig);
> 
> If persist_xml != NULL then persistDef is it's parsed version (or NULL)
> and we need to free it. Otherwise it's just a copy of another pointer.
> In other words, your patch is correct (although checking for persist_xml
> is enough).
> 
> An alternative solution would be to make sure we can always free
> persistDef by making a copy of vm->newDef rather than copying just the
> pointer and free it unconditionally.
> 
> Anyway, your patch is good enough. ACK.
> 
> I'll push it later.

Oh, the patch is corrupted and thus it doesn't apply. And it's hard to
see what your real name is. Is it Wei Fuqiang or something else? We need
proper author's name for each commit.

Please, when sending a new version of this patch, don't copy&paste the
patch into your mail client. Use git send-email whenever possible, or
attach the patch generated by git format-patch to the email.

Jirka