- Devel - libvirt List Archives

[libvirt] [PATCH] CVE-2014-8131: Fix possible deadlock and segfault in qemuConnectGetAllDomainStats()
by Martin Kletzander 11 Dec '14

11 Dec '14

When user doesn't have read access on one of the domains he requested, the for loop could exit abruptly or continue and override pointer which pointed to locked object. This patch fixed two issues at once. One is that domflags might have had QEMU_DOMAIN_STATS_HAVE_JOB even when there was no job started (this is fixed by doing domflags |= QEMU_DOMAIN_STATS_HAVE_JOB only when the job was acquired and cleaning domflags on every start of the loop. Second one is that the domain is kept locked when virConnectGetAllDomainStatsCheckACL() fails and continues the loop when it didn't end. Adding a simple virObjectUnlock() and clearing the pointer ought to do. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Since this is very low priority and it was mistakenly disclosed in another patch there is no embargo on this CVE. Patches to maint branches will be pushed after back-porting (couple of minutes). Having said that, I am probably not the right one to write up the libvirt security notice, but I'll try drafting one, eventually. src/qemu/qemu_driver.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index be37c8f..ae6225b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18740,20 +18740,23 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, privflags |= QEMU_DOMAIN_STATS_HAVE_JOB; for (i = 0; i < ndoms; i++) { - domflags = privflags; virDomainStatsRecordPtr tmp = NULL; + domflags = 0; if (!(dom = qemuDomObjFromDomain(doms[i]))) continue; if (doms != domlist && - !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) + !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) { + virObjectUnlock(dom); + dom = NULL; continue; + } - if (HAVE_JOB(domflags) && + if (HAVE_JOB(privflags) && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0) /* As it was never requested. Gather as much as possible anyway. */ - domflags &= ~QEMU_DOMAIN_STATS_HAVE_JOB; + domflags |= QEMU_DOMAIN_STATS_HAVE_JOB; if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0) goto endjob; @@ -18761,9 +18764,12 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, if (tmp) tmpstats[nstats++] = tmp; - if (HAVE_JOB(domflags) && !qemuDomainObjEndJob(driver, dom)) { - dom = NULL; - continue; + if (HAVE_JOB(domflags)) { + domflags = 0; + if (!qemuDomainObjEndJob(driver, dom)) { + dom = NULL; + continue; + } } virObjectUnlock(dom); -- 2.2.0

2 3

[libvirt] [PATCH] qemu: bulk stats: typo in monitor handling
by Francesco Romani 11 Dec '14

11 Dec '14

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(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; -- 1.9.3

4 3

[libvirt] [PATCH] qemu: bulk stats: Clean up code to check whether we need a domain job
by Peter Krempa 11 Dec '14

11 Dec '14

Storing the information as a flag in a unsigned int seems a bit wasteful. Store it in a bool instead. --- src/qemu/qemu_driver.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df3ba6d..5c60127 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18691,7 +18691,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, int nstats = 0; size_t i; int ret = -1; - unsigned int privflags = 0; + bool needjob; unsigned int domflags = 0; if (ndoms) @@ -18727,8 +18727,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, if (VIR_ALLOC_N(tmpstats, ndoms + 1) < 0) goto cleanup; - if (qemuDomainGetStatsNeedMonitor(stats)) - privflags |= QEMU_DOMAIN_STATS_HAVE_JOB; + bool needjob = qemuDomainGetStatsNeedMonitor(stats); for (i = 0; i < ndoms; i++) { virDomainStatsRecordPtr tmp = NULL; @@ -18744,7 +18743,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, continue; } - if (HAVE_JOB(privflags) && + if (needjob && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) == 0) domflags |= QEMU_DOMAIN_STATS_HAVE_JOB; /* else: without a job it's still possible to gather some data */ -- 2.2.0

2 1

Re: [libvirt] [virt-tools-list] [PATCH] virtinst: refresh pools status before fetch_pools
by Chun Yan Liu 11 Dec '14

11 Dec '14

>>> On 12/5/2014 at 09:54 PM, in message <5481B907.4040507(a)redhat.com>, Cole Robinson <crobinso(a)redhat.com> wrote: > On 12/05/2014 03:40 AM, Chunyan Liu wrote: > > Currently, when connecting to hypervisor, if there are pools active > > but in fact target path already deleted (or for other reasons the > > pool is not working), libvirtd not refresh status yet, fetch_pools > > will fail, that will cause "connecting to hypervisor" process > > reporting error and exit. The whole connection work failed. > > > > With the patch, always refresh pool status before fetch pools. Let > > the libvirtd pool status reflect the reality, avoid the non-synced > > status affects the hypervisor connection. > > > > Signed-off-by: Chunyan Liu <cyliu(a)suse.com> > > --- > > virtinst/pollhelpers.py | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/virtinst/pollhelpers.py b/virtinst/pollhelpers.py > > index a9b1527..e8702f0 100644 > > --- a/virtinst/pollhelpers.py > > +++ b/virtinst/pollhelpers.py > > @@ -133,6 +133,19 @@ def fetch_pools(backend, origmap, build_func): > > > > if backend.check_support( > > backend.SUPPORT_CONN_LISTALLSTORAGEPOOLS) and not > _force_old_poll: > > + > > + # Refresh pools before poll_helper. For those > > + # 'active' but target path not exist (or other reasons > > + # causing the pool not working), but libvirtd not > > + # refresh the status, this will make it refreshed > > + # and mark that pool as 'inactive'. > > + objs = backend.listAllStoragePools() > > + for obj in objs: > > + try: > > + obj.refresh(0) > > + except Exception, e: > > + pass > > + > > return _new_poll_helper(origmap, name, > > backend.listAllStoragePools, build_func) > > else: > > > > This is a very heavy hammer, refresh is a potentially long running operation > > so this could cause decent slowdown in some scenarios. > > IMO this is essentially a libvirt bug, for pools with target directories > (dir, > fs, netfs), libvirt should be periodically checking the directory ctime and > doing the pool refresh for us. And if the target has disappeared, it shuts > down the pool (like shutting down a VM if it crashes). Hi, libvirt list, I'm not sure if Cole's suggestion could be done in libvirt, so just forward the mail to libvirt mailing list. Any opinions? Chunyan > > We have so many hacks sprinkled around in virtinst/virt-manager dealing with > the fallout of this lacking libvirt feature. Really wish I had implemented > it > years ago. But I'd rather focus on that then adding yet more hacks. > > If there's a specific issue you're hitting that's manifesting itself > elsewhere > in the app, let us know and maybe there's a way to mitigate it in the > interim > > - Cole > > _______________________________________________ > virt-tools-list mailing list > virt-tools-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/virt-tools-list > >

3 2

[libvirt] [PATCHv3 0/2] Fix snapshot-revert and managedsave with host-passthrough CPU mode
by Ján Tomko 11 Dec '14

11 Dec '14

v1: https://www.redhat.com/archives/libvir-list/2014-October/msg01120.html v2: https://www.redhat.com/archives/libvir-list/2014-November/msg00218.html v3: * do not format the features at all, instead of adding a model to them Ján Tomko (2): Do not format CPU features without a model Ignore CPU features without a model for host-passthrough src/conf/cpu_conf.c | 7 ++- ...argv-cpu-host-passthrough-features-invalid.args | 22 +++++++++ ...2argv-cpu-host-passthrough-features-invalid.xml | 55 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.xml -- 2.0.4

2 4

[libvirt] [PATCH] storage: unify permission formatting
by Martin Kletzander 11 Dec '14

11 Dec '14

Volume and pool formatting functions took different approaches to unspecified uids/gids. When unknown, it is always parsed as -1, but one of the functions formatted it as unsigned int (wrong) and one as int (better). Due to that, our two of our XML files from tests cannot be parsed on 32-bit machines. RNG schema needs to be modified as well, but because both storagepool.rng and storagevol.rng need same schema for permission element, save some space by moving it to storagecommon.rng. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- docs/schemas/storagecommon.rng | 29 +++++++++++++++++++++++++ docs/schemas/storagepool.rng | 30 +------------------------- docs/schemas/storagevol.rng | 23 -------------------- src/conf/storage_conf.c | 9 ++++---- tests/storagevolxml2xmlout/vol-gluster-dir.xml | 4 ++-- tests/storagevolxml2xmlout/vol-sheepdog.xml | 4 ++-- 6 files changed, 38 insertions(+), 61 deletions(-) diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index 06b2f81..629505f 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -93,4 +93,33 @@ <notAllowed/> </define> + <define name='permissions'> + <optional> + <element name='permissions'> + <interleave> + <element name='mode'> + <ref name='octalMode'/> + </element> + <element name='owner'> + <choice> + <ref name='unsignedInt'/> + <value>-1</value> + </choice> + </element> + <element name='group'> + <choice> + <ref name='unsignedInt'/> + <value>-1</value> + </choice> + </element> + <optional> + <element name='label'> + <text/> + </element> + </optional> + </interleave> + </element> + </optional> + </define> + </grammar> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 0f05c5c..db6ff49 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -3,6 +3,7 @@ <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> <include href='basictypes.rng'/> + <include href='storagecommon.rng'/> <start> <ref name='pool'/> </start> @@ -224,35 +225,6 @@ </interleave> </define> - <define name='permissions'> - <optional> - <element name='permissions'> - <interleave> - <element name='mode'> - <ref name='octalMode'/> - </element> - <element name='owner'> - <choice> - <ref name='unsignedInt'/> - <value>-1</value> - </choice> - </element> - <element name='group'> - <choice> - <ref name='unsignedInt'/> - <value>-1</value> - </choice> - </element> - <optional> - <element name='label'> - <text/> - </element> - </optional> - </interleave> - </element> - </optional> - </define> - <define name='target'> <element name='target'> <interleave> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 1b2d4cc..7450547 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -59,29 +59,6 @@ </interleave> </define> - <define name='permissions'> - <optional> - <element name='permissions'> - <interleave> - <element name='mode'> - <ref name='octalMode'/> - </element> - <element name='owner'> - <ref name='unsignedInt'/> - </element> - <element name='group'> - <ref name='unsignedInt'/> - </element> - <optional> - <element name='label'> - <text/> - </element> - </optional> - </interleave> - </element> - </optional> - </define> - <define name='timestamps'> <optional> <element name='timestamps'> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 3987470..e1be064 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1203,7 +1203,6 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) (int) def->target.perms.uid); virBufferAsprintf(&buf, "<group>%d</group>\n", (int) def->target.perms.gid); - virBufferEscapeString(&buf, "<label>%s</label>\n", def->target.perms.label); @@ -1527,10 +1526,10 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAsprintf(buf, "<mode>0%o</mode>\n", def->perms->mode); - virBufferAsprintf(buf, "<owner>%u</owner>\n", - (unsigned int) def->perms->uid); - virBufferAsprintf(buf, "<group>%u</group>\n", - (unsigned int) def->perms->gid); + virBufferAsprintf(buf, "<owner>%d</owner>\n", + (int) def->perms->uid); + virBufferAsprintf(buf, "<group>%d</group>\n", + (int) def->perms->gid); virBufferEscapeString(buf, "<label>%s</label>\n", diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir.xml b/tests/storagevolxml2xmlout/vol-gluster-dir.xml index f188ceb..538b31d 100644 --- a/tests/storagevolxml2xmlout/vol-gluster-dir.xml +++ b/tests/storagevolxml2xmlout/vol-gluster-dir.xml @@ -10,8 +10,8 @@ <format type='dir'/> <permissions> <mode>0600</mode> - <owner>4294967295</owner> - <group>4294967295</group> + <owner>-1</owner> + <group>-1</group> </permissions> </target> </volume> diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml index e08e36c..0a1f32c 100644 --- a/tests/storagevolxml2xmlout/vol-sheepdog.xml +++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml @@ -9,8 +9,8 @@ <format type='unknown'/> <permissions> <mode>0600</mode> - <owner>4294967295</owner> - <group>4294967295</group> + <owner>-1</owner> + <group>-1</group> </permissions> </target> </volume> -- 2.2.0

2 2

[libvirt] [PATCHv4 2/2] security: Add a new func use stat to get process DAC label
by Luyao Huang 11 Dec '14

11 Dec '14

When use qemuProcessAttach to attach a qemu process, cannot get a right DAC label. Add a new func to get process label via stat func. Do not remove virDomainDefGetSecurityLabelDef before try to use stat to get process DAC label, because There are some other func call virSecurityDACGetProcessLabel. Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- v2 add support freeBSD. v3 use snprintf instead of VirAsprintf and move the error settings in virSecurityDACGetProcessLabelInternal. v4 remove errno.h include and thanks Eric advice move this version comment to this place. src/security/security_dac.c | 84 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 3 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..300b245 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -23,6 +23,11 @@ #include <sys/stat.h> #include <fcntl.h> +#ifdef __FreeBSD__ +# include <sys/sysctl.h> +# include <sys/user.h> +#endif + #include "security_dac.h" #include "virerror.h" #include "virfile.h" @@ -1236,17 +1241,90 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; } +#ifdef __linux__ +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct stat sb; + char *path = NULL; + int ret = -1; + + VIR_INFO("Getting DAC user and group on process '%d'", pid); + + if (virAsprintf(&path, "/proc/%d", (int) pid) < 0) + goto cleanup; + + if (lstat(path, &sb) < 0) { + virReportSystemError(errno, + _("unable to get PID %d uid and gid via stat"), + pid); + goto cleanup; + } + + snprintf(seclabel->label, VIR_SECURITY_LABEL_BUFLEN, + "+%u:+%u", (unsigned int) sb.st_uid, (unsigned int) sb.st_gid); + ret = 0; + +cleanup: + VIR_FREE(path); + return ret; +} +#elif defined(__FreeBSD__) +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct kinfo_proc p; + int mib[4]; + size_t len = 4; + + sysctlnametomib("kern.proc.pid", mib, &len); + + len = sizeof(struct kinfo_proc); + mib[3] = pid; + + if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) { + virReportSystemError(errno, + _("unable to get PID %d uid and gid via sysctl"), + pid); + return -1; + } + + snprintf(seclabel->label, VIR_SECURITY_LABEL_BUFLEN, + "+%u:+%u", (unsigned int) p.ki_ruid, (unsigned int) p.ki_rgid); + + return 0; +} +#else +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + "Cannot get proccess DAC label for pid %d on this platform", + (int) pid); + return -1; +} +#endif + static int virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, - pid_t pid ATTRIBUTE_UNUSED, + pid_t pid, virSecurityLabelPtr seclabel) { virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (!secdef || !seclabel) - return -1; + if (secdef == NULL) { + VIR_DEBUG("missing label for DAC security " + "driver in domain %s", def->name); + + if (virSecurityDACGetProcessLabelInternal(pid, seclabel) < 0) + return -1; + return 0; + } if (secdef->label) ignore_value(virStrcpy(seclabel->label, secdef->label, -- 1.8.3.1

3 2

[libvirt] libvirt issues in Fedora 21
by snaper＠openmailbox.org 11 Dec '14

11 Dec '14

Sorry if its not the right place to post. Fedora 21 user here and I've run into two problems that were not there in Fedora 20. First enabling hugepages causes a machine to refuse to boot. It gives errors saying "Error starting domain: Unable to read from monitor: Connection reset by peer " This problem is easily reproducible, all you have to do is add the setting to any vm you have. Second is the vm does not work if a shared folder is added. I only tried adding them as mapped mode. Please try to fix them.

2 1

[libvirt] [PATCH] qemu: bulk stats: add pcpu placement information
by Francesco Romani 11 Dec '14

11 Dec '14

This patch adds the information about the physical cpu placement of virtual cpus for bulk stats. This is the only difference in output with the virDomainGetVcpus() API. Management software, like oVirt, needs this information to properly manage NUMA configurations. Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- src/libvirt-domain.c | 2 ++ src/qemu/qemu_driver.c | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cb76d8c..63fc967 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -10888,6 +10888,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * from virVcpuState enum. * "vcpu.<num>.time" - virtual cpu time spent by virtual CPU <num> * as unsigned long long. + * "vcpu.<num>.physical" - real CPU number on which virtual CPU <num> is + * running, or -1 if offline. * * VIR_DOMAIN_STATS_INTERFACE: Return network interface statistics. * The typed parameter keys are in this format: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 830fca7..ab0652d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18348,6 +18348,15 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, param_name, cpuinfo[i].cpuTime) < 0) goto cleanup; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "vcpu.%zu.physical", i); + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + param_name, + cpuinfo[i].cpu) < 0) + goto cleanup; } ret = 0; -- 1.9.3

2 2

[libvirt] How to use blockcopy in libvirt-java
by Star Guo 11 Dec '14

11 Dec '14

Hi, ALL, I just find the page on git: https://github.com/qemu/qemu/blob/master/docs/live-block-ops.txt This metion “Live block copy”, so Live Block Copy is the feature of live-block-ops. And what about the live-block-migration ? Best Regards, Star Guo 发件人: Star Guo [mailto:starg@ceph.me] 发送时间: 2014年12月10日 18:17 收件人: 'libvirt-users(a)redhat.com' 抄送: 'libvir-list(a)redhat.com' 主题: How to use blockcopy in libvirt-java Hi, ALL, I just look for the class and method to use blockcopy in libvirt-java on this page: http://libvirt.org/sources/java/javadoc/ However, I can not find out. Libvirt-java Support blockcopy ? Thanks. Best Regards, Star Guo

1 0