From f92d164c1349252ea4c3a47e5af32a674c9b07f6 Mon Sep 17 00:00:00 2001 From: Weilun Zhu Date: Wed, 20 Jun 2018 16:45:27 +0800 Subject: [PATCH] qemu: fix msg could be a wild pointer in qemuMonitorIOProcess() 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 Reviewed-by: Jiri Denemark --- src/qemu/qemu_monitor.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d6771c1d52..6ed475ede0 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -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; } -- GitLab