On 12/11/14 08:44, Francesco Romani wrote:
A typo in qemuConnectGetAllDomainStats makes the code
mark the monitor as available when qemuDomainObjBeginJob
fails, instead of when it succeeds, as the correct flow
requires.
This patch fixes the check and updates the code documentation
accordingly.
Wow, nice catch. The logic inversion was introduced by the CVE fix:
commit 57023c0a3af4af1c547189c1f6712ed5edeb0c0b
Author: Martin Kletzander <mkletzan(a)redhat.com>
Date: Thu Nov 27 15:47:52 2014 +0100
CVE-2014-8131: Fix possible deadlock and segfault in
qemuConnectGetAllDomainStats()
I'm going to tweak the message appropriately (as it's definitely not a
typo :) )
Also I'm CCing the libvirt-security list, as this patch needs to be
backported along with the CVE to the appropriate branches.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
src/qemu/qemu_driver.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 830fca7..129e10c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18745,9 +18745,12 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
}
if (HAVE_JOB(privflags) &&
- qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0)
- /* As it was never requested. Gather as much as possible anyway. */
+ qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) == 0)
domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+ /*
+ * else: as it was never requested.
+ * Gather as much as possible anyway.
+ */
if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
goto endjob;
ACK,
I'll push this in a while.
Peter