OpenVZ Forum - RDF feed
type: channel
items: [PATCH] Don't create tunnels with '%' in name.
items: Re: [PATCH] Don't create tunnels with '%' in name.
items: [PATCH] Don't limit the number of tunnels with generic name explicitly.
items: Re: [PATCH] Don't limit the number of tunnels with generic name explicitly.

[PATCH] Don't create tunnels with '%' in name.
type: item
description: Four tunnel drivers (ip_gre, ipip, ip6_tunnel and sit) can receive a pre-defined name for a device from the userspace. Since these drivers call the register_netdevice() after this (rtnl_lock is held), the device's name may contain a '%' character. Not sure how bad is this to have a device with a '%' in its name, but all the other places either use the register_netdev(), or explicitly call dev_alloc_name() before registering, i.e. do not allow for such names. Signed-off-by: Pavel Emelyanov <xemul@openvz.org> --- diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 63f6917..6b9744f 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -274,19 +274,24 @@ static struct ip_tunnel * ipgre_tunnel_locate(struct ip_tunnel_parm *parms, int if (!dev) return NULL; + if (strchr(name, '%')) { + if (dev_alloc_name(dev, name) < 0) + goto failed_free; + } + dev->init = ipgre_tunnel_init; nt = netdev_priv(dev); nt->parms = *parms; - if (register_netdevice(dev) < 0) { - free_netdev(dev); - goto failed; - } + if (register_netdevice(dev) < 0) + goto failed_free; dev_hold(dev); ipgre_tunnel_link(nt); return nt; +failed_free: + free_netdev(dev); failed: return NULL; } diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c index da28158..118e7d9 100644 --- a/net/ipv4/ipip.c +++ b/net/ipv4/ipip.c @@ -236,19 +236,24 @@ static struct ip_tunnel * ipip_tunnel_locate(struct ip_tunnel_parm *parms, int c if (dev == NULL) return NULL; + if (strchr(name, '%')) { + if (dev_alloc_name(dev, name) < 0) + goto failed_free; + } + nt = netdev_priv(dev); dev->init = ipip_tunnel_init; nt->parms = *parms; - if (register_netdevice(dev) < 0) { - free_netdev(dev); - goto failed; - } + if (register_netdevice(dev) < 0) + goto failed_free; dev_hold(dev); ipip_tunnel_link(nt); return nt; +failed_free: + free_netdev(dev); failed: return NULL; } diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index cd94064..fa83d70 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -245,17 +245,24 @@ static s…
creator: Pavel Emelianov
date: 2008-02-21T12:05:27-00:00

Re: [PATCH] Don't create tunnels with '%' in name.
type: item
description: Pavel Emelyanov wrote: > Four tunnel drivers (ip_gre, ipip, ip6_tunnel and sit) can > receive a pre-defined name for a device from the userspace. > Since these drivers call the register_netdevice() after this > (rtnl_lock is held), the device's name may contain a '%' > character. > > Not sure how bad is this to have a device with a '%' in its > name, but all the other places either use the register_netdev(), > or explicitly call dev_alloc_name() before registering, i.e. > do not allow for such names. > > Signed-off-by: Pavel Emelyanov <xemul@openvz.org> > > --- > > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c > index 63f6917..6b9744f 100644 > --- a/net/ipv4/ip_gre.c > +++ b/net/ipv4/ip_gre.c > @@ -274,19 +274,24 @@ static struct ip_tunnel * ipgre_tunnel_locate(struct ip_tunnel_parm *parms, int > if (!dev) > return NULL; > > + if (strchr(name, '%')) { > + if (dev_alloc_name(dev, name) < 0) > + goto failed_free; > + } > + It would be nicer to replace the entire hand-made name allocation to remove the 100 device limit.
creator: Patrick McHardy
date: 2008-02-21T12:10:16-00:00

Re: [PATCH] Don't create tunnels with '%' in name.
type: item
description: Patrick McHardy wrote: > Pavel Emelyanov wrote: >> Four tunnel drivers (ip_gre, ipip, ip6_tunnel and sit) can >> receive a pre-defined name for a device from the userspace. >> Since these drivers call the register_netdevice() after this >> (rtnl_lock is held), the device's name may contain a '%' >> character. >> >> Not sure how bad is this to have a device with a '%' in its >> name, but all the other places either use the register_netdev(), >> or explicitly call dev_alloc_name() before registering, i.e. >> do not allow for such names. >> >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org> >> >> --- >> >> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c >> index 63f6917..6b9744f 100644 >> --- a/net/ipv4/ip_gre.c >> +++ b/net/ipv4/ip_gre.c >> @@ -274,19 +274,24 @@ static struct ip_tunnel * ipgre_tunnel_locate(struct ip_tunnel_parm *parms, int >> if (!dev) >> return NULL; >> >> + if (strchr(name, '%')) { >> + if (dev_alloc_name(dev, name) < 0) >> + goto failed_free; >> + } >> + > > > It would be nicer to replace the entire hand-made name > allocation to remove the 100 device limit. > Actually, I thought the same, but fixing % in names looks like a BUG-fix for 2.6.25, while removing the hand-made name allocation looks like an enhancement for 2.6.26. No? Thanks, Pavel
creator: Pavel Emelianov
date: 2008-02-21T12:17:27-00:00

Re: [PATCH] Don't create tunnels with '%' in name.
type: item
description: Pavel Emelyanov wrote: > Patrick McHardy wrote: > >> It would be nicer to replace the entire hand-made name >> allocation to remove the 100 device limit. >> > > Actually, I thought the same, but fixing % in names looks like a > BUG-fix for 2.6.25, while removing the hand-made name allocation > looks like an enhancement for 2.6.26. No? Well, its so closely related that I guess it would still look like a bugfix :) But changing this in 2.6.26 is also fine of course, your patch just reminded me since I wanted to change this for a long time and repeatedly forgot about it again.
creator: Patrick McHardy
date: 2008-02-21T12:22:01-00:00

[PATCH] Don't limit the number of tunnels with generic name explicitly.
type: item
description: Patrick McHardy wrote: > Pavel Emelyanov wrote: >> Patrick McHardy wrote: >> >>> It would be nicer to replace the entire hand-made name >>> allocation to remove the 100 device limit. >>> >> Actually, I thought the same, but fixing % in names looks like a >> BUG-fix for 2.6.25, while removing the hand-made name allocation >> looks like an enhancement for 2.6.26. No? > > > Well, its so closely related that I guess it would still look > like a bugfix :) But changing this in 2.6.26 is also fine of > course, your patch just reminded me since I wanted to change > this for a long time and repeatedly forgot about it again. Ok, point taken ;) Here's the 2nd patch that does so. If David decides it can go to 2.6.25, that would be good, otherwise this patch will fit the 2.6.26 as well. Changelog: Use the added dev_alloc_name() call to create tunnel device name, rather than iterate in a hand-made loop with an artificial limit. Thanks Patrick for noticing this. Signed-off-by: Pavel Emelyanov <xemul@openvz.org> --- diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h index c17fa1f..6512d85 100644 --- a/include/net/ip6_tunnel.h +++ b/include/net/ip6_tunnel.h @@ -14,8 +14,6 @@ /* capable of receiving packets */ #define IP6_TNL_F_CAP_RCV 0x20000 -#define IP6_TNL_MAX 128 - /* IPv6 tunnel */ struct ip6_tnl { diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 6b9744f..e7821ba 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -259,16 +259,8 @@ static struct ip_tunnel * ipgre_tunnel_locate(struct ip_tunnel_parm *parms, int if (parms->name[0]) strlcpy(name, parms->name, IFNAMSIZ); - else { - int i; - for (i=1; i<100; i++) { - sprintf(name, "gre%d", i); - if (__dev_get_by_name(&init_net, name) == NULL) - break; - } - if (i==100) - goto failed; - } + else + sprintf(name, "gre%%d"); dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup); if (!dev) @@ -292,7 +284,6 @@ static struct ip_tunnel * ipgre_tunnel_locate(struct ip_tunnel_parm *parms, int failed_free: free_n…
creator: Pavel Emelianov
date: 2008-02-21T12:38:16-00:00

Re: [PATCH] Don't limit the number of tunnels with generic name explicitly.
type: item
description: Pavel Emelyanov wrote: > Patrick McHardy wrote: >> Pavel Emelyanov wrote: >>> Patrick McHardy wrote: >>> >>>> It would be nicer to replace the entire hand-made name >>>> allocation to remove the 100 device limit. >>>> >>> Actually, I thought the same, but fixing % in names looks like a >>> BUG-fix for 2.6.25, while removing the hand-made name allocation >>> looks like an enhancement for 2.6.26. No? >> >> Well, its so closely related that I guess it would still look >> like a bugfix :) But changing this in 2.6.26 is also fine of >> course, your patch just reminded me since I wanted to change >> this for a long time and repeatedly forgot about it again. > > Ok, point taken ;) Here's the 2nd patch that does so. If David > decides it can go to 2.6.25, that would be good, otherwise this > patch will fit the 2.6.26 as well. > > Changelog: > > Use the added dev_alloc_name() call to create tunnel device name, > rather than iterate in a hand-made loop with an artificial limit. > > Thanks Patrick for noticing this. > > Signed-off-by: Pavel Emelyanov <xemul@openvz.org> Looks good to me, thanks.
creator: Patrick McHardy
date: 2008-02-21T12:45:15-00:00

Re: [PATCH] Don't limit the number of tunnels with generic name explicitly.
type: item
description: From: Pavel Emelyanov <xemul@openvz.org> Date: Thu, 21 Feb 2008 15:38:16 +0300 > Changelog: > > Use the added dev_alloc_name() call to create tunnel device name, > rather than iterate in a hand-made loop with an artificial limit. > > Thanks Patrick for noticing this. > > Signed-off-by: Pavel Emelyanov <xemul@openvz.org> Applied, but I had to rework this in two places that didn't apply cleanly. The ip_gre.c and ipip.c changes remove a "failed" label but that can't be done in the current tree as there are other existing references.
creator: davem
date: 2008-02-24T04:19:52-00:00

Re: [PATCH] Don't limit the number of tunnels with generic name explicitly.
type: item
description: David Miller wrote: > From: Pavel Emelyanov <xemul@openvz.org> > Date: Thu, 21 Feb 2008 15:38:16 +0300 > >> Changelog: >> >> Use the added dev_alloc_name() call to create tunnel device name, >> rather than iterate in a hand-made loop with an artificial limit. >> >> Thanks Patrick for noticing this. >> >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org> > > Applied, but I had to rework this in two places that didn't > apply cleanly. That's because you skipped the first patch titled "Don't create tunnels with '%' in name.", which adds the dev_alloc_name() call and tosses the error paths a bit. Without this first patch, these four drivers become broken :( When user doesn't specify the name, the device's name will be e.g. "tunl%d", but not "tunl0" like he expects. > The ip_gre.c and ipip.c changes remove a "failed" label but > that can't be done in the current tree as there are other > existing references. > Yup :( this code was removed in that first patch...
creator: Pavel Emelianov
date: 2008-02-26T07:47:44-00:00

Re: [PATCH] Don't limit the number of tunnels with generic name explicitly.
type: item
description: From: Pavel Emelyanov <xemul@openvz.org> Date: Tue, 26 Feb 2008 10:47:44 +0300 > That's because you skipped the first patch titled "Don't create > tunnels with '%' in name.", which adds the dev_alloc_name() call > and tosses the error paths a bit. Without this first patch, these > four drivers become broken :( When user doesn't specify the name, > the device's name will be e.g. "tunl%d", but not "tunl0" like > he expects. Please respin and post the first patch, I had no idea there was a dependency.
creator: davem
date: 2008-02-26T21:30:54-00:00