Re: bonding: cannot remove certain named devices

Re: bonding: cannot remove certain named devices

[Bodo Eggert]

Stephen Hemminger <shemminger@osdl.org> 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

Re: bonding: cannot remove certain named devices

[David Miller]

From: Bodo Eggert <7eggert@elstempel.de>
Date: Wed, 16 Aug 2006 02:02:03 +0200

> Stephen Hemminger <shemminger@osdl.org> 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.

Re: bonding: cannot remove certain named devices

[Giacomo A. Catenazzi]

David Miller wrote:
> From: Bodo Eggert <7eggert@elstempel.de>
> Date: Wed, 16 Aug 2006 02:02:03 +0200
> 
>> Stephen Hemminger <shemminger@osdl.org> 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.

Re: bonding: cannot remove certain named devices

[Bill Nottingham]

Giacomo A. Catenazzi (cate@debian.org) 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

Re: bonding: cannot remove certain named devices

[Giacomo A. Catenazzi]

Bill Nottingham wrote:
> Giacomo A. Catenazzi (cate@debian.org) 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

Re: bonding: cannot remove certain named devices

[Bodo Eggert]

On Wed, 16 Aug 2006, Bill Nottingham wrote:
> Giacomo A. Catenazzi (cate@debian.org) 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

Re: bonding: cannot remove certain named devices

[Xavier Bestel]

On Wed, 2006-08-16 at 17:11, Bodo Eggert wrote:
> On Wed, 16 Aug 2006, Bill Nottingham wrote:
> > Giacomo A. Catenazzi (cate@debian.org) 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

Re: bonding: cannot remove certain named devices

[Bill Nottingham]

Xavier Bestel (xavier.bestel@free.fr) 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

Re: bonding: cannot remove certain named devices

[David Miller]

From: Xavier Bestel <xavier.bestel@free.fr>
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.

Re: bonding: cannot remove certain named devices

[Alan Cox]

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

Re: bonding: cannot remove certain named devices

[David Miller]

From: Alan Cox <alan@lxorguk.ukuu.org.uk>
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.

Re: bonding: cannot remove certain named devices

[Bill Nottingham]

David Miller (davem@davemloft.net) said: 
> From: Xavier Bestel <xavier.bestel@free.fr>
> 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

Re: bonding: cannot remove certain named devices

[Stephen Hemminger]

Bill Nottingham wrote:
> David Miller (davem@davemloft.net) said: 
>   
>> From: Xavier Bestel <xavier.bestel@free.fr>
>> 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()

[PATCH] net: restrict device names from having whitespace

[Stephen Hemminger]

Don't allow spaces in network device names because it makes
it difficult to provide text interfaces via sysfs.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 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

Re: [PATCH] net: restrict device names from having whitespace

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
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 <shemminger@osdl.org>

I have a patch which does this in my tree already Stephen.

Re: [PATCH] net: restrict device names from having whitespace

[Xavier Bestel]

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

Re: [PATCH] net: restrict device names from having whitespace

[Xavier Bestel]

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;
 }
 
 /**