[libvirt] [PATCH] qemu: monitor: Don't bother extracting vCPU halted state in text monitor

The code causes the 'offset' variable to be overwritten (possibly with NULL if neither of the vCPUs is halted) which causes a crash since the variable is still used after that part. Additionally there's a bug, since strstr() would look up the '(halted)' string in the whole string rather than just the currently processed line the returned data is completely bogus. Rather than switching to single line parsing let's remove the code altogether since it has a commonly used JSON monitor alternative and the data itself is not very useful to report. The code was introduced in commit cc5e695bde Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452106 --- src/qemu/qemu_monitor_text.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 9c9eeea01..66c94fbcd 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -552,12 +552,6 @@ qemuMonitorTextQueryCPUs(qemuMonitorPtr mon, cpu.qemu_id = cpuid; cpu.tid = tid; - /* Extract halted indicator */ - if ((offset = strstr(line, "(halted)")) != NULL) - cpu.halted = true; - else - cpu.halted = false; - if (VIR_APPEND_ELEMENT_COPY(cpus, ncpus, cpu) < 0) { ret = -1; goto cleanup; -- 2.12.2

On Thu, May 18, 2017 at 01:47:03PM +0200, Peter Krempa wrote:
The code causes the 'offset' variable to be overwritten (possibly with NULL if neither of the vCPUs is halted) which causes a crash since the variable is still used after that part.
Additionally there's a bug, since strstr() would look up the '(halted)' string in the whole string rather than just the currently processed line the returned data is completely bogus.
Rather than switching to single line parsing let's remove the code altogether since it has a commonly used JSON monitor alternative and the data itself is not very useful to report.
The code was introduced in commit cc5e695bde
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452106 --- src/qemu/qemu_monitor_text.c | 6 ------ 1 file changed, 6 deletions(-)
ACK When will we able to kill the whole file? Jan

On Thu, May 18, 2017 at 13:53:55 +0200, Ján Tomko wrote:
On Thu, May 18, 2017 at 01:47:03PM +0200, Peter Krempa wrote:
The code causes the 'offset' variable to be overwritten (possibly with NULL if neither of the vCPUs is halted) which causes a crash since the variable is still used after that part.
Additionally there's a bug, since strstr() would look up the '(halted)' string in the whole string rather than just the currently processed line the returned data is completely bogus.
Rather than switching to single line parsing let's remove the code altogether since it has a commonly used JSON monitor alternative and the data itself is not very useful to report.
The code was introduced in commit cc5e695bde
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452106 --- src/qemu/qemu_monitor_text.c | 6 ------ 1 file changed, 6 deletions(-)
ACK
When will we able to kill the whole file?
Once we finaly decide, that centos 6.0 + upstream libvirt is an insane configuration.
participants (2)
-
Ján Tomko
-
Peter Krempa