From 9f9eae34c1208f8b24fbd96669c71ebfa143b31c Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Tue, 12 Aug 2014 13:58:50 -0400 Subject: [PATCH] Rebased to version 1.2.7 --- ...ly-report-active-commit-for-job-info.patch | 36 ++++++ ...avoid-memory-leak-during-block-pivot.patch | 51 +++++++++ ...kjob-fix-use-after-free-in-blockcopy.patch | 108 ++++++++++++++++++ libvirt.spec | 104 ++++++++++++----- sources | 2 +- 5 files changed, 270 insertions(+), 31 deletions(-) create mode 100644 0001-blockjob-correctly-report-active-commit-for-job-info.patch create mode 100644 0002-blockjob-avoid-memory-leak-during-block-pivot.patch create mode 100644 0003-blockjob-fix-use-after-free-in-blockcopy.patch diff --git a/0001-blockjob-correctly-report-active-commit-for-job-info.patch b/0001-blockjob-correctly-report-active-commit-for-job-info.patch new file mode 100644 index 0000000..b6bfe3a --- /dev/null +++ b/0001-blockjob-correctly-report-active-commit-for-job-info.patch @@ -0,0 +1,36 @@ +From 2151695a5119a8d7f44d416c730df50a1e42695a Mon Sep 17 00:00:00 2001 +Message-Id: <2151695a5119a8d7f44d416c730df50a1e42695a.1407860168.git.crobinso@redhat.com> +From: Eric Blake +Date: Tue, 5 Aug 2014 08:49:32 -0600 +Subject: [PATCH 1/3] blockjob: correctly report active commit for job info + +Commit 232a31b munged job info to report 'active commit' instead of +'commit' when generating events, but forgot to also munge the polling +variant of the command. + +* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Adjust type as +needed. + +Signed-off-by: Eric Blake +(cherry picked from commit e8cc973041e7ac4ddeefe343af751863c76687fe) +--- + src/qemu/qemu_driver.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c +index a3de784..57cc913 100644 +--- a/src/qemu/qemu_driver.c ++++ b/src/qemu/qemu_driver.c +@@ -15103,6 +15103,9 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, + ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, + bandwidth, info, mode, async); + qemuDomainObjExitMonitor(driver, vm); ++ if (info && info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT && ++ disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) ++ info->type = disk->mirrorJob; + if (ret < 0) { + if (mode == BLOCK_JOB_ABORT && disk->mirror) + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; +-- +1.9.3 + diff --git a/0002-blockjob-avoid-memory-leak-during-block-pivot.patch b/0002-blockjob-avoid-memory-leak-during-block-pivot.patch new file mode 100644 index 0000000..fd306c0 --- /dev/null +++ b/0002-blockjob-avoid-memory-leak-during-block-pivot.patch @@ -0,0 +1,51 @@ +From 7620b422e515249bbfff02d0372301334fe1dd50 Mon Sep 17 00:00:00 2001 +Message-Id: <7620b422e515249bbfff02d0372301334fe1dd50.1407860168.git.crobinso@redhat.com> +In-Reply-To: <2151695a5119a8d7f44d416c730df50a1e42695a.1407860168.git.crobinso@redhat.com> +References: <2151695a5119a8d7f44d416c730df50a1e42695a.1407860168.git.crobinso@redhat.com> +From: Eric Blake +Date: Wed, 6 Aug 2014 14:48:59 -0600 +Subject: [PATCH 2/3] blockjob: avoid memory leak during block pivot + +Valgrind caught a memory leak: + +==2018== 9 bytes in 1 blocks are definitely lost in loss record 143 of 927 +==2018== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) +==2018== by 0x8C42369: strdup (strdup.c:42) +==2018== by 0x50EACC9: virStrdup (virstring.c:676) +==2018== by 0x50E79E5: virStorageSourceCopy (virstoragefile.c:1845) +==2018== by 0x20A3FAA7: qemuDomainBlockCommit (qemu_driver.c:15620) +==2018== by 0x51DC6B2: virDomainBlockCommit (libvirt.c:20092) + +I traced it to the fact that blockcopy and blockcommit end up +reparsing a backing chain on pivot, but the chain parsing code +doesn't gracefully handle the case where the backing file is +already known. + +I'm not exactly sure when this was introduced, but suspect that the +refactoring in commit 9944b71 and friends that moved towards probing +in-place rather than into a temporary structure are part of the cause. + +* src/util/virstoragefile.c (virStorageFileGetMetadataInternal): +Don't leak any prior value. + +Signed-off-by: Eric Blake +(cherry picked from commit a595a005725f142e1a258d10f7647982efa3cfd8) +--- + src/util/virstoragefile.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c +index 3da9073..5b6b2f5 100644 +--- a/src/util/virstoragefile.c ++++ b/src/util/virstoragefile.c +@@ -817,6 +817,7 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta, + goto cleanup; + } + ++ VIR_FREE(meta->backingStoreRaw); + if (fileTypeInfo[meta->format].getBackingStore != NULL) { + int store = fileTypeInfo[meta->format].getBackingStore(&meta->backingStoreRaw, + backingFormat, +-- +1.9.3 + diff --git a/0003-blockjob-fix-use-after-free-in-blockcopy.patch b/0003-blockjob-fix-use-after-free-in-blockcopy.patch new file mode 100644 index 0000000..f510230 --- /dev/null +++ b/0003-blockjob-fix-use-after-free-in-blockcopy.patch @@ -0,0 +1,108 @@ +From 9617e31b5349b193469874706abcbcb013e6a6fd Mon Sep 17 00:00:00 2001 +Message-Id: <9617e31b5349b193469874706abcbcb013e6a6fd.1407860168.git.crobinso@redhat.com> +In-Reply-To: <2151695a5119a8d7f44d416c730df50a1e42695a.1407860168.git.crobinso@redhat.com> +References: <2151695a5119a8d7f44d416c730df50a1e42695a.1407860168.git.crobinso@redhat.com> +From: Eric Blake +Date: Wed, 6 Aug 2014 14:06:23 -0600 +Subject: [PATCH 3/3] blockjob: fix use-after-free in blockcopy + +Commit febf84c2 tried to delay in-memory modification of the actual +domain disk structure until after the qemu event was received. +However, I missed that the code for block pivot had been temporarily +setting disk->src = disk->mirror prior to the qemu command, in order +to label the backing chain of a reused external blockcopy disk; +and calls into qemu while still in that state before finally undoing +things at the cleanup label. Since the qemu event handler then does: + virStorageSourceFree(disk->src); + disk->src = disk->mirror; +we have the sad race that a fast enough qemu event can cause a leak of +the original disk->src, as well as a use-after-free of the disk->mirror +contents, bad enough to crash libvirtd in some of my test runs, even +though the common case of the qemu event being much later won't trip +the race. + +I'll go wear the brown paper bag of shame, for introducing a crasher +in between rc1 and rc2 of the freeze for 1.2.7 :( My only +consolation is that virDomainBlockJobAbort requires the domain:write +ACL, so it is not a CVE. + +The valgrind report when the race occurs looks like: + +==25612== Invalid read of size 4 +==25612== at 0x50E7C90: virStorageSourceGetActualType (virstoragefile.c:1948) +==25612== by 0x209C0B18: qemuDomainDetermineDiskChain (qemu_domain.c:2473) +==25612== by 0x209D7F6A: qemuProcessHandleBlockJob (qemu_process.c:1087) +==25612== by 0x209F40C9: qemuMonitorEmitBlockJob (qemu_monitor.c:1357) +... +==25612== Address 0xe4b5610 is 0 bytes inside a block of size 200 free'd +==25612== at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) +==25612== by 0x50839E9: virFree (viralloc.c:582) +==25612== by 0x50E7E51: virStorageSourceFree (virstoragefile.c:2015) +==25612== by 0x209D7EFF: qemuProcessHandleBlockJob (qemu_process.c:1073) +==25612== by 0x209F40C9: qemuMonitorEmitBlockJob (qemu_monitor.c:1357) + +* src/qemu/qemu_driver.c (qemuDomainBlockPivot): Don't corrupt +disk->src, and only label chain for blockcopy. + +Signed-off-by: Eric Blake +(cherry picked from commit 265680c58ebbee30bb70369e7d9905a599afbd6a) +--- + src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++--------------- + 1 file changed, 25 insertions(+), 15 deletions(-) + +diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c +index 57cc913..a050dbc 100644 +--- a/src/qemu/qemu_driver.c ++++ b/src/qemu/qemu_driver.c +@@ -14888,23 +14888,33 @@ qemuDomainBlockPivot(virConnectPtr conn, + } + } + +- /* We previously labeled only the top-level image; but if the +- * image includes a relative backing file, the pivot may result in +- * qemu needing to open the entire backing chain, so we need to +- * label the entire chain. This action is safe even if the +- * backing chain has already been labeled; but only necessary when +- * we know for sure that there is a backing chain. */ +- oldsrc = disk->src; +- disk->src = disk->mirror; ++ /* For active commit, the mirror is part of the already labeled ++ * chain. For blockcopy, we previously labeled only the top-level ++ * image; but if the user is reusing an external image that ++ * includes a backing file, the pivot may result in qemu needing ++ * to open the entire backing chain, so we need to label the ++ * entire chain. This action is safe even if the backing chain ++ * has already been labeled; but only necessary when we know for ++ * sure that there is a backing chain. */ ++ if (disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { ++ oldsrc = disk->src; ++ disk->src = disk->mirror; ++ ++ if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) ++ goto cleanup; + +- if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) +- goto cleanup; ++ if (disk->mirror->format && ++ disk->mirror->format != VIR_STORAGE_FILE_RAW && ++ (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, ++ disk) < 0 || ++ qemuSetupDiskCgroup(vm, disk) < 0 || ++ virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, ++ disk) < 0)) ++ goto cleanup; + +- if (disk->mirror->format && disk->mirror->format != VIR_STORAGE_FILE_RAW && +- (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, disk) < 0 || +- qemuSetupDiskCgroup(vm, disk) < 0 || +- virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, disk) < 0)) +- goto cleanup; ++ disk->src = oldsrc; ++ oldsrc = NULL; ++ } + + /* Attempt the pivot. Record the attempt now, to prevent duplicate + * attempts; but the actual disk change will be made when emitting +-- +1.9.3 + diff --git a/libvirt.spec b/libvirt.spec index 213760d..58e37d8 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -54,11 +54,28 @@ %define with_vbox 0%{!?_without_vbox:%{server_drivers}} %define with_qemu_tcg %{with_qemu} -# Change if we ever provide qemu-kvm binaries on non-x86 hosts -%if 0%{?fedora} >= 20 - %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64 -%else - %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x + +%define qemu_kvm_arches %{ix86} x86_64 + +%if 0%{?fedora} + %if 0%{?fedora} < 16 + # Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc + # I think F17 is the first release with the power64 macro + %ifarch ppc64 + %define with_qemu_tcg 0 + %endif + %endif + %if 0%{?fedora} >= 18 + %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x + %endif + %if 0%{?fedora} >= 20 + %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64 + %endif +%endif + +%if 0%{?rhel} + %define with_qemu_tcg 0 + %define qemu_kvm_arches x86_64 %endif %ifarch %{qemu_kvm_arches} @@ -67,6 +84,10 @@ %define with_qemu_kvm 0 %endif +%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm} + %define with_qemu 0 +%endif + # Then the hypervisor drivers that run outside libvirtd, in libvirt.so %define with_openvz 0%{!?_without_openvz:1} %define with_vmware 0%{!?_without_vmware:1} @@ -187,27 +208,21 @@ %define with_firewalld 1 %endif -# RHEL-5 has restricted QEMU to x86_64 only and is too old for LXC +# RHEL-5 is too old for LXC %if 0%{?rhel} == 5 - %define with_qemu_tcg 0 - %ifnarch x86_64 - %define with_qemu 0 - %define with_qemu_kvm 0 - %endif %define with_lxc 0 %endif -# RHEL-6 has restricted QEMU to x86_64 only, stopped including Xen -# on all archs. Other archs all have LXC available though +# RHEL-6 stopped including Xen on all archs. %if 0%{?rhel} >= 6 - %define with_qemu_tcg 0 - %ifnarch x86_64 - %define with_qemu 0 - %define with_qemu_kvm 0 - %endif %define with_xen 0 %endif +# Fedora doesn't have new enough Xen for libxl until F18 +%if 0%{?fedora} && 0%{?fedora} < 18 + %define with_libxl 0 +%endif + # PolicyKit was introduced in Fedora 8 / RHEL-6 or newer %if 0%{?fedora} >= 8 || 0%{?rhel} >= 6 %define with_polkit 0%{!?_without_polkit:1} @@ -246,12 +261,12 @@ %endif # Enable sanlock library for lock management with QEMU -# Sanlock is available only on x86_64 for RHEL +# Sanlock is available only on arches where kvm is available for RHEL %if 0%{?fedora} >= 16 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}} %endif %if 0%{?rhel} >= 6 - %ifarch x86_64 + %ifarch %{qemu_kvm_arches} %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}} %endif %endif @@ -262,9 +277,12 @@ %endif # Enable wireshark plugins for all distros shipping libvirt 1.2.2 or newer -%if 0%{?fedora} >= 21 - %define with_wireshark 0%{!?_without_wireshark:1} -%endif +#%if 0%{?fedora} >= 21 +# %define with_wireshark 0%{!?_without_wireshark:1} +#%endif +# Except this is presently busted on F21/rawhide with wireshark 1.12.0 +# https://bugzilla.redhat.com/show_bug.cgi?id=1129419 +%define with_wireshark 0 # Disable some drivers when building without libvirt daemon. # The logic is the same as in configure.ac @@ -368,8 +386,8 @@ Summary: Library providing a simple virtualization API Name: libvirt -Version: 1.2.6 -Release: 2%{?dist}%{?extra_release} +Version: 1.2.7 +Release: 1%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root @@ -380,6 +398,10 @@ URL: http://libvirt.org/ %endif Source: http://libvirt.org/sources/%{?mainturl}libvirt-%{version}.tar.gz +Patch0001: 0001-blockjob-correctly-report-active-commit-for-job-info.patch +Patch0002: 0002-blockjob-avoid-memory-leak-during-block-pivot.patch +Patch0003: 0003-blockjob-fix-use-after-free-in-blockcopy.patch + %if %{with_libvirtd} Requires: libvirt-daemon = %{version}-%{release} %if %{with_network} @@ -506,7 +528,9 @@ BuildRequires: libapparmor-devel %if %{with_network} BuildRequires: dnsmasq >= 2.41 BuildRequires: iptables + %if (0%{?fedora} && 0%{?fedora} < 17) || (0%{?rhel} && 0%{?rhel} < 7) BuildRequires: iptables-ipv6 + %endif BuildRequires: radvd %endif %if %{with_nwfilter} @@ -517,10 +541,14 @@ BuildRequires: module-init-tools BuildRequires: cyrus-sasl-devel %endif %if %{with_polkit} - %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 -BuildRequires: polkit-devel >= 0.93 + %if 0%{?fedora} >= 20 || 0%{?rhel} >= 7 +BuildRequires: polkit-devel >= 0.112 %else + %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 +BuildRequires: polkit-devel >= 0.93 + %else BuildRequires: PolicyKit-devel >= 0.6 + %endif %endif %endif %if %{with_storage_fs} @@ -680,10 +708,14 @@ Requires: avahi-libs %endif %endif %if %{with_polkit} - %if 0%{?fedora} >= 12 || 0%{?rhel} >=6 -Requires: polkit >= 0.93 + %if 0%{?fedora} >= 20 || 0%{?rhel} >= 7 +Requires: polkit >= 0.112 %else + %if 0%{?fedora} >= 12 || 0%{?rhel} >=6 +Requires: polkit >= 0.93 + %else Requires: PolicyKit >= 0.6 + %endif %endif %endif %if %{with_cgconfig} @@ -750,7 +782,9 @@ Requires: libvirt-daemon = %{version}-%{release} Requires: dnsmasq >= 2.41 Requires: radvd Requires: iptables + %if (0%{?fedora} && 0%{?fedora} < 17) || (0%{?rhel} && 0%{?rhel} < 7) Requires: iptables-ipv6 + %endif %description daemon-driver-network The network driver plugin for the libvirtd daemon, providing @@ -765,7 +799,9 @@ Summary: Nwfilter driver plugin for the libvirtd daemon Group: Development/Libraries Requires: libvirt-daemon = %{version}-%{release} Requires: iptables + %if (0%{?fedora} && 0%{?fedora} < 17) || (0%{?rhel} && 0%{?rhel} < 7) Requires: iptables-ipv6 + %endif Requires: ebtables %description daemon-driver-nwfilter @@ -1189,6 +1225,10 @@ driver %prep %setup -q +%patch0001 -p1 +%patch0002 -p1 +%patch0003 -p1 + %build %if ! %{with_xen} %define _without_xen --without-xen @@ -1477,7 +1517,7 @@ rm -fr %{buildroot} # on RHEL 5, thus we need to expand it here. make install DESTDIR=%{?buildroot} SYSTEMD_UNIT_DIR=%{_unitdir} -for i in object-events dominfo domsuspend hellolibvirt openauth xml/nwfilter systemtap dommigrate +for i in object-events dominfo domsuspend hellolibvirt openauth xml/nwfilter systemtap dommigrate domtop do (cd examples/$i ; make clean ; rm -rf .deps .libs Makefile Makefile.in) done @@ -2148,6 +2188,7 @@ exit 0 %{_datadir}/libvirt/schemas/basictypes.rng %{_datadir}/libvirt/schemas/capability.rng %{_datadir}/libvirt/schemas/domain.rng +%{_datadir}/libvirt/schemas/domaincaps.rng %{_datadir}/libvirt/schemas/domaincommon.rng %{_datadir}/libvirt/schemas/domainsnapshot.rng %{_datadir}/libvirt/schemas/interface.rng @@ -2220,6 +2261,9 @@ exit 0 %doc examples/systemtap %changelog +* Tue Aug 12 2014 Cole Robinson - 1.2.7-1 +- Rebased to version 1.2.7 + * Tue Jul 15 2014 Peter Robinson 1.2.6-2 - Enable kvm on aarch64 - Cleanup F-16/18 conditionals diff --git a/sources b/sources index 6e3c47e..031bc84 100644 --- a/sources +++ b/sources @@ -1 +1 @@ -ac1c3edbafb38f7978debe9507e5515c libvirt-1.2.6.tar.gz +d556b3d815a222fd9680f9f3948595cb libvirt-1.2.7.tar.gz