- Devel - libvirt List Archives

[libvirt] [PATCH v3] nwfilter: probe for inverted ctdir
by Stefan Berger 29 Mar '13

29 Mar '13

Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT two times, one time with '--ctdir original' which should then work on 'fixed' netfilter and one other time with '--ctdir reply' which should only work on the 'old' netfilter. If neither one of the tests gets the data through, then the loopback device is probably not configured correctly. If both tests get the data through something must be seriously wrong. In both of these two latter cases no '--ctdir' will then be applied to the rules. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- v2->v3: - probing with --ctdir original and --ctdir reply v1->v2: - using virSocketAddrParseIPv4 --- src/nwfilter/nwfilter_ebiptables_driver.c | 169 ++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -27,6 +27,10 @@ #include <string.h> #include <sys/stat.h> #include <fcntl.h> +#include <arpa/inet.h> +#include <sys/select.h> +#include <sys/time.h> +#include <unistd.h> #include "internal.h" @@ -85,6 +89,17 @@ static char *iptables_cmd_path; static char *ip6tables_cmd_path; static char *grep_cmd_path; +/* + * --ctdir original vs. --ctdir reply's meaning was inverted in netfilter + * at some point. We probe for it. + */ +enum ctdirStatus { + CTDIR_STATUS_UNKNOWN = 0, + CTDIR_STATUS_CORRECTED = (1 << 0), + CTDIR_STATUS_OLD = (1 << 1), +}; +static enum ctdirStatus iptables_ctdir_corrected; + #define PRINT_ROOT_CHAIN(buf, prefix, ifname) \ snprintf(buf, sizeof(buf), "libvirt-%c-%s", prefix, ifname) #define PRINT_CHAIN(buf, prefix, ifname, suffix) \ @@ -1262,6 +1277,17 @@ iptablesEnforceDirection(int directionIn virNWFilterRuleDefPtr rule, virBufferPtr buf) { + switch (iptables_ctdir_corrected) { + case CTDIR_STATUS_UNKNOWN: + /* could not be determined or s.th. is seriously wrong */ + return; + case CTDIR_STATUS_CORRECTED: + directionIn = !directionIn; + break; + case CTDIR_STATUS_OLD: + break; + } + if (rule->tt != VIR_NWFILTER_RULE_DIRECTION_INOUT) virBufferAsprintf(buf, " -m conntrack --ctdir %s", (directionIn) ? "Original" @@ -4304,6 +4330,146 @@ ebiptablesDriverTestCLITools(void) return ret; } +static void +ebiptablesDriverProbeCtdir(void) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + static const char cmdline[] = + "$IPT -%c INPUT %c -i lo -p udp --dport %hu " + "-m state --state ESTABLISHED -j ACCEPT " CMD_SEPARATOR + "$IPT -%c INPUT %c -i lo -p udp --dport %hu " + "-m conntrack --ctdir %s -j ACCEPT " CMD_SEPARATOR + "$IPT -%c INPUT %c -i lo -p udp --dport %hu -j DROP"; + /* + * Above '--ctdir original' gets this test to receive a message on + * 'fixed' netfilter. + */ + unsigned short port; + int ssockfd = -1, csockfd = -1; + virSocketAddr saddr; + struct sockaddr_in *serveraddr = &saddr.data.inet4; + fd_set readfds; + struct timeval timeout = { + .tv_sec = 0, + .tv_usec = 1000 * 200, + }; + int n, i, results = 0; + const char *ctdiropts[2] = { "original", "reply" }; + unsigned char data[10]; + + if (virSocketAddrParseIPv4(&saddr, "127.0.0.1") < 0) { + VIR_ERROR(_("Could not parse IP address")); + goto cleanup; + } + + if ((ssockfd = socket(AF_INET, SOCK_DGRAM, 0)) < 0 || + (csockfd = socket(AF_INET, SOCK_DGRAM, 0)) < 0) { + VIR_ERROR(_("Could not open UDP socket")); + goto cleanup; + } + + for (port = 0xffff; port > 1024; port--) { + serveraddr->sin_port = htons(port); + if (bind(ssockfd, (struct sockaddr *)serveraddr, + sizeof(*serveraddr)) == 0) + break; + } + if (port == 1024) { + VIR_ERROR(_("Could not bind to any UDP socket")); + goto cleanup; + } + + i = 0; + while (true) { + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + virBufferAsprintf(&buf, cmdline, + 'I', '1', port, + 'I', '2', port, ctdiropts[i], + 'I', '3', port); + + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) { + VIR_ERROR(_("Could not apply iptables rules")); + goto cleanup_iptables; + } + + virBufferFreeAndReset(&buf); + + if (sendto(csockfd, cmdline, 1, 0, (struct sockaddr *)serveraddr, + sizeof(*serveraddr)) < 0) { + VIR_ERROR(_("Could not send to UDP socket")); + goto cleanup_iptables; + } + + FD_ZERO(&readfds); + FD_SET(ssockfd, &readfds); + + while (true) { + n = select(ssockfd + 1, &readfds, NULL, NULL, &timeout); + if (n < 0) { + if (errno == EINTR) + continue; + VIR_ERROR(_("Select failed")); + goto cleanup_iptables; + } + if (n == 0) { + VIR_INFO("Ctdir probing received no data"); + break; + } + VIR_INFO("Ctdir probing received data"); + results |= (1 << i); + read(ssockfd, data, sizeof(data)); + break; + } + + if (i + 1 == ARRAY_CARDINALITY(ctdiropts)) + break; + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + virBufferAsprintf(&buf, cmdline, + 'D', ' ', port, + 'D', ' ', port, ctdiropts[i], + 'D', ' ', port); + ebiptablesExecCLI(&buf, NULL, NULL); + + i++; + } + + switch (results) { + case 0x0: + /* no test passed -- loopback device not setup? */ + case 0x3: + /* both test passed -- s.th. is wrong */ + iptables_ctdir_corrected = CTDIR_STATUS_UNKNOWN; + break; + case 0x1: + iptables_ctdir_corrected = CTDIR_STATUS_CORRECTED; + break; + case 0x2: + iptables_ctdir_corrected = CTDIR_STATUS_OLD; + break; + } + +cleanup_iptables: + virBufferFreeAndReset(&buf); + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + virBufferAsprintf(&buf, cmdline, + 'D', ' ', port, + 'D', ' ', port, ctdiropts[i], + 'D', ' ', port); + ebiptablesExecCLI(&buf, NULL, NULL); + +cleanup: + virBufferFreeAndReset(&buf); + VIR_FORCE_CLOSE(ssockfd); + VIR_FORCE_CLOSE(csockfd); +} + static int ebiptablesDriverInit(bool privileged) { @@ -4341,6 +4507,9 @@ ebiptablesDriverInit(bool privileged) return -ENOTSUP; } + if (iptables_cmd_path) + ebiptablesDriverProbeCtdir(); + ebiptables_driver.flags = TECHDRV_FLAG_INITIALIZED; return 0;

4 12

[libvirt] [PATCH] security_manager.c: Append seclabel iff generated
by Michal Privoznik 29 Mar '13

29 Mar '13

With my previous patches, we unconditionally appended a seclabel, even if it wasn't generated but found in array of defined seclabels. This resulted in double free later when doing virDomainDefFree and iterating over the array of defined seclabels. Moreover, there was another possibility of double free, if the seclabel was generated in the last iteration of the process of walking trough security managers array. --- src/security/security_manager.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index b55af69..b671a91 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -463,6 +463,7 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, } else if (vm->nseclabels && generated) { VIR_DEBUG("Skipping auto generated seclabel of type none"); virSecurityLabelDefFree(seclabel); + seclabel = NULL; continue; } } @@ -472,8 +473,8 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, } else { /* The seclabel must be added to @vm prior calling domainGenSecurityLabel * which may require seclabel to be presented already */ - - if (VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel) < 0) { + if (generated && + VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel) < 0) { virReportOOMError(); goto cleanup; } @@ -484,6 +485,8 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, vm->nseclabels--; goto cleanup; } + + seclabel = NULL; } } -- 1.8.1.5

2 1

[libvirt] [PATCH] util: Fix the conflict type for virIsCapableFCHost
by Osier Yang 28 Mar '13

28 Mar '13

--- Pushed under build-breaker rule. --- src/util/virutil.c | 3 ++- src/util/virutil.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 557225c..d5f122f 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -3582,7 +3582,8 @@ virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, } int -virIsCapableFCHost(int host ATTRIBUTE_UNUSED) +virIsCapableFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); return -1; diff --git a/src/util/virutil.h b/src/util/virutil.h index 47357fa..2a797cb 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -314,7 +314,7 @@ enum { int virManageVport(const int parent_host, const char *wwpn, const char *wwnn, - int operation) + int operation) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); #endif /* __VIR_UTIL_H__ */ -- 1.8.1.4

1 0

[libvirt] [PATCH] virutil: Fix compilation on non-linux platforms
by Michal Privoznik 28 Mar '13

28 Mar '13

There has been a typo in virIsCapbleVport function name. --- Pushed under build breaker rule. src/util/virutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index d5f122f..87a97c9 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -3590,8 +3590,8 @@ virIsCapableFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, } int -virIsCapbleVport(const char *sysfs_prefix ATTRIBUTE_UNUSED, - int host ATTRIBUTE_UNUSED) +virIsCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); return -1; -- 1.8.1.5

1 0

[libvirt] [PATCH v2 0/2] Correctly treat seclabel of type none
by Michal Privoznik 28 Mar '13

28 Mar '13

Don't forget other seclabels when adding a <seclabel type='none'/>. Michal Privoznik (2): security_manager: Don't manipulate domain XML in virDomainDefGetSecurityLabelDef security: Don't add seclabel of type none if there's already a seclabel src/conf/domain_conf.c | 56 +++++++++++------------------------------ src/conf/domain_conf.h | 7 ++++-- src/libvirt_private.syms | 1 - src/security/security_manager.c | 55 +++++++++++++++++++++++++++------------- src/security/security_selinux.c | 8 ++++-- 5 files changed, 63 insertions(+), 64 deletions(-) -- 1.8.1.5

3 9

[libvirt] [libvirt-sandbox][PATCH] Sync lxc-enter-namespace options with libvirt
by Alex Jia 28 Mar '13

28 Mar '13

The option 'nolabel' has been changed by current libvirt upstream, it should be 'noseclabel' now, so need to sync the option change with libvirt. # ./tools/virsh -c lxc:/// lxc-enter-namespace --help | grep label lxc-enter-namespace <domain> [--noseclabel] {[--cmd] <string>}... --noseclabel Do not change process security label Signed-off-by: Alex Jia <ajia(a)redhat.com> --- bin/virt-sandbox-service | 6 +++--- bin/virt-sandbox-service-bash-completion.sh | 2 +- bin/virt-sandbox-service-execute.pod | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index c05e13f..478769d 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -838,8 +838,8 @@ def fullpath(cmd): def execute(args): myexec = [ "virsh", "-c", "lxc:///", "lxc-enter-namespace" ] # myexec = [ "virt-sandbox-service-util", "execute" ] - if args.nolabel: - myexec.append("--nolabel") + if args.noseclabel: + myexec.append("--noseclabel") myexec.extend([ args.name, "--", fullpath(args.command[0])] + args.command[1:]) os.execv("/usr/bin/virsh", myexec) # myexec.extend( "-e", cmd, args.name ] @@ -966,7 +966,7 @@ def gen_connect_args(subparser): def gen_execute_args(subparser): parser = subparser.add_parser("execute", help=("Execute a command within a sandbox container")) - parser.add_argument("-N", "--nolabel", dest="nolabel", + parser.add_argument("-N", "--noseclabel", dest="noseclabel", default=False, action="store_true", help=_("do not modify the label of the executable process. By default all commands execute with the label of the sandbox")) requires_name(parser) diff --git a/bin/virt-sandbox-service-bash-completion.sh b/bin/virt-sandbox-service-bash-completion.sh index a886cf4..c855fd2 100755 --- a/bin/virt-sandbox-service-bash-completion.sh +++ b/bin/virt-sandbox-service-bash-completion.sh @@ -57,7 +57,7 @@ _virt_sandbox_service () { [ALL]='-h --help' [CREATE]='-u --unitfile -p --path -t --type -l --level -d --dynamic -n --clone -i --image -s --size' [LIST]='-r --running' - [EXECUTE]='-N --nolabel' + [EXECUTE]='-N --noseclabel' ) for ((i=0; $i <= $COMP_CWORD; i++)); do diff --git a/bin/virt-sandbox-service-execute.pod b/bin/virt-sandbox-service-execute.pod index 46a3b2b..f79ce73 100644 --- a/bin/virt-sandbox-service-execute.pod +++ b/bin/virt-sandbox-service-execute.pod @@ -31,7 +31,7 @@ Display help message The connection URI for the hypervisor (only LXC or QEMU are supported currently). -=item B<-N>, B<--nolabel> +=item B<-N>, B<--noseclabel> Execute command within the container. -- 1.7.1

2 2

[libvirt] [PATCH] net: use newer iptables syntax
by Stefan Seyfried 28 Mar '13

28 Mar '13

Hi all, iptables-1.4.18 removed the long deprecated "state" match. Use "conntrack" instead in forwarding rules. Fixes openSUSE bug https://bugzilla.novell.com/811251 #811251. real patch is attached as I'm pretty sure that thunderbird will mess it up otherwise :( Basically it's s/--match state/--match conntrack/ s/--state /--ctstate/ in src/til/viriptables.c Best regards, Stefan -- Stefan Seyfried Linux Consultant & Developer Mail: seyfried(a)b1-systems.de GPG Key: 0x731B665B B1 Systems GmbH Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537

5 4

[libvirt] [PATCH] conf: fix memory leak of class_id bitmap
by Guannan Ren 28 Mar '13

28 Mar '13

When libvirtd loads active network configs from network state directory, it should release the class_id memory block which was allocated at the time of loading xml from network config directory. virBitmapParse will create a new memory block of bitmap class_id which causes a memory leak. This happens when at least one virtual network is active before. ==12234== 8,216 (24 direct, 8,192 indirect) bytes in 1 blocks are definitely \ lost in loss record 702 of 709 ==12234== at 0x4A06B2F: calloc (vg_replace_malloc.c:593) ==12234== by 0x37AB04D77D: virAlloc (in /usr/lib64/libvirt.so.0.1000.3) ==12234== by 0x37AB04EF89: virBitmapNew (in /usr/lib64/libvirt.so.0.1000.3) ==12234== by 0x37AB0BFB37: virNetworkAssignDef (in /usr/lib64/libvirt.so.0.1000.3) ==12234== by 0x37AB0BFD31: ??? (in /usr/lib64/libvirt.so.0.1000.3) ==12234== by 0x37AB0BFE92: virNetworkLoadAllConfigs (in /usr/lib64/libvirt.so.0.1000.3) ==12234== by 0x10650E5A: ??? (in /usr/lib64/libvirt/connection-driver/libvirt_driver_network.so) ==12234== by 0x37AB0EB72F: virStateInitialize (in /usr/lib64/libvirt.so.0.1000.3) ==12234== by 0x40DE04: ??? (in /usr/sbin/libvirtd) ==12234== by 0x37AB0832E8: ??? (in /usr/lib64/libvirt.so.0.1000.3) ==12234== by 0x3796807D14: start_thread (in /usr/lib64/libpthread-2.16.so) ==12234== by 0x37960F246C: clone (in /usr/lib64/libc-2.16.so) --- src/conf/network_conf.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index c022fe4..7a45414 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1995,14 +1995,16 @@ virNetworkObjUpdateParseFile(const char *filename, ctxt->node = node; class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt); - if (class_id && - virBitmapParse(class_id, 0, + if (class_id) { + virBitmapFree(net->class_id); + if (virBitmapParse(class_id, 0, &net->class_id, CLASS_ID_BITMAP_SIZE) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Malformed 'class_id' attribute: %s"), - class_id); - VIR_FREE(class_id); - goto cleanup; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed 'class_id' attribute: %s"), + class_id); + VIR_FREE(class_id); + goto cleanup; + } } VIR_FREE(class_id); -- 1.7.11.2

2 2

[libvirt] [PATCH 1/2] qemu:release qemu config object when qemu driver shutdown
by Guannan Ren 28 Mar '13

28 Mar '13

--- src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1f9b8b2..96bf235 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -950,6 +950,7 @@ qemuShutdown(void) { return -1; virNWFilterUnRegisterCallbackDriver(&qemuCallbackDriver); + virObjectUnref(qemu_driver->config); virObjectUnref(qemu_driver->activePciHostdevs); virObjectUnref(qemu_driver->inactivePciHostdevs); virObjectUnref(qemu_driver->activeUsbHostdevs); -- 1.7.11.2

2 2

[libvirt] [PATCH 2/2] uml:release config object when uml driver shutdown
by Guannan Ren 28 Mar '13

28 Mar '13

--- src/uml/uml_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index b08212d..09a777c 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -640,6 +640,7 @@ umlShutdown(void) { virEventRemoveHandle(uml_driver->inotifyWatch); VIR_FORCE_CLOSE(uml_driver->inotifyFD); virObjectUnref(uml_driver->caps); + virObjectUnref(uml_driver->xmlconf); /* shutdown active VMs * XXX allow them to stay around & reconnect */ -- 1.7.11.2

2 2