- Devel - libvirt List Archives

[Libvir] Fix buffer overflow in dumping XML
by Daniel P. Berrange 22 Mar '07

22 Mar '07

The new bufferContentAndFree() method used for the QEMU daemon rellocs the buffer size down to release memory held by the buffer which was never used for any data. Unfortunately it reallocs it 1 byte too small, so later uses of strlen()/strcpy() either magically work, or randomly append gargage or crash the daemon depending on the phase of the moon :-) Re-allocing the buffer to relase a few bytes memory isn't really an optimization since the caller is going to free the entire block a very short while later, so this patch simply removes the realloc call. As an aside, the virBuffer functions in src/xml.c and the buffer functions in qemud/buf.c are both flawed wrt to the way they call the Grow method. The method expects the len parameter to be extra bytes needed, but several of the callers pass in the total desired length, so it allocates too much memory. There are various other non-fatal flaws which need to be cleaned up in this code, but the attached patch just focuses on the current fatal buffer overflow for now. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

3 2

[Libvir] [PATCH] virsh vcpupin guards CPU not existing.
by Masayuki Sunou 22 Mar '07

22 Mar '07

Hi When CPU not existing is set to virsh vcpupin, CPU affinity is set to all CPU. -------------------------------------------------------------------------------- # virsh vcpuinfo 0 VCPU: 0 CPU: 0 State: blocked CPU time: 428.8s CPU Affinity: y- # virsh vcpupin 0 0 100 libvir: Xen error : failed Xen syscall ioctl -1208339792 # virsh vcpuinfo 0 VCPU: 0 CPU: 0 State: blocked CPU time: 429.6s CPU Affinity: yy -------------------------------------------------------------------------------- This patch returns an error, when CPU not existing is set to virsh vcpupin. -------------------------------------------------------------------------------- # virsh vcpupin 0 0 100 error: Physical CPU 100 doesn't exist. -------------------------------------------------------------------------------- Signed-off-by: Masayuki Sunou <fj1826dm(a)aa.jp.fujitsu.com> Thanks, Masayuki Sunou. -------------------------------------------------------------------------------- Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.67 diff -u -p -r1.67 virsh.c --- src/virsh.c 20 Mar 2007 15:31:46 -0000 1.67 +++ src/virsh.c 22 Mar 2007 06:05:14 -0000 @@ -1358,6 +1358,11 @@ cmdVcpupin(vshControl * ctl, vshCmd * cm if (cpu < VIR_NODEINFO_MAXCPUS(nodeinfo)) { VIR_USE_CPU(cpumap, cpu); + } else { + vshError(ctl, FALSE, _("Physical CPU %d doesn't exist."), cpu); + free(cpumap); + virDomainFree(dom); + return FALSE; } cpulist = index(cpulist, ','); if (cpulist) --------------------------------------------------------------------------------

2 1

[Libvir] [PATCH] Add Xen Interface Version Check
by Atsushi SAKAI 22 Mar '07

22 Mar '07

Add Xen Interface version Check XEN_SYSCTL_INTERFACE_VERSION XEN_DOMCTL_INTERFACE_VERSION Currently following distribution is considered. sys dom RHEL5.0 2 3 Fedora 7 2 4 Xen-unstable 3 5 Please check virVcpuInfoPtr ipt; error message is collect or not + fprintf(stderr, "Memory allocation failed at xenHypervisorIniti()\n"); Signed-off-by: Atsushi SAKAI <sakaia(a)jp.fujitsu.com> Thanks Atsushi SAKAI --- libvirt-0.2.1/src/xen_internal.c 2007-03-16 06:36:41.000000000 +0900 +++ libvirt-0.2.1.hyper/src/xen_internal.c 2007-03-19 19:09:08.000000000 +0900 @@ -655,7 +655,6 @@ return (0); } -#ifndef PROXY /** * xenHypervisorDoV2Dom: * @handle: the handle to the Xen hypervisor @@ -697,7 +696,6 @@ return (0); } -#endif /* PROXY */ /** * virXen_getdomaininfolist: @@ -1057,6 +1055,8 @@ } return(ret); } +#endif /* !PROXY*/ + /** * virXen_getvcpusinfo: * @handle: the hypervisor handle @@ -1169,7 +1169,6 @@ } return(ret); } -#endif /* !PROXY*/ /** * xenHypervisorInit: @@ -1184,6 +1183,7 @@ hypercall_t hc; v0_hypercall_t v0_hc; xen_getdomaininfo info; + virVcpuInfoPtr ipt; if (initialized) { if (hypervisor_version == -1) @@ -1291,15 +1291,47 @@ * or the old ones */ hypervisor_version = 2; - /* TODO: one probably will need to autodetect thse subversions too */ + + ipt = malloc(sizeof(virVcpuInfo)); + if (ipt == NULL){ +#ifdef DEBUG + fprintf(stderr, "Memory allocation failed at xenHypervisorIniti()\n"); +#endif + return(-1); + } + /* Currently consider RHEL5.0 Fedora7 and xen-unstable */ sys_interface_version = 2; /* XEN_SYSCTL_INTERFACE_VERSION */ - dom_interface_version = 3; /* XEN_DOMCTL_INTERFACE_VERSION */ if (virXen_getdomaininfo(fd, 0, &info) == 1) { + /* RHEL 5.0 */ + dom_interface_version = 3; /* XEN_DOMCTL_INTERFACE_VERSION */ + if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0){ #ifdef DEBUG - fprintf(stderr, "Using hypervisor call v2, sys version 2\n"); + fprintf(stderr, "Using hypervisor call v2, sys ver2 dom ver3\n"); #endif - goto done; + goto done; + } + /* Fedora 7 */ + dom_interface_version = 4; /* XEN_DOMCTL_INTERFACE_VERSION */ + if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0){ +#ifdef DEBUG + fprintf(stderr, "Using hypervisor call v2, sys ver2 dom ver4\n"); +#endif + goto done; + } } + + sys_interface_version = 3; /* XEN_SYSCTL_INTERFACE_VERSION */ + if (virXen_getdomaininfo(fd, 0, &info) == 1) { + /* xen-unstable */ + dom_interface_version = 5; /* XEN_DOMCTL_INTERFACE_VERSION */ + if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0){ +#ifdef DEBUG + fprintf(stderr, "Using hypervisor call v2, sys ver3 dom ver5\n"); +#endif + goto done; + } + } + hypervisor_version = 1; sys_interface_version = -1; if (virXen_getdomaininfo(fd, 0, &info) == 1) {

2 4

[Libvir] QEMU networking XML docs summary
by Daniel P. Berrange 21 Mar '07

21 Mar '07

What follows is a summary of the ways in which you can setup QEMU networking 1. Userspace SLIRP stack Provides a virtual LAN with NAT to the outside world. The virtual network has DHCP & DNS services and will give the guest VM addresses starting from 10.0.2.15. The default router will be 10.0.2.2 and the DNS server will be 10.0.2.3. This networking is the only option for unprivileged users who need their VMS to have outgoing access. Example configs are: <interface type='user'/> <interface type='user'> <mac address="11:22:33:44:55:66:/> </interface> 2. Virtual network Provides a virtual network using a bridge device in the host. Depending on the virtual network configuration, the network may be totally isolated, NAT'ing to aan explicit network device, or NAT'ing to the default route. DHCP and DNS are provided on the virtual network in all cases and the IP range can be determined by examining the virtual network config with 'virsh net-dumpxml <network name>'. There is one virtual network called 'default' setup out of the box which does NAT'ing to the default route and has an IP range of 192.168.22.0/255.255.255.0. Each guest will have an associated tun device created with a name of vnetN, which can also be overriden with the <target> element. Example configs are <interface type='network'> <source network='default'/> </interface> <interface type='network'> <source network='default'/> <target dev='vnet7'/> <mac address="11:22:33:44:55:66:/> </interface> 3. Bridge to to LAN Provides a bridge from the VM directly onto the LAN. This assumes there is a bridge device on the host which has one or more of the hosts physical NICs enslaved. The guest VM will have an associated tun device created with a name of vnetN, which can also be overriden with the <target> element. The tun device will be enslaved to the bridge. The IP range / network configuration is whatever is used on the LAN. This provides the guest VM full incoming & outgoing net access just like a physical machine. Examples include <interface type='bridge'> <source dev='br0'/> </interface> <interface type='bridge'> <source dev='br0'/> <target dev='vnet7'/> <mac address="11:22:33:44:55:66:/> </interface> 4. Generic connection to LAN Provides a means for the administrator to execute an arbitrary script to connect the guest's network to the LAN. The guest will have a tun device created with a name of vnetN, which can also be overriden with the <target> element. After creating the tun device a shell script will be run which is expected to do whatever host network integration is required. By default this script is called /etc/qemu-ifup but can be overriden. <interface type='ethernet'/> <interface type='ethernet'> <target dev='vnet7'/> <script path='/etc/qemu-ifup-mynet'/> </interface> 5. Multicast tunnel A multicast group is setup to represent a virtual network. Any VMs whose network devices are in the same multicast group can talk to each other even across hosts. This mode is also available to unprivileged users. There is no default DNS or DHCP support and no outgoing network access. To provide outgoing network access, one of the VMs should have a 2nd NIC which is connected to one of the first 4 network types and do the appropriate routing. The multicast protocol is compatible with that used by user mode linux guests too. The source address used must be from the multicast address block <interface type='mcast'> <source address='230.0.0.1' port='5558'/> </interface> 6. TCP tunnel A TCP client/server architecture provides a virtual network. One VM provides the server end of the netowrk, all other VMS are configured as clients. All network traffic is routed between the VMs via the server. This mode is also available to unprivileged users. There is no default DNS or DHCP support and no outgoing network access. To provide outgoing network access, one of the VMs should have a 2nd NIC which is connected to one of the first 4 network types and do the appropriate routing. Example server config <interface type='server'> <source address='192.168.0.1' port='5558'/> </interface> Example client config <interface type='client'> <source address='192.168.0.1' port='5558'/> </interface> NB, options 2 -> 4 are also supported by Xen VMs, so it is possible to use these configs to have networking with both Xen & QEMU/KVMs connected to each other. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

2 2

[Libvir] Persistence / re-instate of iptables rules
by Daniel P. Berrange 21 Mar '07

21 Mar '07

With the virtual networking capability we have to add various rules to the iptables chains to ensure that outgoing connections are forwarded + NATed to the physical LAN. Now if the user does 'service iptables restart' these rules are lost until you restart the VM. This obviously sucks. We've been exploring the possibility of adapting the Fedora / RHEL iptables scripts to allow user-defined chains which are automatically restored from a 'safe' config file during a restart. This is not present in FC6 / RHEL5 or even F6 yet, nor does it help non-Fedora userrs. We already have ability to add / remove rules from iptables, so I was wondering how hard it would be to list existing rules. From whence we can look at existing rules to see if our virtual network forwarding/NAT rules were missing. The idea being that a simple 'killall -SIGHUP libvirt_qemud' could trigger libvirt to check & re-add the iptables rules if missing. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

2 1

Re: [Libvir] [patch 4/5] iptables: maintain rules in memory even when not writing to disk
by Daniel Veillard 21 Mar '07

21 Mar '07

On Wed, Mar 21, 2007 at 12:48:01PM +0000, Mark McLoughlin wrote: > Re-factor things a little for the next patch so that we maintain > the rules in memory even when we're not going to write them to > disk. [...] > if (!(r = (char **)realloc(rules->rules, sizeof(char *) * (rules->nrules+1)))) > return ENOMEM; Seems the rules tab is zeroed, but when you extend it you don't zeroe the new part of the array, but it may not give problem in practice (or a very hard one to track down ;-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

2 1

Re: [Libvir] [patch 2/5] iptables: use calloc() instead of malloc()/memset()
by Daniel Veillard 21 Mar '07

21 Mar '07

On Wed, Mar 21, 2007 at 12:47:59PM +0000, Mark McLoughlin wrote: > Replace a few instances of malloc() followed by memset(0) with > calloc() Humpf, that's just the opposite of the other parts of libvirt I wrote. Could you justify ? It's hard to catch it's a calloc or a malloc call, and hence notice if it was initialized or not. The extra memset line makes it clear, plus it will allow me more easilly to plug in libxml2 memory wrapper. I doubt the patch is related to other parts needed for the restart code it's just coding policy and IMHO different from teh rest of libvirt. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

2 1

[Libvir] [patch 5/5] iptables: reload rules on SIGHUP
by Mark McLoughlin 21 Mar '07

21 Mar '07

As suggested by danpb, make libvirt_qemud handle SIGHUP by re-loading the iptables rules. Signed-off-by: Mark McLoughlin <markmc(a)redhat.com> Index: libvirt/qemud/iptables.c =================================================================== --- libvirt.orig/qemud/iptables.c +++ libvirt/qemud/iptables.c @@ -36,6 +36,8 @@ #include <sys/stat.h> #include <sys/wait.h> +#include "internal.h" + enum { ADD = 0, REMOVE @@ -48,11 +50,18 @@ enum { typedef struct { + char *rule; + char **argv; + int flipflop; +} iptRule; + +typedef struct +{ char *table; char *chain; - int nrules; - char **rules; + int nrules; + iptRule *rules; #ifdef IPTABLES_DIR @@ -73,7 +82,7 @@ struct _iptablesContext #ifdef IPTABLES_DIR static int writeRules(const char *path, - char * const *rules, + const iptRules *rules, int nrules) { char tmp[PATH_MAX]; @@ -96,7 +105,7 @@ writeRules(const char *path, } for (i = 0; i < nrules; i++) { - if (fputs(rules[i], f) == EOF || + if (fputs(rules[i].rule, f) == EOF || fputc('\n', f) == EOF) { fclose(f); if (istmp) @@ -173,19 +182,43 @@ buildPath(const char *table, } #endif /* IPTABLES_DIR */ -static int -iptRulesAppend(iptRules *rules, - const char *rule) +static void +iptRuleFree(iptRule *rule) { - char **r; + if (rule->rule) + free(rule->rule); + rule->rule = NULL; + + if (rule->argv) { + int i = 0; + while (rule->argv[i]) + free(rule->argv[i++]); + free(rule->argv); + rule->argv = NULL; + } +} - if (!(r = (char **)realloc(rules->rules, sizeof(char *) * (rules->nrules+1)))) +static int +iptRulesAppend(iptRules *rules, + char *rule, + char **argv, + int flipflop) +{ + iptRule *r; + + if (!(r = (iptRule *)realloc(rules->rules, sizeof(iptRule) * (rules->nrules+1)))) { + int i = 0; + while (argv[i]) + free(argv[i++]); + free(argv); return ENOMEM; + } rules->rules = r; - if (!(rules->rules[rules->nrules] = strdup(rule))) - return ENOMEM; + rules->rules[rules->nrules].rule = rule; + rules->rules[rules->nrules].argv = argv; + rules->rules[rules->nrules].flipflop = flipflop; rules->nrules++; @@ -211,17 +244,17 @@ iptRulesRemove(iptRules *rules, int i; for (i = 0; i < rules->nrules; i++) - if (!strcmp(rules->rules[i], strdup(rule))) + if (!strcmp(rules->rules[i].rule, strdup(rule))) break; if (i >= rules->nrules) return EINVAL; - free(rules->rules[i]); + iptRuleFree(&rules->rules[i]); memmove(&rules->rules[i], &rules->rules[i+1], - (rules->nrules - i - 1) * sizeof (char *)); + (rules->nrules - i - 1) * sizeof (iptRule)); rules->nrules--; @@ -253,16 +286,14 @@ iptRulesFree(iptRules *rules) } - for (i = 0; i < rules->nrules; i++) { - free(rules->rules[i]); - rules->rules[i] = NULL; - } - - rules->nrules = 0; - if (rules->rules) { + for (i = 0; i < rules->nrules; i++) + iptRuleFree(&rules->rules[i]); + free(rules->rules); rules->rules = NULL; + + rules->nrules = 0; } #ifdef IPTABLES_DIR @@ -480,10 +511,13 @@ iptablesAddRemoveRule(iptRules *rules, i (retval = iptablesAddRemoveChain(rules, action))) goto error; - if (action == ADD) - retval = iptRulesAppend(rules, rule); - else + if (action == ADD) { + retval = iptRulesAppend(rules, rule, argv, flipflop); + rule = NULL; + argv = NULL; + } else { retval = iptRulesRemove(rules, rule); + } error: if (rule) @@ -535,6 +569,45 @@ iptablesContextFree(iptablesContext *ctx free(ctx); } +static void +iptRulesReload(iptRules *rules) +{ + int i; + int retval; + + for (i = 0; i < rules->nrules; i++) { + iptRule *rule = &rules->rules[i]; + char *orig; + + orig = rule->argv[rule->flipflop]; + rule->argv[rule->flipflop] = (char *) "--delete"; + + if ((retval = iptablesSpawn(WITH_ERRORS, rule->argv))) + qemudLog(QEMUD_WARN, "Failed to remove iptables rule '%s' from chain '%s' in table '%s': %s", + rule->rule, rules->chain, rules->table, strerror(errno)); + + rule->argv[rule->flipflop] = orig; + } + + if ((retval = iptablesAddRemoveChain(rules, REMOVE)) || + (retval = iptablesAddRemoveChain(rules, ADD))) + qemudLog(QEMUD_WARN, "Failed to re-create chain '%s' in table '%s': %s", + rules->chain, rules->table, strerror(retval)); + + for (i = 0; i < rules->nrules; i++) + if ((retval = iptablesSpawn(WITH_ERRORS, rules->rules[i].argv))) + qemudLog(QEMUD_WARN, "Failed to add iptables rule '%s' to chain '%s' in table '%s': %s", + rules->rules[i].rule, rules->chain, rules->table, strerror(retval)); +} + +void +iptablesReloadRules(iptablesContext *ctx) +{ + iptRulesReload(ctx->input_filter); + iptRulesReload(ctx->forward_filter); + iptRulesReload(ctx->nat_postrouting); +} + static int iptablesInput(iptablesContext *ctx, const char *iface, Index: libvirt/qemud/iptables.h =================================================================== --- libvirt.orig/qemud/iptables.h +++ libvirt/qemud/iptables.h @@ -27,6 +27,8 @@ typedef struct _iptablesContext iptables iptablesContext *iptablesContextNew (void); void iptablesContextFree (iptablesContext *ctx); +void iptablesReloadRules (iptablesContext *ctx); + int iptablesAddTcpInput (iptablesContext *ctx, const char *iface, int port); Index: libvirt/qemud/qemud.c =================================================================== --- libvirt.orig/qemud/qemud.c +++ libvirt/qemud/qemud.c @@ -88,6 +88,11 @@ static int qemudDispatchSignal(struct qe case SIGHUP: qemudLog(QEMUD_INFO, "Reloading configuration on SIGHUP"); ret = qemudScanConfigs(server); + + if (server->iptables) { + qemudLog(QEMUD_INFO, "Reloading iptables rules"); + iptablesReloadRules(server->iptables); + } break; case SIGINT: --

1 0

[Libvir] [patch 4/5] iptables: maintain rules in memory even when not writing to disk
by Mark McLoughlin 21 Mar '07

21 Mar '07

Re-factor things a little for the next patch so that we maintain the rules in memory even when we're not going to write them to disk. Signed-off-by: Mark McLoughlin <markmc(a)redhat.com> Index: libvirt/qemud/iptables.c =================================================================== --- libvirt.orig/qemud/iptables.c +++ libvirt/qemud/iptables.c @@ -51,14 +51,14 @@ typedef struct char *table; char *chain; + int nrules; + char **rules; + #ifdef IPTABLES_DIR char dir[PATH_MAX]; char path[PATH_MAX]; - int nrules; - char **rules; - #endif /* IPTABLES_DIR */ } iptRules; @@ -171,13 +171,13 @@ buildPath(const char *table, else return 0; } +#endif /* IPTABLES_DIR */ static int iptRulesAppend(iptRules *rules, const char *rule) { char **r; - int err; if (!(r = (char **)realloc(rules->rules, sizeof(char *) * (rules->nrules+1)))) return ENOMEM; @@ -189,24 +189,29 @@ iptRulesAppend(iptRules *rules, rules->nrules++; - if ((err = ensureDir(rules->dir))) - return err; +#ifdef IPTABLES_DIR + { + int err; - if ((err = writeRules(rules->path, rules->rules, rules->nrules))) - return err; + if ((err = ensureDir(rules->dir))) + return err; + + if ((err = writeRules(rules->path, rules->rules, rules->nrules))) + return err; + } +#endif /* IPTABLES_DIR */ return 0; } static int iptRulesRemove(iptRules *rules, - const char *rule) + char *rule) { int i; - int err; for (i = 0; i < rules->nrules; i++) - if (!strcmp(rules->rules[i], rule)) + if (!strcmp(rules->rules[i], strdup(rule))) break; if (i >= rules->nrules) @@ -220,16 +225,23 @@ iptRulesRemove(iptRules *rules, rules->nrules--; - if ((err = writeRules(rules->path, rules->rules, rules->nrules))) - return err; +#ifdef IPTABLES_DIR + { + int err; + + if ((err = writeRules(rules->path, rules->rules, rules->nrules))) + return err; + } +#endif /* IPTABLES_DIR */ return 0; } -#endif /* IPTABLES_DIR */ static void iptRulesFree(iptRules *rules) { + int i; + if (rules->table) { free(rules->table); rules->table = NULL; @@ -240,25 +252,22 @@ iptRulesFree(iptRules *rules) rules->chain = NULL; } -#ifdef IPTABLES_DIR - { - int i; - rules->dir[0] = '\0'; - rules->path[0] = '\0'; - - for (i = 0; i < rules->nrules; i++) { - free(rules->rules[i]); - rules->rules[i] = NULL; - } + for (i = 0; i < rules->nrules; i++) { + free(rules->rules[i]); + rules->rules[i] = NULL; + } - rules->nrules = 0; + rules->nrules = 0; - if (rules->rules) { - free(rules->rules); - rules->rules = NULL; - } + if (rules->rules) { + free(rules->rules); + rules->rules = NULL; } + +#ifdef IPTABLES_DIR + rules->dir[0] = '\0'; + rules->path[0] = '\0'; #endif /* IPTABLES_DIR */ free(rules); @@ -279,15 +288,15 @@ iptRulesNew(const char *table, if (!(rules->chain = strdup(chain))) goto error; + rules->rules = NULL; + rules->nrules = 0; + #ifdef IPTABLES_DIR if (buildDir(table, rules->dir, sizeof(rules->dir))) goto error; if (buildPath(table, chain, rules->path, sizeof(rules->path))) goto error; - - rules->rules = NULL; - rules->nrules = 0; #endif /* IPTABLES_DIR */ return rules; @@ -464,12 +473,10 @@ iptablesAddRemoveRule(iptRules *rules, i (retval = iptablesAddRemoveChain(rules, action))) goto error; -#ifdef IPTABLES_DIR if (action == ADD) retval = iptRulesAppend(rules, rule); else retval = iptRulesRemove(rules, rule); -#endif /* IPTABLES_DIR */ error: if (rule) --

1 0

[Libvir] [patch 3/5] iptables: fix iptablesPhysdevForward()
by Mark McLoughlin 21 Mar '07

21 Mar '07

Remove the target interface parameter from iptablesPhysdevForward(). This rule is intended to allow frames to be forwarded across the bridge from the supplied bridge port. In this context, the --out parameter would match the outgoing bridge port, which will never be network->def->forwardDev. Signed-off-by: Mark McLoughlin <markmc(a)redhat.com> Index: libvirt/qemud/iptables.c =================================================================== --- libvirt.orig/qemud/iptables.c +++ libvirt/qemud/iptables.c @@ -577,41 +577,28 @@ iptablesRemoveUdpInput(iptablesContext * static int iptablesPhysdevForward(iptablesContext *ctx, const char *iface, - const char *target, int action) { - if (target && target[0]) { - return iptablesAddRemoveRule(ctx->forward_filter, - action, - "--match", "physdev", - "--physdev-in", iface, - "--out", target, - "--jump", "ACCEPT", - NULL); - } else { - return iptablesAddRemoveRule(ctx->forward_filter, - action, - "--match", "physdev", - "--physdev-in", iface, - "--jump", "ACCEPT", - NULL); - } + return iptablesAddRemoveRule(ctx->forward_filter, + action, + "--match", "physdev", + "--physdev-in", iface, + "--jump", "ACCEPT", + NULL); } int iptablesAddPhysdevForward(iptablesContext *ctx, - const char *iface, - const char *target) + const char *iface) { - return iptablesPhysdevForward(ctx, iface, target, ADD); + return iptablesPhysdevForward(ctx, iface, ADD); } int iptablesRemovePhysdevForward(iptablesContext *ctx, - const char *iface, - const char *target) + const char *iface) { - return iptablesPhysdevForward(ctx, iface, target, REMOVE); + return iptablesPhysdevForward(ctx, iface, REMOVE); } static int Index: libvirt/qemud/iptables.h =================================================================== --- libvirt.orig/qemud/iptables.h +++ libvirt/qemud/iptables.h @@ -42,11 +42,9 @@ int iptablesRemoveUdpInput int port); int iptablesAddPhysdevForward (iptablesContext *ctx, - const char *iface, - const char *target); + const char *iface); int iptablesRemovePhysdevForward (iptablesContext *ctx, - const char *iface, - const char *target); + const char *iface); int iptablesAddInterfaceForward (iptablesContext *ctx, const char *iface, Index: libvirt/qemud/conf.c =================================================================== --- libvirt.orig/qemud/conf.c +++ libvirt/qemud/conf.c @@ -1128,7 +1128,7 @@ qemudNetworkIfaceConnect(struct qemud_se } if (net->type == QEMUD_NET_NETWORK && network->def->forward) { - if ((err = iptablesAddPhysdevForward(server->iptables, ifname, network->def->forwardDev))) { + if ((err = iptablesAddPhysdevForward(server->iptables, ifname))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "Failed to add iptables rule to allow bridging from '%s' :%s", ifname, strerror(err)); @@ -1152,7 +1152,7 @@ qemudNetworkIfaceConnect(struct qemud_se no_memory: if (net->type == QEMUD_NET_NETWORK && network->def->forward) - iptablesRemovePhysdevForward(server->iptables, ifname, network->def->forwardDev); + iptablesRemovePhysdevForward(server->iptables, ifname); qemudReportError(server, VIR_ERR_NO_MEMORY, "tapfds"); error: if (retval) Index: libvirt/qemud/qemud.c =================================================================== --- libvirt.orig/qemud/qemud.c +++ libvirt/qemud/qemud.c @@ -1042,8 +1042,7 @@ qemudNetworkIfaceDisconnect(struct qemud return; } - if (network->def->forward) - iptablesRemovePhysdevForward(server->iptables, net->dst.network.ifname, network->def->forwardDev); + iptablesRemovePhysdevForward(server->iptables, net->dst.network.ifname); } int qemudShutdownVMDaemon(struct qemud_server *server, struct qemud_vm *vm) { @@ -1242,83 +1241,87 @@ qemudAddIptablesRules(struct qemud_serve } /* allow bridging from the bridge interface itself */ - if ((err = iptablesAddPhysdevForward(server->iptables, network->bridge, network->def->forwardDev))) { + if ((err = iptablesAddPhysdevForward(server->iptables, network->bridge))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "failed to add iptables rule to allow bridging from '%s' : %s\n", network->bridge, strerror(err)); goto err1; } - /* allow forwarding packets from the bridge interface */ - if ((err = iptablesAddInterfaceForward(server->iptables, network->bridge, network->def->forwardDev))) { + /* allow DHCP requests through to dnsmasq */ + if ((err = iptablesAddTcpInput(server->iptables, network->bridge, 67))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to allow forwarding from '%s' : %s\n", + "failed to add iptables rule to allow DHCP requests from '%s' : %s\n", network->bridge, strerror(err)); goto err2; } - /* allow forwarding packets to the bridge interface if they are part of an existing connection */ - if ((err = iptablesAddStateForward(server->iptables, network->bridge, network->def->forwardDev))) { + if ((err = iptablesAddUdpInput(server->iptables, network->bridge, 67))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to allow forwarding to '%s' : %s\n", + "failed to add iptables rule to allow DHCP requests from '%s' : %s\n", network->bridge, strerror(err)); goto err3; } - /* enable masquerading */ - if ((err = iptablesAddNonBridgedMasq(server->iptables, network->def->forwardDev))) { + /* allow DNS requests through to dnsmasq */ + if ((err = iptablesAddTcpInput(server->iptables, network->bridge, 53))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to enable masquerading : %s\n", - strerror(err)); + "failed to add iptables rule to allow DNS requests from '%s' : %s\n", + network->bridge, strerror(err)); goto err4; } - /* allow DHCP requests through to dnsmasq */ - if ((err = iptablesAddTcpInput(server->iptables, network->bridge, 67))) { + if ((err = iptablesAddUdpInput(server->iptables, network->bridge, 53))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to allow DHCP requests from '%s' : %s\n", + "failed to add iptables rule to allow DNS requests from '%s' : %s\n", network->bridge, strerror(err)); goto err5; } - if ((err = iptablesAddUdpInput(server->iptables, network->bridge, 67))) { + /* The remaining rules are only needed for IP forwarding */ + if (!network->def->forward) + return 1; + + /* allow forwarding packets from the bridge interface */ + if ((err = iptablesAddInterfaceForward(server->iptables, network->bridge, network->def->forwardDev))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to allow DHCP requests from '%s' : %s\n", + "failed to add iptables rule to allow forwarding from '%s' : %s\n", network->bridge, strerror(err)); goto err6; } - /* allow DNS requests through to dnsmasq */ - if ((err = iptablesAddTcpInput(server->iptables, network->bridge, 53))) { + /* allow forwarding packets to the bridge interface if they are part of an existing connection */ + if ((err = iptablesAddStateForward(server->iptables, network->bridge, network->def->forwardDev))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to allow DNS requests from '%s' : %s\n", + "failed to add iptables rule to allow forwarding to '%s' : %s\n", network->bridge, strerror(err)); goto err7; } - if ((err = iptablesAddUdpInput(server->iptables, network->bridge, 53))) { + /* enable masquerading */ + if ((err = iptablesAddNonBridgedMasq(server->iptables, network->def->forwardDev))) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "failed to add iptables rule to allow DNS requests from '%s' : %s\n", - network->bridge, strerror(err)); + "failed to add iptables rule to enable masquerading : %s\n", + strerror(err)); goto err8; } return 1; err8: - iptablesRemoveTcpInput(server->iptables, network->bridge, 53); + iptablesRemoveStateForward(server->iptables, network->bridge, network->def->forwardDev); err7: - iptablesRemoveUdpInput(server->iptables, network->bridge, 67); + iptablesRemoveInterfaceForward(server->iptables, network->bridge, network->def->forwardDev); err6: - iptablesRemoveTcpInput(server->iptables, network->bridge, 67); + iptablesRemoveUdpInput(server->iptables, network->bridge, 53); err5: - iptablesRemoveNonBridgedMasq(server->iptables, network->def->forwardDev); + iptablesRemoveTcpInput(server->iptables, network->bridge, 53); err4: - iptablesRemoveStateForward(server->iptables, network->bridge, network->def->forwardDev); + iptablesRemoveUdpInput(server->iptables, network->bridge, 67); err3: - iptablesRemoveInterfaceForward(server->iptables, network->bridge, network->def->forwardDev); + iptablesRemoveTcpInput(server->iptables, network->bridge, 67); err2: - iptablesRemovePhysdevForward(server->iptables, network->bridge, network->def->forwardDev); + iptablesRemovePhysdevForward(server->iptables, network->bridge); err1: return 0; } @@ -1327,15 +1330,15 @@ static void qemudRemoveIptablesRules(struct qemud_server *server, struct qemud_network *network) { if (network->def->forward) { - iptablesRemoveUdpInput(server->iptables, network->bridge, 53); - iptablesRemoveTcpInput(server->iptables, network->bridge, 53); - iptablesRemoveUdpInput(server->iptables, network->bridge, 67); - iptablesRemoveTcpInput(server->iptables, network->bridge, 67); iptablesRemoveNonBridgedMasq(server->iptables, network->def->forwardDev); iptablesRemoveStateForward(server->iptables, network->bridge, network->def->forwardDev); iptablesRemoveInterfaceForward(server->iptables, network->bridge, network->def->forwardDev); - iptablesRemovePhysdevForward(server->iptables, network->bridge, network->def->forwardDev); } + iptablesRemoveUdpInput(server->iptables, network->bridge, 53); + iptablesRemoveTcpInput(server->iptables, network->bridge, 53); + iptablesRemoveUdpInput(server->iptables, network->bridge, 67); + iptablesRemoveTcpInput(server->iptables, network->bridge, 67); + iptablesRemovePhysdevForward(server->iptables, network->bridge); } static int @@ -1412,8 +1415,7 @@ int qemudStartNetworkDaemon(struct qemud goto err_delbr; } - if (network->def->forward && - !qemudAddIptablesRules(server, network)) + if (!qemudAddIptablesRules(server, network)) goto err_delbr1; if (network->def->forward && --

1 0