|
 |
9119d9 |
From 410c5b18640b585a2cd6b56c40b8e8c75355725c Mon Sep 17 00:00:00 2001
|
|
|
9119d9 |
Message-Id: <410c5b18640b585a2cd6b56c40b8e8c75355725c@dist-git>
|
|
|
9119d9 |
From: Martin Kletzander <mkletzan@redhat.com>
|
|
|
9119d9 |
Date: Sat, 13 Dec 2014 10:10:01 +0100
|
|
|
9119d9 |
Subject: [PATCH] CVE-2014-8131: Fix possible deadlock and segfault in
|
|
|
9119d9 |
qemuConnectGetAllDomainStats()
|
|
|
9119d9 |
|
|
|
9119d9 |
https://bugzilla.redhat.com/show_bug.cgi?id=1172570
|
|
|
9119d9 |
|
|
|
9119d9 |
When user doesn't have read access on one of the domains he requested,
|
|
|
9119d9 |
the for loop could exit abruptly or continue and override pointer which
|
|
|
9119d9 |
pointed to locked object.
|
|
|
9119d9 |
|
|
|
9119d9 |
This patch fixed two issues at once. One is that domflags might have
|
|
|
9119d9 |
had QEMU_DOMAIN_STATS_HAVE_JOB even when there was no job started (this
|
|
|
9119d9 |
is fixed by doing domflags |= QEMU_DOMAIN_STATS_HAVE_JOB only when the
|
|
|
9119d9 |
job was acquired and cleaning domflags on every start of the loop.
|
|
|
9119d9 |
Second one is that the domain is kept locked when
|
|
|
9119d9 |
virConnectGetAllDomainStatsCheckACL() fails and continues the loop when
|
|
|
9119d9 |
it didn't end. Adding a simple virObjectUnlock() and clearing the
|
|
|
9119d9 |
pointer ought to do.
|
|
|
9119d9 |
|
|
|
9119d9 |
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
|
|
9119d9 |
(cherry picked from commit 57023c0a3af4af1c547189c1f6712ed5edeb0c0b)
|
|
|
9119d9 |
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
|
|
9119d9 |
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
|
|
|
9119d9 |
---
|
|
|
9119d9 |
src/qemu/qemu_driver.c | 20 +++++++++++++-------
|
|
|
9119d9 |
1 file changed, 13 insertions(+), 7 deletions(-)
|
|
|
9119d9 |
|
|
|
9119d9 |
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
|
|
|
9119d9 |
index adf158a..1625ddd 100644
|
|
|
9119d9 |
--- a/src/qemu/qemu_driver.c
|
|
|
9119d9 |
+++ b/src/qemu/qemu_driver.c
|
|
|
9119d9 |
@@ -18250,20 +18250,23 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
|
|
|
9119d9 |
privflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
|
|
|
9119d9 |
|
|
|
9119d9 |
for (i = 0; i < ndoms; i++) {
|
|
|
9119d9 |
- domflags = privflags;
|
|
|
9119d9 |
virDomainStatsRecordPtr tmp = NULL;
|
|
|
9119d9 |
+ domflags = 0;
|
|
|
9119d9 |
|
|
|
9119d9 |
if (!(dom = qemuDomObjFromDomain(doms[i])))
|
|
|
9119d9 |
continue;
|
|
|
9119d9 |
|
|
|
9119d9 |
if (doms != domlist &&
|
|
|
9119d9 |
- !virConnectGetAllDomainStatsCheckACL(conn, dom->def))
|
|
|
9119d9 |
+ !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) {
|
|
|
9119d9 |
+ virObjectUnlock(dom);
|
|
|
9119d9 |
+ dom = NULL;
|
|
|
9119d9 |
continue;
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
|
|
|
9119d9 |
- if (HAVE_JOB(domflags) &&
|
|
|
9119d9 |
+ if (HAVE_JOB(privflags) &&
|
|
|
9119d9 |
qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0)
|
|
|
9119d9 |
/* As it was never requested. Gather as much as possible anyway. */
|
|
|
9119d9 |
- domflags &= ~QEMU_DOMAIN_STATS_HAVE_JOB;
|
|
|
9119d9 |
+ domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
|
|
|
9119d9 |
|
|
|
9119d9 |
if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
|
|
|
9119d9 |
goto endjob;
|
|
|
9119d9 |
@@ -18271,9 +18274,12 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
|
|
|
9119d9 |
if (tmp)
|
|
|
9119d9 |
tmpstats[nstats++] = tmp;
|
|
|
9119d9 |
|
|
|
9119d9 |
- if (HAVE_JOB(domflags) && !qemuDomainObjEndJob(driver, dom)) {
|
|
|
9119d9 |
- dom = NULL;
|
|
|
9119d9 |
- continue;
|
|
|
9119d9 |
+ if (HAVE_JOB(domflags)) {
|
|
|
9119d9 |
+ domflags = 0;
|
|
|
9119d9 |
+ if (!qemuDomainObjEndJob(driver, dom)) {
|
|
|
9119d9 |
+ dom = NULL;
|
|
|
9119d9 |
+ continue;
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
}
|
|
|
9119d9 |
|
|
|
9119d9 |
virObjectUnlock(dom);
|
|
|
9119d9 |
--
|
|
|
9119d9 |
2.2.0
|
|
|
9119d9 |
|