module tool with 2.6.9 limitation issue

module tool with 2.6.9 limitation issue

[yiding_wang]

I am using moudle-init-tools-3.1-pre6 with kernel 2.6.9. The new insmod seems have restrictions which failed using parameters to load a driver module.

My module parameter is in the form of modname="*************** ****", a quite long one.
Run - insmod modname.o modname="*********** *******" (with a script), it complains about the space and treats the string next to the space to be a "Unknown parameter".

By replacing the space with any character, then it complains 
"modname: string parameter too long"

Reducing the length of the parameter to less than 1k character works fine.

Same long parameter string wit space in between works fine under 2.4.25 with original insmod.

Questions:
1, Is this a bug or new insmod has restrictions? 
2, If it is restriction on special character such as space, or limitation on parameter length, then why and what is the limit? 
3, If insmod has limitation, then what is better way to pass long parameter with some special character? 

Thanks!

Eddie

Re: module tool with 2.6.9 limitation issue

[Randy.Dunlap]

[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]

yiding_wang@agilent.com wrote:
> I am using moudle-init-tools-3.1-pre6 with kernel 2.6.9. The new insmod seems have restrictions which failed using parameters to load a driver module.
> 
> My module parameter is in the form of modname="*************** ****", a quite long one.
> Run - insmod modname.o modname="*********** *******" (with a script), it complains about the space and treats the string next to the space to be a "Unknown parameter".
> 
> By replacing the space with any character, then it complains 
> "modname: string parameter too long"

Patch for that one is attached.  It might be overkill,
but it works with this patch (as long as parameter value length
is <= 1024 -- not that I tried one of that length).

Rusty or anyone else, where do the quotes come from?
Here's what I enter:
insmod test_module modprm="this test"

but the kernel sees this parameter string:
"modprm=this test"
(with the quotation marks).
Who/what moved the quotation marks?  Is bash doing any of that?
I don't see it in insmod or modprobe (with a quick look).

> Reducing the length of the parameter to less than 1k character works fine.

Right, parameter value length cannot be > 1024.

> Same long parameter string wit space in between works fine under 2.4.25 with original insmod.
> 
> Questions:
> 1, Is this a bug or new insmod has restrictions? 

The limit is actually in linux/kernel/params.c, not in insmod.
It seems a little arbitrary, but a parameter that long also seems
quite excessive.  Just break it into multiple parameters.

> 2, If it is restriction on special character such as space, or limitation on parameter length, then why and what is the limit? 

Just 1024 bytes in total length.

> 3, If insmod has limitation, then what is better way to pass long parameter with some special character? 

Just break it into multiple parameters....

-- 
~Randy

[-- Attachment #2: modprm_quoted.patch --]
[-- Type: text/x-patch, Size: 1175 bytes --]

linux-2610-rc1-bk21
description

Signed-off-by: Randy Dunlap <rddunlap@osdl.org>

diffstat:=
 kernel/params.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff -Naurp ./kernel/params.c~modprm_quoted ./kernel/params.c
--- ./kernel/params.c~modprm_quoted	2004-11-11 15:13:26.000000000 -0800
+++ ./kernel/params.c	2004-11-11 19:42:36.714124784 -0800
@@ -77,10 +77,16 @@ static int parse_one(char *param,
 static char *next_arg(char *args, char **param, char **val)
 {
 	unsigned int i, equals = 0;
-	int in_quote = 0;
+	int in_quote = 0, quoted = 0;
+	char *next;
 
 	/* Chew any extra spaces */
 	while (*args == ' ') args++;
+	if (*args == '"') {
+		args++;
+		in_quote = 1;
+		quoted = 1;
+	}
 
 	for (i = 0; args[i]; i++) {
 		if (args[i] == ' ' && !in_quote)
@@ -106,13 +112,16 @@ static char *next_arg(char *args, char *
 			if (args[i-1] == '"')
 				args[i-1] = '\0';
 		}
+		if (quoted && args[i-1] == '"')
+			args[i-1] = '\0';
 	}
 
 	if (args[i]) {
 		args[i] = '\0';
-		return args + i + 1;
+		next = args + i + 1;
 	} else
-		return args + i;
+		next = args + i;
+	return next;
 }
 
 /* Args looks like "foo=bar,bar2 baz=fuz wiz". */

[PATCH] handle quoted module parameters

[Randy.Dunlap]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

Here's a patch with better description.


Fix module parameter quote handling.
Module parameter strings (with spaces) are quoted like so:
"modprm=this test"
and not like this:
modprm="this test"

Signed-off-by: Randy Dunlap <rddunlap@osdl.org>

diffstat:=
  kernel/params.c |   15 ++++++++++++---
  1 files changed, 12 insertions(+), 3 deletions(-)


-- 
~Randy

[-- Attachment #2: modprm_quoted.patch --]
[-- Type: text/x-patch, Size: 991 bytes --]


diff -Naurp ./kernel/params.c~modprm_quoted ./kernel/params.c
--- ./kernel/params.c~modprm_quoted	2004-11-11 15:13:26.000000000 -0800
+++ ./kernel/params.c	2004-11-11 19:42:36.714124784 -0800
@@ -77,10 +77,16 @@ static int parse_one(char *param,
 static char *next_arg(char *args, char **param, char **val)
 {
 	unsigned int i, equals = 0;
-	int in_quote = 0;
+	int in_quote = 0, quoted = 0;
+	char *next;
 
 	/* Chew any extra spaces */
 	while (*args == ' ') args++;
+	if (*args == '"') {
+		args++;
+		in_quote = 1;
+		quoted = 1;
+	}
 
 	for (i = 0; args[i]; i++) {
 		if (args[i] == ' ' && !in_quote)
@@ -106,13 +112,16 @@ static char *next_arg(char *args, char *
 			if (args[i-1] == '"')
 				args[i-1] = '\0';
 		}
+		if (quoted && args[i-1] == '"')
+			args[i-1] = '\0';
 	}
 
 	if (args[i]) {
 		args[i] = '\0';
-		return args + i + 1;
+		next = args + i + 1;
 	} else
-		return args + i;
+		next = args + i;
+	return next;
 }
 
 /* Args looks like "foo=bar,bar2 baz=fuz wiz". */

Re: [PATCH] handle quoted module parameters

[Rusty Russell]

On Thu, 2004-11-11 at 20:16 -0800, Randy.Dunlap wrote: 
> Here's a patch with better description.
> 
> 
> Fix module parameter quote handling.
> Module parameter strings (with spaces) are quoted like so:
> "modprm=this test"
> and not like this:
> modprm="this test"

Well, the quote handling in insmod was ripped out after 3.0, exactly
because it was broken like this.  But modprobe will use the latter form,
since it will paste it straight from the modprobe.conf file (which needs
quotes in options lines).

Hope that clarifies,
Rusty.
PS. module-init-tools 3.1 just out...
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

RE: [PATCH] handle quoted module parameters

[yiding_wang]

Hello Randy,

Thanks for your two responses!

Based on your patch, the format of argument will be changed from standard format before:
Used to be:
modprm1=first,ext modprm2=second,ext modprm3="third1,ext third2,ext"
where the quotation in modprm3 represents the whole string, including space, to be the value of third parameter modprm3.

Now the patch changes modprm3 to "modprm3=third1,ext third2,ext" which equivalent to putting quotation mark on normal parameter define "modprm1=first,ext". Do you think linux community will take that change?

Another question is the parameter length is not limited in 2.4.x kernel. Why this is restricted under 2.6.x. (param_set_charp())?

Regards,

Eddie


-----Original Message-----
From: Randy.Dunlap [mailto:rddunlap@osdl.org]
Sent: Thursday, November 11, 2004 8:17 PM
To: Randy.Dunlap
Cc: yiding_wang@agilent.com; arjan@infradead.org;
linux-kernel@vger.kernel.org; rusty@rustcorp.com.au; akpm
Subject: [PATCH] handle quoted module parameters


Here's a patch with better description.


Fix module parameter quote handling.
Module parameter strings (with spaces) are quoted like so:
"modprm=this test"
and not like this:
modprm="this test"

Signed-off-by: Randy Dunlap <rddunlap@osdl.org>

diffstat:=
  kernel/params.c |   15 ++++++++++++---
  1 files changed, 12 insertions(+), 3 deletions(-)


-- 
~Randy

RE: [PATCH] handle quoted module parameters

[Adam Heath]

On Fri, 12 Nov 2004,  wrote:

> Hello Randy,
>
> Thanks for your two responses!
>
> Based on your patch, the format of argument will be changed from standard format before:
> Used to be:
> modprm1=first,ext modprm2=second,ext modprm3="third1,ext third2,ext"
> where the quotation in modprm3 represents the whole string, including space, to be the value of third parameter modprm3.
>
> Now the patch changes modprm3 to "modprm3=third1,ext third2,ext" which equivalent to putting quotation mark on normal parameter define "modprm1=first,ext". Do you think linux community will take that change?
>
> Another question is the parameter length is not limited in 2.4.x kernel. Why this is restricted under 2.6.x. (param_set_charp())?

Er, no, that's a wrong assumption.

Quoting like this is handled by the shell.  It tells it how to parse the
single cmdline string, into separate parts.

There is *no* difference between:

foo="111 222 333"\ 444' 555'

and

foo='111 222 333 444 555'

and

foo="111 222 333 444 555'

RE: [PATCH] handle quoted module parameters

[yiding_wang]

>There is *no* difference between:
>foo="111 222 333"\ 444' 555'
>and
>foo='111 222 333 444 555'
>and
>foo="111 222 333 444 555'

But there is a difference between foo="111 222 333" and "foo=111 222 333". The new patch is changing from former to later.

Eddie


-----Original Message-----
From: Adam Heath [mailto:doogie@debian.org]
Sent: Friday, November 12, 2004 11:41 AM
To: yiding_wang@agilent.com
Cc: rddunlap@osdl.org; arjan@infradead.org;
linux-kernel@vger.kernel.org; rusty@rustcorp.com.au; akpm@osdl.org
Subject: RE: [PATCH] handle quoted module parameters


On Fri, 12 Nov 2004,  wrote:

> Hello Randy,
>
> Thanks for your two responses!
>
> Based on your patch, the format of argument will be changed from standard format before:
> Used to be:
> modprm1=first,ext modprm2=second,ext modprm3="third1,ext third2,ext"
> where the quotation in modprm3 represents the whole string, including space, to be the value of third parameter modprm3.
>
> Now the patch changes modprm3 to "modprm3=third1,ext third2,ext" which equivalent to putting quotation mark on normal parameter define "modprm1=first,ext". Do you think linux community will take that change?
>
> Another question is the parameter length is not limited in 2.4.x kernel. Why this is restricted under 2.6.x. (param_set_charp())?

Er, no, that's a wrong assumption.

Quoting like this is handled by the shell.  It tells it how to parse the
single cmdline string, into separate parts.

There is *no* difference between:

foo="111 222 333"\ 444' 555'

and

foo='111 222 333 444 555'

and

foo="111 222 333 444 555'

Re: [PATCH] handle quoted module parameters

[Rusty Russell]

On Sat, 2004-11-13 at 14:22 -0800, Randy.Dunlap wrote:
> yiding_wang@agilent.com wrote:
> >>There is *no* difference between:
> >>foo="111 222 333"\ 444' 555'
> >>and
> >>foo='111 222 333 444 555'
> >>and
> >>foo="111 222 333 444 555'
> > 
> > 
> > But there is a difference between foo="111 222 333" and "foo=111 222 333". The new patch is changing from former to later.
> 
> Actually the patch allows (or _should allow_) either format for quote
> marks.  I didn't remove the older code, just added support for the
> case of quote marks as "foo=this is a test".

Yes, I have no fundamental problem with the patch, but it'd need
thorough testing (eg. with __setup) since this area has broken before.

> Why is the module param length limit of 1024 a problem?

The 1024 test is there because we want to limit how much we output
through sysfs.  We could up it to PAGE_SIZE-1.

Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

RE: [PATCH] handle quoted module parameters

[yiding_wang]

Hello Rusty,

The broken part I encountered is from the latest module-init-tools 3.1. Is that possible to restore the allowable parameter length as it for 2.4.x, at least increase it from 1K to 4K?

Regards,

Eddie

-----Original Message-----
From: Rusty Russell [mailto:rusty@rustcorp.com.au]
Sent: Sunday, November 14, 2004 5:15 PM
To: Randy.Dunlap
Cc: yiding_wang@agilent.com; arjan@infradead.org; lkml - Kernel Mailing
List; Andrew Morton
Subject: Re: [PATCH] handle quoted module parameters


On Thu, 2004-11-11 at 20:16 -0800, Randy.Dunlap wrote: 
> Here's a patch with better description.
> 
> 
> Fix module parameter quote handling.
> Module parameter strings (with spaces) are quoted like so:
> "modprm=this test"
> and not like this:
> modprm="this test"

Well, the quote handling in insmod was ripped out after 3.0, exactly
because it was broken like this.  But modprobe will use the latter form,
since it will paste it straight from the modprobe.conf file (which needs
quotes in options lines).

Hope that clarifies,
Rusty.
PS. module-init-tools 3.1 just out...
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman