Skip to contentSkip to conversation

bonding: cannot remove certain named devices

PATCH In review Bodo Eggert 8 participants 17 messages Aug 16, 2006 – Aug 18, 2006 7 messages not in the archive

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

Bodo Eggert

author
#7 lore raw
Quoted 3 lines · Stephen Hemminger

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.

Signature · 4 lines
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
maintainer
#8 lore raw

From: Bodo Eggert <[email protected]>
Date: Wed, 16 Aug 2006 02:02:03 +0200

Quoted 9 lines
Quoted 3 lines · Stephen Hemminger

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.

#9 lore raw
Quoted 8 lines · David Miller

From: Bodo Eggert <[email protected]>
Date: Wed, 16 Aug 2006 02:02:03 +0200

Quoted 5 lines
Quoted 3 lines · Stephen Hemminger

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.

Quoted 7 lines
Quoted 3 lines

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

Quoted 2 lines

No? Great, I'm glad that's settled.

#10 lore raw
Quoted 7 lines · Giacomo A. Catenazzi
Quoted 3 lines

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

#11 lore raw
Quoted 4 lines · Bill Nottingham
Quoted 3 lines · Giacomo A. Catenazzi
Quoted 3 lines

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?

Quoted 8 lines
Quoted 3 lines

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

#12 lore raw
In reply to Bill Nottingham
Quoted 11 lines · Bill Nottingham
Quoted 7 lines · Giacomo A. Catenazzi
Quoted 3 lines

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.

Quoted 2 lines

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.

Signature · 2 lines
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
#13 lore raw
Quoted 15 lines · Bodo Eggert
Quoted 11 lines · Bill Nottingham
Quoted 7 lines · Giacomo A. Catenazzi
Quoted 3 lines

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.

Quoted 7 lines
Quoted 2 lines

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

#14 lore raw
Quoted 6 lines · Xavier Bestel
Quoted 4 lines

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

maintainer
#15 lore raw
In reply to Xavier Bestel

From: Xavier Bestel <[email protected]>
Date: Thu, 17 Aug 2006 09:29:43 +0200

Quoted 1 line

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.

#16 lore raw

Ar Iau, 2006-08-17 am 16:23 -0700, ysgrifennodd David Miller:

Quoted 1 line

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...

Quoted 2 lines

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

maintainer
#17 lore raw

From: Alan Cox <[email protected]>
Date: Fri, 18 Aug 2006 01:34:43 +0100

Quoted 6 lines

Ar Iau, 2006-08-17 am 16:23 -0700, ysgrifennodd David Miller:

Quoted 2 lines

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.

#18 lore raw
In reply to David Miller
Quoted 11 lines · David Miller

From: Xavier Bestel <[email protected]>
Date: Thu, 17 Aug 2006 09:29:43 +0200

Quoted 1 line

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

#19 lore raw
Quoted 16 lines · Bill Nottingham
Quoted 4 lines · David Miller

From: Xavier Bestel <[email protected]>
Date: Thu, 17 Aug 2006 09:29:43 +0200

Quoted 9 lines
Quoted 2 lines

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.

Quoted 6 lines

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()

#20 lore raw

PATCH 1/1 net: restrict device names from having whitespace

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(-)
net/core/dev.c +20 -4
index d95e262..56c8afb 100644
@@ -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;
 }
 
 /**
Signature · 1 line
1.4.0
maintainer
#21 lore raw

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

From: Stephen Hemminger <[email protected]>
Date: Thu, 17 Aug 2006 23:11:27 -0700

Quoted 4 lines

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.

#22 lore raw

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

In reply to Stephen Hemminger
Quoted 2 lines · Stephen Hemminger

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

#23 lore raw

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

Quoted 7 lines · Xavier Bestel
Quoted 2 lines · Stephen Hemminger

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

net/core/dev.c +17 -4
index d95e262..906cbf3 100644
@@ -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;
 }
 
 /**