[libvirt] Segfault in virDomainObjListSearchName when listing domains (qemu backend)
by Guido Winkelmann
Hi,
I'm seeing a crash in libvirt when trying to list all domains using virsh.
Here's the backtrace:
=====================
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffeebfd710 (LWP 1691)]
0x00007ffff7411746 in __strcmp_sse42 () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install
openssl-1.0.0a-1.fc12.x86_64
(gdb) bt
#0 0x00007ffff7411746 in __strcmp_sse42 () from /lib64/libc.so.6
#1 0x00007ffff7ac9d79 in virDomainObjListSearchName (payload=0x73fdd0,
name=<value optimized out>, data=0x7fffdc0008c0) at conf/domain_conf.c:367
#2 0x00007ffff7ab476e in virHashSearch (table=0x6f9c30, iter=0x7ffff7ac9d50
<virDomainObjListSearchName>, data=0x7fffdc0008c0) at util/hash.c:582
#3 0x00007ffff7ac9d33 in virDomainFindByName (doms=<value optimized out>,
name=0x7fffdc0008c0 "basiccentos54image") at conf/domain_conf.c:377
#4 0x00000000004430f6 in qemudDomainLookupByName (conn=0x7fffe8000a80,
name=0x7fffdc0008c0 "basiccentos54image") at qemu/qemu_driver.c:4166
#5 0x00007ffff7af95cd in virDomainLookupByName (conn=0x7fffe8000a80,
name=0x7fffdc0008c0 "basiccentos54image") at libvirt.c:2169
#6 0x0000000000423e64 in remoteDispatchDomainLookupByName (server=<value
optimized out>, client=<value optimized out>, conn=0x7fffe8000a80, hdr=<value
optimized out>, rerr=0x7fffeebfcc70,
args=<value optimized out>, ret=0x7fffeebfcbc0) at remote.c:2030
#7 0x0000000000426a91 in remoteDispatchClientCall (server=<value optimized
out>, client=0x7ffff0001300, msg=0x7ffff0041570) at dispatch.c:506
#8 0x0000000000426e43 in remoteDispatchClientRequest (server=0x6e3cd0,
client=0x7ffff0001300, msg=0x7ffff0041570) at dispatch.c:388
#9 0x0000000000417ed8 in qemudWorker (data=0x7ffff0000908) at libvirtd.c:1568
#10 0x00007ffff7878a3a in start_thread () from /lib64/libpthread.so.0
#11 0x00007ffff73d377d in clone () from /lib64/libc.so.6
#12 0x0000000000000000 in ?? ()
(gdb)
=====================
This is with the newest version from git, pulled about 30 minutes ago.
This happens when I try to start up one of the defined domains using either
libvirt or virsh and then try to list all the defined domains in virsh using
list --all.
The attempt to start one of the domains already fails with the following
output:
=====================
virsh # start testserver-a
error: Failed to start domain testserver-a
error: internal error process exited while connecting to monitor:
17:21:14.760: debug : virCgroupNew:542 : New group /libvirt/qemu/testserver-a
17:21:14.760: debug : virCgroupDetect:232 : Detected mount/mapping 0:cpu at
/mnt/cgroups/cpu in /sysdefault
17:21:14.760: debug : virCgroupDetect:232 : Detected mount/mapping 1:cpuacct
at /mnt/cgroups/cpuacct in /sysdefault
17:21:14.760: debug : virCgroupDetect:232 : Detected mount/mapping 3:memory at
/mnt/cgroups/memory in /sysdefault
17:21:14.760: debug : virCgroupDetect:232 : Detected mount/mapping 4:devices
at /mnt/cgroups/devices in /sysdefault
17:21:14.760: debug : virCgroupMakeGroup:484 : Make group
/libvirt/qemu/testserver-a
17:21:14.760: debug : virCgroupMakeGroup:496 : Make controller
/mnt/cgroups/cpu/sysdefault/libvirt/qemu/testserver-a/
17:21:14.760: debug : virCgroupMakeGroup:496 : Make controller
/mnt/cgroups/cpuacct/sysdefault/libvirt/qemu/testserver-a/
17:21:14.760: debug : virCgroupMakeGroup:496 : Make controller
/mnt/cgroups/memory/sysdefault/libvirt/qemu/testserver-a/
=====================
This happens with all the domains I have currently defined.
Calling list --all before that produces no problems.
Calling list --all after that always produces said crash.
qemu is qemu-kvm 0.12.4, built from sources.
The host system is a Fedora 12 install.
Guido
14 years, 4 months
[libvirt] [PATCH 00/11] CVE: Multiple flaws in disk handling
by Daniel P. Berrange
This patch series attempts to fix 3 security flaws in the handling
of virtual disk formats. This is just another occurrance of the
problem previously identified in Xen
https://www.redhat.com/security/data/cve/CVE-2008-2004.html
In essence, if a guest is configured with a disk, hda, backed in
the host by a plain file, then admin in the guest OS can run
'qemu-img -f qcow2 /dev/hda' to write a qcow2 header into the disk.
If the host OS ever probes the backing file to identify its format,
then it will see it as a qcow2 file now, instead of the expected
raw.
In QEMU this problem is addressed in two ways
- When configuring a guest with -drive, always set format=raw|qcow2|etc
Libvirt will always set this when <driver type='raw|qcow2|etc'>
is set for a disk.
- When creating a qcow2 guest, always set an explicit backing
store format with -o backing_store_format=raw|qcow2|etc
or -F raw|qcow2|etc. Libvirt will always set this when a
<format type='raw|qcow2'/> is set for a volume backing store
Despite having support for these QEMU features to prevent probing,
there are flaws in the way libvirt handles disks in various places.
- In the security drivers (SELinux, CGroups, DAC) when labelling
files on the host, instead of looking at the <driver type='raw|qcow2'/>
in the XML to identify the disk format, libvirt probes the file for
magic data. Thus even if the app has taken the care to set an
explicit format, for all disks, libvirt can be tricked into setting
security labels on files that it shouldn't. If the guest OS somehow
manages to break out and compromise QEMU it can take advantage of
this labelling flaw to access files it shouldn't.
- In the security drivers (SELinux, CGroups, DAC) when labelling
the backing store for files on the host, instead of looking at
embedded backing store format in the master disk image, libvirt
probes magic data. Thus even if the app has taken the care to set
an explicit format, for all disk backing stores, libvirt can be
tricked into setting security labels on files that it shouldn't.
If the guest OS somehow manages to break out and compromise QEMU
it can take advantage of this labelling flaw to access files it
shouldn't.
- In the storage driver that creates qcow2 files, the backing store
format is only set explicitly if the kvm-img binary is found. Any
users with just a qemu-img binary will not have the backing store
format set, even if qemu-img supports this capability.
As a general matter of policy, libvirt does not mandate use of the
<driver type='raw|qcow2'/> tag in the XML for disks, and if omitted
will allow QEMU to probe disk formats. This is a weak default policy.
If no explicit driver is set, libvirt should presume raw, since that
is the only safe option. It should require a host level configuration
option for the QEMU driver, to allow probing to be used.
This patch series addresses all three of the security flaws, and also
changes the default QEMU driver policy to disable all disk format
probing out of the box. This latter change will break some existing
deployments, so it is possible to turn it off in qemu.conf.
The changes required significant re-factoring of the util/storage_file.c
code and centralizes all processing of disk backing stores to minimise
the risk of mistakes.
There are also 5 additional test cases to the TCK, 3 to verify the
security flaws, and 2 to verify that libvirtd is configured to disallow
disk format probing.
The file based storage pool needs to report the format of every volume
in the pool. There is no way todo this other than to probe the volume
metadata. Unfortunately if an application like virt-manager directly
copies the <format> from the storage volume XML, straight into the
<driver type=> of the guest disk, all our work to avoid the security
problems is potentially null & void. NB, this is only a problem if
re-using a pre-existing storage volume. If creating a new storage vol
when provisioning a guest / hotplugging a disk, virt-manager can trust
the storage volume format.
I think we need to advise applications that when picking a pre-existing
storage volume from a pool, they must not trust the volume format, and
must confirm with the user whether to use the probed format, or force to
raw
The CVEs are
https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2010-2237
https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2010-2238
https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2010-2239
In addition, this patch series fixes the problem of mutually recursive
backing stores which would cause an infinite loop in libvirtd.
Regards,
Daniel
14 years, 4 months
[libvirt] [PATCH 0/2] Explicitly manage balloon devices
by Daniel P. Berrange
The balloon device is not present in the XML currently. This means
we can't track custom PCI addresses for it. This means we can't
provide a way for apps to get the same PCI address ordering as
found with older QEMU
14 years, 4 months
[libvirt] [PATCH] Remove inappropriate use of VIR_ERR_NO_SUPPORT
by Daniel P. Berrange
The VIR_ERR_NO_SUPPORT refers to an API which is not implemented.
There is a separate VIR_ERR_CONFIG_UNSUPPORTED for XML config
options that are not available with the current hypervisor.
* src/qemu/qemu_conf.c, src/qemu/qemu_driver.c: Remove
many VIR_ERR_NO_SUPPORT replace with VIR_ERR_CONFIG_UNSUPPORTED
---
src/qemu/qemu_conf.c | 18 +++++++++---------
src/qemu/qemu_driver.c | 36 ++++++++++++++++++------------------
2 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 1d0bd88..4fee43e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1839,7 +1839,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
ret = virAsprintf(&devname, "xenblk%d", devid);
break;
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported disk name mapping for bus '%s'"),
virDomainDiskBusTypeToString(disk->bus));
return -1;
@@ -3390,7 +3390,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
goto cleanup;
if (!ncpus || !host) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("CPU specification not supported by hypervisor"));
goto cleanup;
}
@@ -3564,7 +3564,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (migrateFrom) {
if (STRPREFIX(migrateFrom, "tcp")) {
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("TCP migration is not supported with this QEMU binary"));
return -1;
}
@@ -3572,13 +3572,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
migrateFrom = "exec:cat";
} else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO)) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("STDIO migration is not supported with this QEMU binary"));
return -1;
}
} else if (STRPREFIX(migrateFrom, "exec")) {
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("STDIO migration is not supported with this QEMU binary"));
return -1;
}
@@ -4030,7 +4030,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* QEMU doesn't implement a SATA driver */
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("SATA is not supported with this QEMU binary"));
goto error;
}
@@ -4386,7 +4386,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
case VIR_DOMAIN_CHR_TARGET_TYPE_GUESTFWD:
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) ||
!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("guestfwd requires QEMU to support -chardev & -device"));
goto error;
}
@@ -4411,7 +4411,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO:
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtio channel requires QEMU to support -device"));
goto error;
}
@@ -4743,7 +4743,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
ADD_ARG(devstr);
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("PCI device assignment is not supported by this version of qemu"));
goto error;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 96277cd..21ae084 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -809,7 +809,7 @@ getVolumeQcowPassphrase(virConnectPtr conn,
enc = disk->encryption;
if (!conn) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot find secrets without a connection"));
goto cleanup;
}
@@ -7540,7 +7540,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn,
int vlan;
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_HOST_NET_ADD)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("installed qemu version does not support host_net_add"));
return -1;
}
@@ -7548,7 +7548,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn,
if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("network device type '%s' cannot be attached: "
"qemu is not using a unix socket monitor"),
virDomainNetTypeToString(net->type));
@@ -7559,7 +7559,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn,
return -1;
} else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("network device type '%s' cannot be attached: "
"qemu is not using a unix socket monitor"),
virDomainNetTypeToString(net->type));
@@ -7592,7 +7592,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn,
vlan = qemuDomainNetVLAN(net);
if (vlan < 0) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Unable to attach network devices without vlan"));
goto cleanup;
}
@@ -7928,7 +7928,7 @@ static int qemudDomainAttachHostDevice(struct qemud_driver *driver,
unsigned long long qemuCmdFlags)
{
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("hostdev mode '%s' not supported"),
virDomainHostdevModeTypeToString(hostdev->mode));
return -1;
@@ -7970,7 +7970,7 @@ static int qemudDomainAttachHostDevice(struct qemud_driver *driver,
break;
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("hostdev subsys type '%s' not supported"),
virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type));
goto error;
@@ -8066,7 +8066,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
if (ret == 0)
dev->data.disk = NULL;
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk bus '%s' cannot be hotplugged."),
virDomainDiskBusTypeToString(dev->data.disk->bus));
/* fallthrough */
@@ -8074,7 +8074,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
break;
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk device type '%s' cannot be hotplugged"),
virDomainDiskDeviceTypeToString(dev->data.disk->device));
/* Fallthrough */
@@ -8091,7 +8091,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
if (ret == 0)
dev->data.controller = NULL;
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk controller bus '%s' cannot be hotplugged."),
virDomainControllerTypeToString(dev->data.controller->type));
/* fallthrough */
@@ -8107,7 +8107,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
if (ret == 0)
dev->data.hostdev = NULL;
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("device type '%s' cannot be attached"),
virDomainDeviceTypeToString(dev->type));
goto endjob;
@@ -8296,7 +8296,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk bus '%s' cannot be updated."),
virDomainDiskBusTypeToString(dev->data.disk->bus));
break;
@@ -8314,7 +8314,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
break;
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk device type '%s' cannot be updated"),
virDomainDiskDeviceTypeToString(dev->data.disk->device));
break;
@@ -8892,7 +8892,7 @@ static int qemudDomainDetachHostDevice(struct qemud_driver *driver,
int ret;
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("hostdev mode '%s' not supported"),
virDomainHostdevModeTypeToString(hostdev->mode));
return -1;
@@ -8906,7 +8906,7 @@ static int qemudDomainDetachHostDevice(struct qemud_driver *driver,
ret = qemudDomainDetachHostUsbDevice(driver, vm, dev, qemuCmdFlags);
break;
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("hostdev subsys type '%s' not supported"),
virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type));
return -1;
@@ -8967,7 +8967,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
qemuCmdFlags);
}
else {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("This type of disk cannot be hot unplugged"));
}
} else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
@@ -8977,7 +8977,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
ret = qemudDomainDetachPciControllerDevice(driver, vm, dev,
qemuCmdFlags);
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk controller bus '%s' cannot be hotunplugged."),
virDomainControllerTypeToString(dev->data.controller->type));
/* fallthrough */
@@ -8985,7 +8985,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
ret = qemudDomainDetachHostDevice(driver, vm, dev, qemuCmdFlags);
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("This type of device cannot be hot unplugged"));
}
--
1.7.1.1
14 years, 4 months
[libvirt] [RFC][PATCH] Fix a deadlock in bi-directional p2p concurrent migration.
by Chris Lalancette
If you try to execute two concurrent migrations p2p
from A->B and B->A, the two libvirtd's will deadlock
trying to perform the migrations. The reason for this is
that in p2p migration, the libvirtd's are responsible for
making the RPC Prepare, Migrate, and Finish calls. However,
they are currently holding the driver lock while doing so,
which basically guarantees deadlock in this scenario.
This patch fixes the situation by adding
qemuDomainObjEnterRemoteWithDriver and
qemuDomainObjExitRemoteWithDriver helper methods. The Enter
take an additional object reference, then drops both the
domain object lock and the driver lock. The Exit takes
both the driver and domain object lock, then drops the
reference. Adding calls to these Enter and Exit helpers
around remote calls in the various migration methods
seems to fix the problem for me in testing.
I *believe* that this makes the situation safe, though
I'm not 100% certain. The additional domain object reference
ensures that the domain object won't disappear while this
operation is happening. The BeginJob that is called inside
of qemudDomainMigratePerform ensures that we can't execute a
second migrate (or shutdown, or save, etc) job while the
migration is active. That being said, my understanding of the
locking is still a bit shaky, so I'd like some additional
confirmation that this is indeed safe.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
src/qemu/qemu_driver.c | 39 ++++++++++++++++++++++++++++++++-------
1 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f2607c..e81af84 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -531,6 +531,21 @@ static void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virD
}
}
+static void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver,
+ virDomainObjPtr obj)
+{
+ virDomainObjRef(obj);
+ virDomainObjUnlock(obj);
+ qemuDriverUnlock(driver);
+}
+
+static void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver,
+ virDomainObjPtr obj)
+{
+ qemuDriverLock(driver);
+ virDomainObjLock(obj);
+ virDomainObjUnref(obj);
+}
static int qemuCgroupControllerActive(struct qemud_driver *driver,
int controller)
@@ -10753,9 +10768,11 @@ static int doTunnelMigrate(virDomainPtr dom,
/* virStreamNew only fails on OOM, and it reports the error itself */
goto cleanup;
+ qemuDomainObjEnterRemoteWithDriver(driver, vm);
internalret = dconn->driver->domainMigratePrepareTunnel(dconn, st,
flags, dname,
resource, dom_xml);
+ qemuDomainObjExitRemoteWithDriver(driver, vm);
if (internalret < 0)
/* domainMigratePrepareTunnel sets the error for us */
@@ -10833,8 +10850,10 @@ cancel:
finish:
dname = dname ? dname : dom->name;
+ qemuDomainObjEnterRemoteWithDriver(driver, vm);
ddomain = dconn->driver->domainMigrateFinish2
(dconn, dname, NULL, 0, uri, flags, retval);
+ qemuDomainObjExitRemoteWithDriver(driver, vm);
cleanup:
if (client_sock != -1)
@@ -10873,16 +10892,20 @@ static int doNonTunnelMigrate(virDomainPtr dom,
virDomainPtr ddomain = NULL;
int retval = -1;
char *uri_out = NULL;
+ int rc;
+ qemuDomainObjEnterRemoteWithDriver(driver, vm);
/* NB we don't pass 'uri' into this, since that's the libvirtd
* URI in this context - so we let dest pick it */
- if (dconn->driver->domainMigratePrepare2(dconn,
- NULL, /* cookie */
- 0, /* cookielen */
- NULL, /* uri */
- &uri_out,
- flags, dname,
- resource, dom_xml) < 0)
+ rc = dconn->driver->domainMigratePrepare2(dconn,
+ NULL, /* cookie */
+ 0, /* cookielen */
+ NULL, /* uri */
+ &uri_out,
+ flags, dname,
+ resource, dom_xml);
+ qemuDomainObjExitRemoteWithDriver(driver, vm);
+ if (rc < 0)
/* domainMigratePrepare2 sets the error for us */
goto cleanup;
@@ -10899,8 +10922,10 @@ static int doNonTunnelMigrate(virDomainPtr dom,
finish:
dname = dname ? dname : dom->name;
+ qemuDomainObjEnterRemoteWithDriver(driver, vm);
ddomain = dconn->driver->domainMigrateFinish2
(dconn, dname, NULL, 0, uri_out, flags, retval);
+ qemuDomainObjExitRemoteWithDriver(driver, vm);
if (ddomain)
virUnrefDomain(ddomain);
--
1.7.1.1
14 years, 4 months
[libvirt] [PATCH] CVE-2010-2242 Apply a source port mapping to virtual network masquerading
by Daniel P. Berrange
For
https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2010-2242
IPtables will seek to preserve the source port unchanged when
doing masquerading, if possible. NFS has a pseudo-security
option where it checks for the source port <= 1023 before
allowing a mount request. If an admin has used this to make the
host OS trusted for mounts, the default iptables behaviour will
potentially allow NAT'd guests access too. This needs to be
stopped.
With this change, the iptables -t nat -L -n -v rules for the
default network will be
Chain POSTROUTING (policy ACCEPT 95 packets, 9163 bytes)
pkts bytes target prot opt in out source destination
14 840 MASQUERADE tcp -- * * 192.168.122.0/24 !192.168.122.0/24 masq ports: 1024-65535
75 5752 MASQUERADE udp -- * * 192.168.122.0/24 !192.168.122.0/24 masq ports: 1024-65535
0 0 MASQUERADE all -- * * 192.168.122.0/24 !192.168.122.0/24
* src/network/bridge_driver.c: Add masquerade rules for TCP
and UDP protocols
* src/util/iptables.c, src/util/iptables.c: Add source port
mappings for TCP & UDP protocols when masquerading.
---
src/network/bridge_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++--
src/util/iptables.c | 70 +++++++++++++++++++++++++++++------------
src/util/iptables.h | 6 ++-
3 files changed, 122 insertions(+), 27 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 72255c1..80ed57a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -638,18 +638,74 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
goto masqerr2;
}
- /* enable masquerading */
+ /*
+ * Enable masquerading.
+ *
+ * We need to end up with 3 rules in the table in this order
+ *
+ * 1. protocol=tcp with sport mapping restricton
+ * 2. protocol=udp with sport mapping restricton
+ * 3. generic any protocol
+ *
+ * The sport mappings are required, because default IPtables
+ * MASQUERADE is maintain port number unchanged where possible.
+ *
+ * NFS can be configured to only "trust" port numbers < 1023.
+ *
+ * Guests using NAT thus need to be prevented from having port
+ * numbers < 1023, otherwise they can bypass the NFS "security"
+ * check on the source port number.
+ *
+ * Since we use '--insert' to add rules to the header of the
+ * chain, we actually need to add them in the reverse of the
+ * order just mentioned !
+ */
+
+ /* First the generic masquerade rule for other protocols */
if ((err = iptablesAddForwardMasquerade(driver->iptables,
network->def->network,
- network->def->forwardDev))) {
+ network->def->forwardDev,
+ NULL))) {
virReportSystemError(err,
_("failed to add iptables rule to enable masquerading to '%s'"),
network->def->forwardDev ? network->def->forwardDev : NULL);
goto masqerr3;
}
+ /* UDP with a source port restriction */
+ if ((err = iptablesAddForwardMasquerade(driver->iptables,
+ network->def->network,
+ network->def->forwardDev,
+ "udp"))) {
+ virReportSystemError(err,
+ _("failed to add iptables rule to enable UDP masquerading to '%s'"),
+ network->def->forwardDev ? network->def->forwardDev : NULL);
+ goto masqerr4;
+ }
+
+ /* TCP with a source port restriction */
+ if ((err = iptablesAddForwardMasquerade(driver->iptables,
+ network->def->network,
+ network->def->forwardDev,
+ "tcp"))) {
+ virReportSystemError(err,
+ _("failed to add iptables rule to enable TCP masquerading to '%s'"),
+ network->def->forwardDev ? network->def->forwardDev : NULL);
+ goto masqerr5;
+ }
+
return 1;
+ masqerr5:
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ network->def->network,
+ network->def->forwardDev,
+ "udp");
+ masqerr4:
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ network->def->network,
+ network->def->forwardDev,
+ NULL);
masqerr3:
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
network->def->network,
@@ -814,8 +870,17 @@ networkRemoveIptablesRules(struct network_driver *driver,
if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
iptablesRemoveForwardMasquerade(driver->iptables,
- network->def->network,
- network->def->forwardDev);
+ network->def->network,
+ network->def->forwardDev,
+ "tcp");
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ network->def->network,
+ network->def->forwardDev,
+ "udp");
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ network->def->network,
+ network->def->forwardDev,
+ NULL);
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
network->def->network,
network->def->bridge,
diff --git a/src/util/iptables.c b/src/util/iptables.c
index d06b857..f63e8c6 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -692,25 +692,49 @@ iptablesRemoveForwardRejectIn(iptablesContext *ctx,
*/
static int
iptablesForwardMasquerade(iptablesContext *ctx,
- const char *network,
- const char *physdev,
- int action)
+ const char *network,
+ const char *physdev,
+ const char *protocol,
+ int action)
{
- if (physdev && physdev[0]) {
- return iptablesAddRemoveRule(ctx->nat_postrouting,
- action,
- "--source", network,
- "!", "--destination", network,
- "--out-interface", physdev,
- "--jump", "MASQUERADE",
- NULL);
+ if (protocol && protocol[0]) {
+ if (physdev && physdev[0]) {
+ return iptablesAddRemoveRule(ctx->nat_postrouting,
+ action,
+ "--source", network,
+ "-p", protocol,
+ "!", "--destination", network,
+ "--out-interface", physdev,
+ "--jump", "MASQUERADE",
+ "--to-ports", "1024-65535",
+ NULL);
+ } else {
+ return iptablesAddRemoveRule(ctx->nat_postrouting,
+ action,
+ "--source", network,
+ "-p", protocol,
+ "!", "--destination", network,
+ "--jump", "MASQUERADE",
+ "--to-ports", "1024-65535",
+ NULL);
+ }
} else {
- return iptablesAddRemoveRule(ctx->nat_postrouting,
- action,
- "--source", network,
- "!", "--destination", network,
- "--jump", "MASQUERADE",
- NULL);
+ if (physdev && physdev[0]) {
+ return iptablesAddRemoveRule(ctx->nat_postrouting,
+ action,
+ "--source", network,
+ "!", "--destination", network,
+ "--out-interface", physdev,
+ "--jump", "MASQUERADE",
+ NULL);
+ } else {
+ return iptablesAddRemoveRule(ctx->nat_postrouting,
+ action,
+ "--source", network,
+ "!", "--destination", network,
+ "--jump", "MASQUERADE",
+ NULL);
+ }
}
}
@@ -719,6 +743,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
* @ctx: pointer to the IP table context
* @network: the source network name
* @physdev: the physical input device or NULL
+ * @protocol: the network protocol or NULL
*
* Add rules to the IP table context to allow masquerading
* network @network on @physdev. This allow the bridge to
@@ -729,9 +754,10 @@ iptablesForwardMasquerade(iptablesContext *ctx,
int
iptablesAddForwardMasquerade(iptablesContext *ctx,
const char *network,
- const char *physdev)
+ const char *physdev,
+ const char *protocol)
{
- return iptablesForwardMasquerade(ctx, network, physdev, ADD);
+ return iptablesForwardMasquerade(ctx, network, physdev, protocol, ADD);
}
/**
@@ -739,6 +765,7 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
* @ctx: pointer to the IP table context
* @network: the source network name
* @physdev: the physical input device or NULL
+ * @protocol: the network protocol or NULL
*
* Remove rules from the IP table context to stop masquerading
* network @network on @physdev. This stops the bridge from
@@ -749,7 +776,8 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
int
iptablesRemoveForwardMasquerade(iptablesContext *ctx,
const char *network,
- const char *physdev)
+ const char *physdev,
+ const char *protocol)
{
- return iptablesForwardMasquerade(ctx, network, physdev, REMOVE);
+ return iptablesForwardMasquerade(ctx, network, physdev, protocol, REMOVE);
}
diff --git a/src/util/iptables.h b/src/util/iptables.h
index 7d55a6d..b47d854 100644
--- a/src/util/iptables.h
+++ b/src/util/iptables.h
@@ -85,9 +85,11 @@ int iptablesRemoveForwardRejectIn (iptablesContext *ctx,
int iptablesAddForwardMasquerade (iptablesContext *ctx,
const char *network,
- const char *physdev);
+ const char *physdev,
+ const char *protocol);
int iptablesRemoveForwardMasquerade (iptablesContext *ctx,
const char *network,
- const char *physdev);
+ const char *physdev,
+ const char *protocol);
#endif /* __QEMUD_IPTABLES_H__ */
--
1.6.6.1
14 years, 4 months
[libvirt] [PATCH] daemon: dispatch.c should include stdio.h (and stdarg.h)
by Ryota Ozaki
dispatch.c requires stdio.h (and stdarg.h), however, currently
dispatch.c implicitly relys on rpc/xdr.h to include stdio.h.
If rpc/xdr.h unxpectedly does not include stdio.h, the compilation
of dispatch.c fails.
This can happen, for example, when portablexdr is installed
under /usr/local; because portablexdr's rpc/xdr.h does not
include stdio.h and gcc looks up it not /usr/include/rpc/xdr.h.
Note that stdarg.h is also included according to man va_start,
although stdio.h seems including it anyway.
---
daemon/dispatch.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/daemon/dispatch.c b/daemon/dispatch.c
index 8f55eaa..966db71 100644
--- a/daemon/dispatch.c
+++ b/daemon/dispatch.c
@@ -23,6 +23,9 @@
#include <config.h>
+#include <stdio.h>
+#include <stdarg.h>
+
#include "dispatch.h"
#include "remote.h"
--
1.6.6.1
14 years, 4 months
[libvirt] [PATCH] uml_driver: correct logic error in umlMonitorCommand
by Jim Meyering
The stray "< 0" removed by the fix below
would have made umlMonitorCommand always fail
with its "incomplete reply ..." diagnostic.
>From 5f075332e33f7e9bf2d7979934b8dc7c3312b6e8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Tue, 13 Jul 2010 15:28:35 -0500
Subject: [PATCH] uml_driver: correct logic error in umlMonitorCommand
* src/uml/uml_driver.c (umlMonitorCommand): Correct flaw that would
cause unconditional "incomplete reply ..." failure, since "nbytes"
was always 0 or 1.
---
src/uml/uml_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 110179e..1e0f5ac 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -730,7 +730,7 @@ static int umlMonitorCommand(const struct uml_driver *driver,
ssize_t nbytes;
addrlen = sizeof(addr);
nbytes = recvfrom(priv->monitor, &res, sizeof res, 0,
- (struct sockaddr *)&addr, &addrlen) < 0;
+ (struct sockaddr *)&addr, &addrlen);
if (nbytes < 0) {
if (errno == EAGAIN || errno == EINTR)
continue;
--
1.7.1.460.gf3c4c
14 years, 4 months
[libvirt] [PATCH] qemuConnectMonitor: do not mask SELinux failure
by Jim Meyering
coverity spotted the expressions that could never be true:
(a && b && c) < 0. Here's the fix:
>From 6d61d90a6df81900f4b2be752403758175000973 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Tue, 13 Jul 2010 15:15:04 -0500
Subject: [PATCH] qemuConnectMonitor: fix a bug that would have masked SELinux failure
* src/qemu/qemu_driver.c (qemuConnectMonitor): Correct erroneous
parenthesization in two expressions. Without this fix, failure
to set or clear SELinux security context in the monitor would go
undiagnosed. Also correct a diagnostic and split some long lines.
---
src/qemu/qemu_driver.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 487bfa3..96277cd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1194,10 +1194,12 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm)
qemuDomainObjPrivatePtr priv = vm->privateData;
int ret = -1;
- if ((driver->securityDriver &&
- driver->securityDriver->domainSetSecuritySocketLabel &&
- driver->securityDriver->domainSetSecuritySocketLabel(driver->securityDriver,vm)) < 0) {
- VIR_ERROR(_("Failed to set security context for monitor for %s"), vm->def->name);
+ if (driver->securityDriver &&
+ driver->securityDriver->domainSetSecuritySocketLabel &&
+ driver->securityDriver->domainSetSecuritySocketLabel
+ (driver->securityDriver,vm) < 0) {
+ VIR_ERROR(_("Failed to set security context for monitor for %s"),
+ vm->def->name);
goto error;
}
@@ -1213,10 +1215,12 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm)
if (priv->mon == NULL)
virDomainObjUnref(vm);
- if ((driver->securityDriver &&
- driver->securityDriver->domainClearSecuritySocketLabel &&
- driver->securityDriver->domainClearSecuritySocketLabel(driver->securityDriver,vm)) < 0) {
- VIR_ERROR(_("Failed to set security context for monitor for %s"), vm->def->name);
+ if (driver->securityDriver &&
+ driver->securityDriver->domainClearSecuritySocketLabel &&
+ driver->securityDriver->domainClearSecuritySocketLabel
+ (driver->securityDriver,vm) < 0) {
+ VIR_ERROR(_("Failed to clear security context for monitor for %s"),
+ vm->def->name);
goto error;
}
--
1.7.1.460.gf3c4c
14 years, 4 months
[libvirt] [PATCH] python: Fix IOErrorReasonCallback bindings
by Cole Robinson
A copy and paste error was causing us to dispatch the incorrect
routine. Spotted by Dan Kenigsberg.
---
python/libvirt-override.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index ad55940..54a84c2 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -3258,7 +3258,7 @@ libvirt_virConnectDomainEventIOErrorReasonCallback(virConnectPtr conn ATTRIBUTE_
/* Call the Callback Dispatcher */
pyobj_ret = PyObject_CallMethod(pyobj_conn,
- (char*)"dispatchDomainEventIOErrorCallback",
+ (char*)"dispatchDomainEventIOErrorReasonCallback",
(char*)"OssisO",
pyobj_dom,
srcPath, devAlias, action, reason,
--
1.6.6.1
14 years, 4 months
