From 838527c75969a7bcb88292683ebf3e902ed15843 Mon Sep 17 00:00:00 2001 From: Xu Yandong Date: Sat, 9 May 2020 17:26:00 +0800 Subject: [PATCH] Fix CVE-2019-20485 Signed-off-by: Xu Yandong --- ...emu-don-t-hold-both-jobs-for-suspend.patch | 173 ++++++++++++++++++ libvirt.spec | 5 +- 2 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 libvirt-qemu-don-t-hold-both-jobs-for-suspend.patch diff --git a/libvirt-qemu-don-t-hold-both-jobs-for-suspend.patch b/libvirt-qemu-don-t-hold-both-jobs-for-suspend.patch new file mode 100644 index 0000000..8bf7d8d --- /dev/null +++ b/libvirt-qemu-don-t-hold-both-jobs-for-suspend.patch @@ -0,0 +1,173 @@ +From f2110c4d26ec4cef5384edbdd23a31d5691cb924 Mon Sep 17 00:00:00 2001 +From: Xu Yandong +Date: Sat, 9 May 2020 14:54:30 +0800 +Subject: [PATCH] qemu: don't hold both jobs for suspend + +We have to assume that the guest agent may be malicious so we don't want +to allow any agent queries to block any other libvirt API. By holding a +monitor job while we're querying the agent, we open ourselves up to a +DoS. + +So split the function up a bit to only hold the monitor job while +querying qemu for whether the domain supports suspend. Then acquire only +an agent job while issuing the agent suspend command. + +Signed-off-by: Jonathon Jongsma +Signed-off-by: Michal Privoznik +Reviewed-by: Michal Privoznik +(cherry-picked from commit 7317a83f6024dd6b4e1fa19436e24db0132b6cb9) +Signed-off-by: Xu Yandong +--- + src/qemu/qemu_driver.c | 94 ++++++++++++++++++++++++++---------------- + 1 file changed, 59 insertions(+), 35 deletions(-) + +diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c +index 8749c53..dad0710 100644 +--- a/src/qemu/qemu_driver.c ++++ b/src/qemu/qemu_driver.c +@@ -19088,6 +19088,59 @@ qemuDomainProbeQMPCurrentMachine(virQEMUDriverPtr driver, + } + + ++/* returns -1 on error, or if query is not supported, 0 if query was successful */ ++static int ++qemuDomainQueryWakeupSuspendSupport(virQEMUDriverPtr driver, ++ virDomainObjPtr vm, ++ bool *wakeupSupported) ++{ ++ qemuDomainObjPrivatePtr priv = vm->privateData; ++ int ret = -1; ++ ++ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE)) ++ return -1; ++ ++ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) ++ return -1; ++ ++ if ((ret = virDomainObjCheckActive(vm)) < 0) ++ goto endjob; ++ ++ ret = qemuDomainProbeQMPCurrentMachine(driver, vm, wakeupSupported); ++ ++ endjob: ++ qemuDomainObjEndJob(driver, vm); ++ return ret; ++} ++ ++ ++static int ++qemuDomainPMSuspendAgent(virQEMUDriverPtr driver, ++ virDomainObjPtr vm, ++ unsigned int target) ++{ ++ qemuAgentPtr agent; ++ int ret = -1; ++ ++ if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_MODIFY) < 0) ++ return -1; ++ ++ if ((ret = virDomainObjCheckActive(vm)) < 0) ++ goto endjob; ++ ++ if (!qemuDomainAgentAvailable(vm, true)) ++ goto endjob; ++ ++ agent = qemuDomainObjEnterAgent(vm); ++ ret = qemuAgentSuspend(agent, target); ++ qemuDomainObjExitAgent(vm, agent); ++ ++ endjob: ++ qemuDomainObjEndAgentJob(vm); ++ return ret; ++} ++ ++ + static int + qemuDomainPMSuspendForDuration(virDomainPtr dom, + unsigned int target, +@@ -19095,11 +19148,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, + unsigned int flags) + { + virQEMUDriverPtr driver = dom->conn->privateData; +- qemuDomainObjPrivatePtr priv; + virDomainObjPtr vm; +- qemuAgentPtr agent; +- qemuDomainJob job = QEMU_JOB_NONE; + int ret = -1; ++ bool wakeupSupported; + + virCheckFlags(0, -1); + +@@ -19124,17 +19175,6 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, + if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + +- priv = vm->privateData; +- +- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE)) +- job = QEMU_JOB_MODIFY; +- +- if (qemuDomainObjBeginJobWithAgent(driver, vm, job, QEMU_AGENT_JOB_MODIFY) < 0) +- goto cleanup; +- +- if (virDomainObjCheckActive(vm) < 0) +- goto endjob; +- + /* + * The case we want to handle here is when QEMU has the API (i.e. + * QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere +@@ -19142,16 +19182,11 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, + * that don't know about this cap, will keep their old behavior of + * suspending 'in the dark'. + */ +- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE)) { +- bool wakeupSupported; +- +- if (qemuDomainProbeQMPCurrentMachine(driver, vm, &wakeupSupported) < 0) +- goto endjob; +- ++ if (qemuDomainQueryWakeupSuspendSupport(driver, vm, &wakeupSupported) == 0) { + if (!wakeupSupported) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Domain does not have suspend support")); +- goto endjob; ++ goto cleanup; + } + } + +@@ -19161,29 +19196,18 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, + target == VIR_NODE_SUSPEND_TARGET_HYBRID)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S3 state is disabled for this domain")); +- goto endjob; ++ goto cleanup; + } + + if (vm->def->pm.s4 == VIR_TRISTATE_BOOL_NO && + target == VIR_NODE_SUSPEND_TARGET_DISK) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S4 state is disabled for this domain")); +- goto endjob; ++ goto cleanup; + } + } + +- if (!qemuDomainAgentAvailable(vm, true)) +- goto endjob; +- +- agent = qemuDomainObjEnterAgent(vm); +- ret = qemuAgentSuspend(agent, target); +- qemuDomainObjExitAgent(vm, agent); +- +- endjob: +- if (job) +- qemuDomainObjEndJobWithAgent(driver, vm); +- else +- qemuDomainObjEndAgentJob(vm); ++ ret = qemuDomainPMSuspendAgent(driver, vm, target); + + cleanup: + virDomainObjEndAPI(&vm); +-- +2.21.0 + diff --git a/libvirt.spec b/libvirt.spec index 49268ec..dcf0521 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -114,7 +114,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 5.5.0 -Release: 6 +Release: 7 License: LGPLv2+ URL: https://libvirt.org/ @@ -169,6 +169,7 @@ Patch43: libvirt-qemu-avoid-double-reservation-of-PCI-address-for-int.patch Patch44: libvirt-qemu-Forcibly-mknod-even-if-it-exists.patch Patch45: libvirt-qemu-homogenize-MAC-address-in-live-config-when.patch Patch46: libvirt-po-Refresh-translation-for-running-state.patch +Patch47: libvirt-qemu-don-t-hold-both-jobs-for-suspend.patch @@ -1813,6 +1814,8 @@ exit 0 %changelog +* Sat May 09 2020 Xu Yandong - 5.5.0-7 +- Cherry-pick CVE-2019-20485 patches. * Mon Jan 06 2020 Xu Yandong - 5.5.0-6 - Translate running state to chinese. * Wed Dec 25 2019 Xu Yandong - 5.5.0-5 -- Gitee