* [PATCH] net-2.6 : fix dev_get_valid_name
@ 2010-05-18 17:35 Daniel Lezcano
2010-05-18 17:54 ` Stephen Hemminger
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Lezcano @ 2010-05-18 17:35 UTC (permalink / raw)
To: davem; +Cc: opurdila, netdev
the commit:
commit d90310243fd750240755e217c5faa13e24f41536
Author: Octavian Purdila <opurdila@ixiacom.com>
Date: Wed Nov 18 02:36:59 2009 +0000
net: device name allocation cleanups
introduced a bug when there is a hash collision making impossible
to rename a device with eth%d. This bug is very hard to reproduce
and appears rarely.
The problem is coming from we don't pass a temporary buffer to
__dev_alloc_name but 'dev->name' which is modified by the function.
A detailed explanation is here:
http://marc.info/?l=linux-netdev&m=127417784011987&w=2
Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
---
net/core/dev.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 264137f..4704a1a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -936,18 +936,22 @@ int dev_alloc_name(struct net_device *dev, const char *name)
}
EXPORT_SYMBOL(dev_alloc_name);
-static int dev_get_valid_name(struct net *net, const char *name, char *buf,
- bool fmt)
+static int dev_get_valid_name(struct net_device *dev, const char *name, bool fmt)
{
+ struct net *net;
+
+ BUG_ON(!dev_net(dev));
+ net = dev_net(dev);
+
if (!dev_valid_name(name))
return -EINVAL;
if (fmt && strchr(name, '%'))
- return __dev_alloc_name(net, name, buf);
+ return dev_alloc_name(dev, name);
else if (__dev_get_by_name(net, name))
return -EEXIST;
- else if (buf != name)
- strlcpy(buf, name, IFNAMSIZ);
+ else if (strncmp(dev->name, name, IFNAMSIZ))
+ strlcpy(dev->name, name, IFNAMSIZ);
return 0;
}
@@ -979,7 +983,7 @@ int dev_change_name(struct net_device *dev, const char *newname)
memcpy(oldname, dev->name, IFNAMSIZ);
- err = dev_get_valid_name(net, newname, dev->name, 1);
+ err = dev_get_valid_name(dev, newname, 1);
if (err < 0)
return err;
@@ -5083,7 +5087,7 @@ int register_netdevice(struct net_device *dev)
}
}
- ret = dev_get_valid_name(net, dev->name, dev->name, 0);
+ ret = dev_get_valid_name(dev, dev->name, 0);
if (ret)
goto err_uninit;
@@ -5661,7 +5665,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
/* We get here if we can't use the current device name */
if (!pat)
goto out;
- if (dev_get_valid_name(net, pat, dev->name, 1))
+ if (dev_get_valid_name(dev, pat, 1))
goto out;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net-2.6 : fix dev_get_valid_name
2010-05-18 17:35 [PATCH] net-2.6 : fix dev_get_valid_name Daniel Lezcano
@ 2010-05-18 17:54 ` Stephen Hemminger
2010-05-18 19:55 ` Daniel Lezcano
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2010-05-18 17:54 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, opurdila, netdev
On Tue, 18 May 2010 19:35:12 +0200
Daniel Lezcano <daniel.lezcano@free.fr> wrote:
> +static int dev_get_valid_name(struct net_device *dev, const char *name, bool fmt)
> {
> + struct net *net;
> +
> + BUG_ON(!dev_net(dev));
> + net = dev_net(dev);
This is not really part of the same fix. And not sure why it is added here?
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net-2.6 : fix dev_get_valid_name
2010-05-18 17:54 ` Stephen Hemminger
@ 2010-05-18 19:55 ` Daniel Lezcano
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Lezcano @ 2010-05-18 19:55 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, opurdila, netdev
On 05/18/2010 07:54 PM, Stephen Hemminger wrote:
> On Tue, 18 May 2010 19:35:12 +0200
> Daniel Lezcano<daniel.lezcano@free.fr> wrote:
>
>
>> +static int dev_get_valid_name(struct net_device *dev, const char *name, bool fmt)
>> {
>> + struct net *net;
>> +
>> + BUG_ON(!dev_net(dev));
>> + net = dev_net(dev);
>>
> This is not really part of the same fix. And not sure why it is added here?
>
I could have created a temporary buffer within the function and have
passed it to __dev_alloc_name but this is exactly what does
'dev_alloc_name'.
So 'dev_alloc_name' replaces '__dev_alloc_name' but takes a 'dev'
parameter which is not passed to 'dev_get_valid_name'.
Instead of passing an extra parameter 'dev', I just replaced the 'net'
parameter by the 'dev' parameter and extracted the 'net' pointer from it.
I have a few patches cleaning up these functions and the callers but I
prefer to send them against net-next-2.6.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-05-18 19:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-18 17:35 [PATCH] net-2.6 : fix dev_get_valid_name Daniel Lezcano
2010-05-18 17:54 ` Stephen Hemminger
2010-05-18 19:55 ` Daniel Lezcano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).