Re: [PATCH] qemuMonitorIO: Don't use @mon after it's unrefed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/15/2013 12:33 PM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1018267
>
> The aim of virObject refing and urefing is to tell where the object is
> to be used and when is no longer needed. Hence any object shouldn't be
> used after it has been unrefed, as we might be the last to hold the
> reference. The better way is to call virObjectUnref() *after* the last
> object usage. In this specific case, the monitor EOF handler was called
> after the qemuMonitorIO called virObjectUnref. Not only that @mon was
> disposed (which is not used in the handler anyway) but the @mon->vm
> which is causing a SIGSEGV:

ACK. (and I do mean "ACK!!" in both senses of the word :-)

>
> 2013-11-15 10:17:54.425+0000: 20110: error : qemuMonitorIO:688 : internal error: early end of file from monitor: possible problem:
> qemu-kvm: -incoming tcp:01.01.01.0:49152: Failed to bind socket: Cannot assign requested address
>
> Program received signal SIGSEGV, Segmentation fault.
> qemuProcessHandleMonitorEOF (mon=<optimized out>, vm=0x7fb728004170) at qemu/qemu_process.c:299
> 299         if (priv->beingDestroyed) {
> (gdb) p *priv
> Cannot access memory at address 0x0
> (gdb) p vm
> $1 = (virDomainObj *) 0x7fb728004170
> (gdb) p *vm
> $2 = {parent = {parent = {magic = 3735928559, refs = 0, klass = 0xdeadbeef}, lock = {lock = {__data = {__lock = 2, __count = 0, __owner = 20110, __nusers = 1, __kind = 0, __spins = 0, __list = {__prev = 0x0,
>             __next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\216N\000\000\001", '\000' <repeats 26 times>, __align = 2}}}, pid = 0, state = {state = 0, reason = 0}, autostart = 0, persistent = 0,
>   updated = 0, def = 0x0, newDef = 0x0, snapshots = 0x0, current_snapshot = 0x0, hasManagedSave = false, privateData = 0x0, privateDataFreeFunc = 0x0, taint = 304}
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  src/qemu/qemu_monitor.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 30315b3..935f140 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -721,33 +721,33 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
>      /* We have to unlock to avoid deadlock against command thread,
>       * but is this safe ?  I think it is, because the callback
>       * will try to acquire the virDomainObjPtr mutex next */
>      if (eof) {
>          qemuMonitorEofNotifyCallback eofNotify = mon->cb->eofNotify;
>          virDomainObjPtr vm = mon->vm;
>  
>          /* Make sure anyone waiting wakes up now */
>          virCondSignal(&mon->notify);
>          virObjectUnlock(mon);
> -        virObjectUnref(mon);
>          VIR_DEBUG("Triggering EOF callback");
>          (eofNotify)(mon, vm, mon->callbackOpaque);
> +        virObjectUnref(mon);
>      } else if (error) {
>          qemuMonitorErrorNotifyCallback errorNotify = mon->cb->errorNotify;
>          virDomainObjPtr vm = mon->vm;
>  
>          /* Make sure anyone waiting wakes up now */
>          virCondSignal(&mon->notify);
>          virObjectUnlock(mon);
> -        virObjectUnref(mon);
>          VIR_DEBUG("Triggering error callback");
>          (errorNotify)(mon, vm, mon->callbackOpaque);
> +        virObjectUnref(mon);
>      } else {
>          virObjectUnlock(mon);
>          virObjectUnref(mon);
>      }
>  }
>  
>  
>  static qemuMonitorPtr
>  qemuMonitorOpenInternal(virDomainObjPtr vm,
>                          int fd,

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]