[libvirt] [PATCH] qemu: bulk stats: typo in monitor handling
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. Signed-off-by: Francesco Romani <fromani@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; -- 1.9.3
On Thu, Dec 11, 2014 at 08:44:09AM +0100, 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.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
ACK && Pushed. This needs to be back-ported as a part of CVE that I just pushed the fix for yesterday. Thanks, Martin
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; -- 1.9.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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@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
On 12/11/2014 02:57 AM, Peter Krempa wrote:
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:
Fortunately, as there has been no release in between the initial CVE fix and this followup patch, there is no need for an additional CVE. It merely means that the CVE fix is a two-patch effort. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Eric Blake -
Francesco Romani -
Martin Kletzander -
Peter Krempa