2.6.17-rc4+.
Trivial example:
# modprobe bonding (creates bond0)
# ip link set bond0 name "a b"
# echo "-a b" > /sys/class/net/bonding_masters
bonding: unable to delete non-existent bond a
bash: echo: write error: No such device
Bill
On Tue, 15 Aug 2006, Bill Nottingham wrote:
> 2.6.17-rc4+.
>
> Trivial example:
>
> # modprobe bonding (creates bond0)
> # ip link set bond0 name "a b"
> # echo "-a b" > /sys/class/net/bonding_masters
> bonding: unable to delete non-existent bond a
> bash: echo: write error: No such device
>
Yuck. The problem here is the space in the interface name, which the
sysfs code chokes on. The code just does
sscanf(buffer, "%16s", command); /* IFNAMSIZ*/
and goes from there. Because of the space, it only gets the first half of
the interface name.
Suggestions? Do we just omit the sscanf and copy up to the newline (or
EOF)? Should there be another delimiter here?
Are spaces allowed in interface names anyway? I can't believe that
bonding is the only area affected by this.
-Mitch
BTW this will also break if you try to add/remove a slave with a space in
the name through sysfs.
Mitch Williams ([email protected]) said:
> Are spaces allowed in interface names anyway? I can't believe that
> bonding is the only area affected by this.
They're certainly allowed, and the sysfs directory structure, files,
etc. handle it ok. Userspace tends to break in a variety of ways.
I believe the only invalid character in an interface name is '/'.
Bill
On Tue, 15 Aug 2006 16:45:55 -0400
Bill Nottingham <[email protected]> wrote:
> Mitch Williams ([email protected]) said:
> > Are spaces allowed in interface names anyway? I can't believe that
> > bonding is the only area affected by this.
>
> They're certainly allowed, and the sysfs directory structure, files,
> etc. handle it ok. Userspace tends to break in a variety of ways.
>
> I believe the only invalid character in an interface name is '/'.
>
The names "." and ".." are also verboten.
Names with : in them are for IP aliases.
Stephen Hemminger ([email protected]) said:
> > They're certainly allowed, and the sysfs directory structure, files,
> > etc. handle it ok. Userspace tends to break in a variety of ways.
> >
> > I believe the only invalid character in an interface name is '/'.
> >
>
> The names "." and ".." are also verboten.
Right. Well, I suspect they're verboten-because-some-code-breaks-making-the-directory.
> Names with : in them are for IP aliases.
That's certainly not enforced at the kernel level.
Bill
On Tue, 15 Aug 2006, Bill Nottingham wrote:
>
> Stephen Hemminger ([email protected]) said:
> > > They're certainly allowed, and the sysfs directory structure, files,
> > > etc. handle it ok. Userspace tends to break in a variety of ways.
> > >
> > > I believe the only invalid character in an interface name is '/'.
> > >
> >
> > The names "." and ".." are also verboten.
>
> Right. Well, I suspect they're verboten-because-some-code-breaks-making-the-directory.
>
> > Names with : in them are for IP aliases.
>
So can we use
sscanf(buffer, " %[^\n]", command);
instead? This should allow for whitespace in the filename. Bad interface
names will be caught by the call to dev_valid_name().
(I think I'm reading the man page correctly.)
This could have the effect of making the parser way more finicky, though,
since we would allow trailing whitespace. Technically I suppose it's
legal, but it's sure hard to see on the screen.
Anybody have a better solution?
-Mitch
On Tue, 15 Aug 2006 15:41:08 -0700
Mitch Williams <[email protected]> wrote:
> On Tue, 15 Aug 2006, Bill Nottingham wrote:
> >
> > Stephen Hemminger ([email protected]) said:
> > > > They're certainly allowed, and the sysfs directory structure, files,
> > > > etc. handle it ok. Userspace tends to break in a variety of ways.
> > > >
> > > > I believe the only invalid character in an interface name is '/'.
> > > >
> > >
> > > The names "." and ".." are also verboten.
> >
> > Right. Well, I suspect they're verboten-because-some-code-breaks-making-the-directory.
> >
> > > Names with : in them are for IP aliases.
> >
>
> So can we use
> sscanf(buffer, " %[^\n]", command);
> instead? This should allow for whitespace in the filename. Bad interface
> names will be caught by the call to dev_valid_name().
>
> (I think I'm reading the man page correctly.)
>
> This could have the effect of making the parser way more finicky, though,
> since we would allow trailing whitespace. Technically I suppose it's
> legal, but it's sure hard to see on the screen.
>
> Anybody have a better solution?
>
> -Mitch
IMHO idiots who put space's in filenames should be ignored. As long as the
bonding code doesn't throw a fatal error, it has every right to return
"No such device" to the fool.
From: Stephen Hemminger <[email protected]>
Date: Tue, 15 Aug 2006 15:44:44 -0700
> IMHO idiots who put space's in filenames should be ignored. As long
> as the bonding code doesn't throw a fatal error, it has every right
> to return "No such device" to the fool.
I agree that whitespace in device names push the limits of sanity.
But if we believe that, we should enforce it in dev_valid_name().
Does anyone really mind if I add the whitespace check there?
On Tue, 15 Aug 2006, David Miller wrote:
>
> I agree that whitespace in device names push the limits of sanity.
>
> But if we believe that, we should enforce it in dev_valid_name().
>
> Does anyone really mind if I add the whitespace check there?
>
I have no objections. It would certainly make life easier.
OTOH, "push[ing] the limits of sanity" probably describes half of the
subscribers to netdev. But that's neither here nor there.
-Mitch
On Tue, 15 Aug 2006 15:56:12 -0700 (PDT)
David Miller <[email protected]> wrote:
> From: Stephen Hemminger <[email protected]>
> Date: Tue, 15 Aug 2006 15:44:44 -0700
>
> > IMHO idiots who put space's in filenames should be ignored. As long
> > as the bonding code doesn't throw a fatal error, it has every right
> > to return "No such device" to the fool.
>
> I agree that whitespace in device names push the limits of sanity.
>
> But if we believe that, we should enforce it in dev_valid_name().
>
> Does anyone really mind if I add the whitespace check there?
That is a good idea since it protects other interfaces from possible problems.
Stephen Hemminger <[email protected]> wrote:
> IMHO idiots who put space's in filenames should be ignored. As long as the
> bonding code doesn't throw a fatal error, it has every right to return
> "No such device" to the fool.
Maybe you should limit device names to eight uppercase characters and up to
three characters extension, too. NOT! There is no reason to artificially
impose limitations on device names, so don't do that.
--
Ich danke GMX daf?r, die Verwendung meiner Adressen mittels per SPF
verbreiteten L?gen zu sabotieren.
http://david.woodhou.se/why-not-spf.html
From: Bodo Eggert <[email protected]>
Date: Wed, 16 Aug 2006 02:02:03 +0200
> Stephen Hemminger <[email protected]> wrote:
>
> > IMHO idiots who put space's in filenames should be ignored. As long as the
> > bonding code doesn't throw a fatal error, it has every right to return
> > "No such device" to the fool.
>
> Maybe you should limit device names to eight uppercase characters and up to
> three characters extension, too. NOT! There is no reason to artificially
> impose limitations on device names, so don't do that.
Are you willing to work to add the special case code necessary to
handle whitespace characters in the device name over all of the kernel
code and also all of the userland tools too?
No? Great, I'm glad that's settled.
David Miller wrote:
> From: Bodo Eggert <[email protected]>
> Date: Wed, 16 Aug 2006 02:02:03 +0200
>
>> Stephen Hemminger <[email protected]> wrote:
>>
>>> IMHO idiots who put space's in filenames should be ignored. As long as the
>>> bonding code doesn't throw a fatal error, it has every right to return
>>> "No such device" to the fool.
>> Maybe you should limit device names to eight uppercase characters and up to
>> three characters extension, too. NOT! There is no reason to artificially
>> impose limitations on device names, so don't do that.
>
> Are you willing to work to add the special case code necessary to
> handle whitespace characters in the device name over all of the kernel
> code and also all of the userland tools too?
But if you don't handle spaces in userspace, you handle *, ?, [, ], $,
", ', \ in userspace? Should kernel disable also these (insane device
chars) chars?
ciao
cate
>
> No? Great, I'm glad that's settled.
Giacomo A. Catenazzi ([email protected]) said:
> > Are you willing to work to add the special case code necessary to
> > handle whitespace characters in the device name over all of the kernel
> > code and also all of the userland tools too?
>
> But if you don't handle spaces in userspace, you handle *, ?, [, ], $,
> ", ', \ in userspace? Should kernel disable also these (insane device
> chars) chars?
Don't forget unicode characters!
Seriously, while it might be insane to use some of these, I'm wondering
if trying to filter names is more work than fixing the tools.
Bill
Bill Nottingham wrote:
> Giacomo A. Catenazzi ([email protected]) said:
>>> Are you willing to work to add the special case code necessary to
>>> handle whitespace characters in the device name over all of the kernel
>>> code and also all of the userland tools too?
>> But if you don't handle spaces in userspace, you handle *, ?, [, ], $,
>> ", ', \ in userspace? Should kernel disable also these (insane device
>> chars) chars?
>
> Don't forget unicode characters!
>
> Seriously, while it might be insane to use some of these, I'm wondering
> if trying to filter names is more work than fixing the tools.
Fixing tools in always the good approach, and I think in this case wrong
code is really a security problem. IMHO kernel cannot filter all bad
strings.
So, if for the kernel part it is better to filter spaces, ok!
But we should use this user space problems as the motivation to filter
names.
ciao
cate
On Wed, 16 Aug 2006, Bill Nottingham wrote:
> Giacomo A. Catenazzi ([email protected]) said:
> > > Are you willing to work to add the special case code necessary to
> > > handle whitespace characters in the device name over all of the kernel
> > > code and also all of the userland tools too?
> >
> > But if you don't handle spaces in userspace, you handle *, ?, [, ], $,
> > ", ', \ in userspace? Should kernel disable also these (insane device
> > chars) chars?
>
> Don't forget unicode characters!
And long names or control characters.
> Seriously, while it might be insane to use some of these, I'm wondering
> if trying to filter names is more work than fixing the tools.
I think it's sane to avoid control characters and unicode/iso*, since they
can interfere with log output or analysis. I only thought about the kernel
itself and the corresponding userspace tools, which should handle any
character sequence just fine or could be easily fixed.
--
Top 100 things you don't want the sysadmin to say:
14. Any more trouble from you and your account gets moved to the 750
On Wed, 2006-08-16 at 17:11, Bodo Eggert wrote:
> On Wed, 16 Aug 2006, Bill Nottingham wrote:
> > Giacomo A. Catenazzi ([email protected]) said:
>
> > > > Are you willing to work to add the special case code necessary to
> > > > handle whitespace characters in the device name over all of the kernel
> > > > code and also all of the userland tools too?
> > >
> > > But if you don't handle spaces in userspace, you handle *, ?, [, ], $,
> > > ", ', \ in userspace? Should kernel disable also these (insane device
> > > chars) chars?
> >
> > Don't forget unicode characters!
>
> And long names or control characters.
>
> > Seriously, while it might be insane to use some of these, I'm wondering
> > if trying to filter names is more work than fixing the tools.
>
> I think it's sane to avoid control characters and unicode/iso*, since they
> can interfere with log output or analysis. I only thought about the kernel
> itself and the corresponding userspace tools, which should handle any
> character sequence just fine or could be easily fixed.
Why not simply retricting chars to isalnum() ones ?
Xav
Xavier Bestel ([email protected]) said:
> > I think it's sane to avoid control characters and unicode/iso*, since they
> > can interfere with log output or analysis. I only thought about the kernel
> > itself and the corresponding userspace tools, which should handle any
> > character sequence just fine or could be easily fixed.
>
> Why not simply retricting chars to isalnum() ones ?
People might want to use things like '-', '_', etc.
Realistically, any filtering that is done with names has the chance of
breaking 'working' configs that date back to 2.4.
Bill
From: Xavier Bestel <[email protected]>
Date: Thu, 17 Aug 2006 09:29:43 +0200
> Why not simply retricting chars to isalnum() ones ?
As Bill said that would block things like "-" and "_" which are fine.
Bill also mentioned something about "breaking configs going back to
2.4.x" which is bogus because nothing broke when we started blocking
"/" and "." and ".." in networking device names during the addition of
sysfs support for net devices.
Nobody in their right mind puts a space in their network device name.
All you "name purists", go rename the block device name that is used
for your root partition to something with a space in it, and watch how
many startup scripts and command line invocations just explode.
There is absolutely no valid argument for allowing spaces in network
device names.
Ar Iau, 2006-08-17 am 16:23 -0700, ysgrifennodd David Miller:
> Nobody in their right mind puts a space in their network device name.
It works fine. Been there done that. I'm probably not in my right mind
but it causes no problems. Nor btw does UTF-8 naming which is handy if
you want to name your devices in Japanese or Arabic...
> All you "name purists", go rename the block device name that is used
> for your root partition to something with a space in it
Works fine. It doesn't work fine for non root volumes (except by label)
because of the fstab format but root is ok !
Alan
From: Alan Cox <[email protected]>
Date: Fri, 18 Aug 2006 01:34:43 +0100
> Ar Iau, 2006-08-17 am 16:23 -0700, ysgrifennodd David Miller:
> > All you "name purists", go rename the block device name that is used
> > for your root partition to something with a space in it
>
> Works fine. It doesn't work fine for non root volumes (except by label)
> because of the fstab format but root is ok !
Check out how your root device would be fsck'd. The command line run
by the /etc/init* scripts either doesn't quote the device argument or
it consults fstab which as you said has limitations when dealing with
spaces.
It's either going to do something like $device (this is what debian
derived systems do), unquoted, or it will do "fsck -A" which runs into
said fstab format limitations (which is what fedora does).
Either way the point is that this issue is scattered all over the
place.
Preventing spaces in the name doesn't prevent the use of names in non-
romanized languages any moreso than preventing ".", "..", and "/"
already does.
David Miller ([email protected]) said:
> From: Xavier Bestel <[email protected]>
> Date: Thu, 17 Aug 2006 09:29:43 +0200
>
> > Why not simply retricting chars to isalnum() ones ?
>
> As Bill said that would block things like "-" and "_" which are fine.
>
> Bill also mentioned something about "breaking configs going back to
> 2.4.x" which is bogus because nothing broke when we started blocking
> "/" and "." and ".." in networking device names during the addition of
> sysfs support for net devices.
I was mainly referring to if we started to filter it out to isalnum() -
spaces/tab/CR etc. certainly could be filtered. (No idea what would
happen with unicode nbsp or other silly things.)
Bill
Bill Nottingham wrote:
> David Miller ([email protected]) said:
>
>> From: Xavier Bestel <[email protected]>
>> Date: Thu, 17 Aug 2006 09:29:43 +0200
>>
>>
>>> Why not simply retricting chars to isalnum() ones ?
>>>
>> As Bill said that would block things like "-" and "_" which are fine.
>>
>> Bill also mentioned something about "breaking configs going back to
>> 2.4.x" which is bogus because nothing broke when we started blocking
>> "/" and "." and ".." in networking device names during the addition of
>> sysfs support for net devices.
>>
>
> I was mainly referring to if we started to filter it out to isalnum() -
> spaces/tab/CR etc. certainly could be filtered. (No idea what would
> happen with unicode nbsp or other silly things.)
>
> Bill
>
How just restrictiting to !isspace()
Don't allow spaces in network device names because it makes
it difficult to provide text interfaces via sysfs.
Signed-off-by: Stephen Hemminger <[email protected]>
---
net/core/dev.c | 24 ++++++++++++++++++++----
1 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index d95e262..56c8afb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -79,6 +79,7 @@ #include <linux/capability.h>
#include <linux/cpu.h>
#include <linux/types.h>
#include <linux/kernel.h>
+#include <linux/ctype.h>
#include <linux/sched.h>
#include <linux/mutex.h>
#include <linux/string.h>
@@ -636,10 +637,25 @@ struct net_device * dev_get_by_flags(uns
*/
int dev_valid_name(const char *name)
{
- return !(*name == '\0'
- || !strcmp(name, ".")
- || !strcmp(name, "..")
- || strchr(name, '/'));
+ if (*name == '\0') /* null string */
+ return 0;
+
+ if (*name == '.') {
+ if (name[1] == '\0') /* can't have . in directory */
+ return 0;
+ if (name[1] == '.' && name[2] == '\0')
+ return 0; /* or .. */
+ }
+
+ /* Check for blanks and slash because it confuses sysfs interfaces */
+ do {
+ if (*name == '/')
+ return 0;
+ if (isspace(*name))
+ return 0;
+ } while (*++name);
+
+ return 1;
}
/**
--
1.4.0
From: Stephen Hemminger <[email protected]>
Date: Thu, 17 Aug 2006 23:11:27 -0700
> Don't allow spaces in network device names because it makes
> it difficult to provide text interfaces via sysfs.
>
> Signed-off-by: Stephen Hemminger <[email protected]>
I have a patch which does this in my tree already Stephen.
On Fri, 2006-08-18 at 08:11, Stephen Hemminger wrote:
> Don't allow spaces in network device names because it makes
> it difficult to provide text interfaces via sysfs.
Personally I would at least avoid all chars <= ' ', because an interface
name is meant to be displayed and these control chars do no good on a
console nor in X.
Xav
On Fri, 2006-08-18 at 09:17, Xavier Bestel wrote:
> On Fri, 2006-08-18 at 08:11, Stephen Hemminger wrote:
> > Don't allow spaces in network device names because it makes
> > it difficult to provide text interfaces via sysfs.
>
> Personally I would at least avoid all chars <= ' ', because an interface
> name is meant to be displayed and these control chars do no good on a
> console nor in X.
Something like the following patch (short of a full in-kernel utf8
validator). That said it starts looking like policy in the kernel. Maybe
the "no space in devname" should just be enforced by some userspace
tool, not by the kernel itself ?
Xav
diff --git a/net/core/dev.c b/net/core/dev.c
index d95e262..906cbf3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -636,10 +636,23 @@ struct net_device * dev_get_by_flags(uns
*/
int dev_valid_name(const char *name)
{
- return !(*name == '\0'
- || !strcmp(name, ".")
- || !strcmp(name, "..")
- || strchr(name, '/'));
+ if(!*name) /* empty string */
+ return 0;
+ if(*name == '.') { /* . or .. */
+ if(!name[1])
+ return 0;
+ if(name[1] == '.' && !name[2])
+ return 0;
+ }
+ /* control char, space or slash */
+ while(*name) {
+ if(*name == '/')
+ return 0;
+ if(*name <= ' ')
+ return 0;
+ ++name;
+ }
+ return 1;
}
/**