]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
qemu: fix msg could be a wild pointer in qemuMonitorIOProcess()
authorWeilun Zhu <zhuweilun@huawei.com>
Wed, 20 Jun 2018 08:45:27 +0000 (16:45 +0800)
committerJiri Denemark <jdenemar@redhat.com>
Fri, 22 Jun 2018 08:40:59 +0000 (10:40 +0200)
As qemuMonitorJSONIOProcess will call qemuMonitorJSONIOProcessEvent
which unlocks the monitor mutex, there is some extreme situation,
eg qemu send message to monitor twice in a short time, where the
local viriable 'msg' of qemuMonitorIOProcess could be a wild point:

1. qemuMonitorSend() assign mon->msg to parameter 'msg', which is alse a
local variable of its caller qemuMonitorJSONCommandWithFd(), cause
eventloop to send message to monitor, then wait condition.
2. qemu send message to monitor for the first time immediately.
3. qemuMonitorIOProcess() is called, then wake up the qemuMonitorSend()
thread, but the qemuMonitorSend() thread stuck for a while as cpu pressure
or some other reasons, which means the qemu monitor is still unlocked.
4. qemu send event message to monitor for the second time,
such as RTC_CHANGE event
5. qemuMonitorIOProcess() is called again, the local viriable 'msg' is
assigned to mon->msg.
6. qemuMonitorIOProcess() call qemuMonitorJSONIOProcess() to deal with
the qemu event.
7. qemuMonitorJSONIOProcess() unlock the qemu monitor in the macro
'QEMU_MONITOR_CALLBACK', then qemuMonitorSend() thread get the mutex
and free the mon->msg, assign mon->msg to NULL.

Signed-off-by: Weilun Zhu <zhuweilun@huawei.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
src/qemu/qemu_monitor.c

index d6771c1d52241f895ab9756327d07a54e0a6e2e5..6ed475ede0113fbc2acf554471ba1a6c1107c922 100644 (file)
@@ -466,7 +466,11 @@ qemuMonitorIOProcess(qemuMonitorPtr mon)
 #if DEBUG_IO
     VIR_DEBUG("Process done %d used %d", (int)mon->bufferOffset, len);
 #endif
-    if (msg && msg->finished)
+
+    /* As the monitor mutex was unlocked in qemuMonitorJSONIOProcess()
+     * while dealing with qemu event, mon->msg could be changed which
+     * means the above 'msg' may be invalid, thus we use 'mon->msg' here */
+    if (mon->msg && mon->msg->finished)
         virCondBroadcast(&mon->notify);
     return len;
 }