[PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Miles Bader]

Since these are just symbols in the module object, they need symbol name
munging to find the symbol from the parameter name.

[I guess using the stack is bad in general, but parameter names should be
very short, and hey if they're obsolete, it seems pointless to spend
much effort.]

diff -ruN -X../cludes linux-2.5.55-moo.orig/kernel/module.c linux-2.5.55-moo/kernel/module.c
--- linux-2.5.55-moo.orig/kernel/module.c	2003-01-09 13:44:25.000000000 +0900
+++ linux-2.5.55-moo/kernel/module.c	2003-01-09 14:07:36.000000000 +0900
@@ -685,13 +685,18 @@
 		       num, obsparm[i].name, obsparm[i].type);
 
 	for (i = 0; i < num; i++) {
+		char sym_name[strlen (obsparm[i].name) + 2];
+
+		strcpy (sym_name, MODULE_SYMBOL_PREFIX);
+		strcat (sym_name, obsparm[i].name);
+
 		kp[i].name = obsparm[i].name;
 		kp[i].perm = 000;
 		kp[i].set = set_obsolete;
 		kp[i].get = NULL;
 		obsparm[i].addr
 			= (void *)find_local_symbol(sechdrs, symindex, strtab,
-						    obsparm[i].name);
+						    sym_name);
 		if (!obsparm[i].addr) {
 			printk("%s: falsely claims to have parameter %s\n",
 			       name, obsparm[i].name);

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Rusty Russell]

In message <20030109094923.46A933745@mcspd15.ucom.lsi.nec.co.jp> you write:
> Since these are just symbols in the module object, they need symbol name
> munging to find the symbol from the parameter name.

I've got this one already pending (I managed to pick up mail once
while travelling to a funeral, but generally I've been offline for 4
days).

Thanks!
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Bill Davidsen]

On 10 Jan 2003, Miles Bader wrote:

> -- 
> `The suburb is an obsolete and contradictory form of human settlement'

Suburbs are a clever solution to the problem of combining high price, high
taxes, having nothing useful within walking distance, and lack of privacy. 

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Rusty Russell]

In message <Pine.LNX.4.44.0301102134150.9532-100000@home.transmeta.com> you wri
te:
> 
> On Sat, 11 Jan 2003, Rusty Russell wrote:
> > 
> > Just in case someone names a variable over 2000 chars, and uses it as
> > an old-style module parameter?
> 
> No. Just because variable-sized arrays aren't C, and generate crappy code.
> 
> >  	for (i = 0; i < num; i++) {
> > +		char sym_name[strlen(obsparm[i].name)
> > +			     + sizeof(MODULE_SYMBOL_PREFIX)];
> 
> It's still there.

OK, *please* explain to me in little words so I can understand.

Variable-sized arrays are C, as of C99.  They've been a GNU extension
forever.

While gcc 2.95.4 generates fairly horrible code, gcc 3.0 does better
(the two compilers I have on my laptop).

Both generate correct code.

Speed is certainly of absolutely no importance here.

Changing it to do a kmalloc every time around the loop is complex,
inefficient, unneccessary, and introduces another failure path.

In summary, I can't see any reason why the clearest, simplest code
should be avoided.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Linus Torvalds]

On Sat, 11 Jan 2003, Rusty Russell wrote:
> 
> Just in case someone names a variable over 2000 chars, and uses it as
> an old-style module parameter?

No. Just because variable-sized arrays aren't C, and generate crappy code.

>  	for (i = 0; i < num; i++) {
> +		char sym_name[strlen(obsparm[i].name)
> +			     + sizeof(MODULE_SYMBOL_PREFIX)];

It's still there.

		Linus

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Rusty Russell]

In message <Pine.LNX.4.44.0301100902380.12833-100000@home.transmeta.com> you wr
ite:
> 
> On Fri, 10 Jan 2003, Rusty Russell wrote:
> > 
> > Yep.  Maximum length of obsolete parameter name in current kernel:
> > seq_default_timer_resolution (28 chars).
> 
> Don't do this. Make the limit fixed, and check it.

Just in case someone names a variable over 2000 chars, and uses it as
an old-style module parameter?

Done, with protest.  Please apply.

Rusty.
(Miles, your sizeof also merged).
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: Make obsolete module parameters work with MODULE_SYMBOL_PREFIX
Author: Miles Bader
Status: Trivial

D: Since these are just symbols in the module object, they need symbol name
D: munging to find the symbol from the parameter name.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15543-linux-2.5-bk/kernel/module.c .15543-linux-2.5-bk.updated/kernel/module.c
--- .15543-linux-2.5-bk/kernel/module.c	2003-01-10 10:55:43.000000000 +1100
+++ .15543-linux-2.5-bk.updated/kernel/module.c	2003-01-11 15:15:59.000000000 +1100
@@ -680,18 +680,31 @@ static int obsolete_params(const char *n
 		return -ENOMEM;
 
 	DEBUGP("Module %s has %u obsolete params\n", name, num);
-	for (i = 0; i < num; i++)
+	for (i = 0; i < num; i++) {
+		if (strlen(obsparm[i].name) > 1024) {
+			printk("%s: Huge parameter.  Linus 1, Rusty 0.\n",
+			       name);
+			ret = -EINVAL;
+			goto out;
+		}
 		DEBUGP("Param %i: %s type %s\n",
 		       num, obsparm[i].name, obsparm[i].type);
+	}
 
 	for (i = 0; i < num; i++) {
+		char sym_name[strlen(obsparm[i].name)
+			     + sizeof(MODULE_SYMBOL_PREFIX)];
+
+		strcpy(sym_name, MODULE_SYMBOL_PREFIX);
+		strcat(sym_name, obsparm[i].name);
+
 		kp[i].name = obsparm[i].name;
 		kp[i].perm = 000;
 		kp[i].set = set_obsolete;
 		kp[i].get = NULL;
 		obsparm[i].addr
 			= (void *)find_local_symbol(sechdrs, symindex, strtab,
-						    obsparm[i].name);
+						    sym_name);
 		if (!obsparm[i].addr) {
 			printk("%s: falsely claims to have parameter %s\n",
 			       name, obsparm[i].name);

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Linus Torvalds]

On Fri, 10 Jan 2003, Rusty Russell wrote:
> 
> Yep.  Maximum length of obsolete parameter name in current kernel:
> seq_default_timer_resolution (28 chars).

Don't do this. Make the limit fixed, and check it.

		Linus

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Miles Bader]

BTW, I noticed that there's actually a bug in my patch -- it assumes
the length of MODULE_SYMBOL_PREFIX is 1.  So change:

        char sym_name[strlen(obsparm[i].name) + 2];

to:

        char sym_name[strlen(obsparm[i].name) + sizeof MODULE_SYMBOL_PREFIX];

-Miles
-- 
Would you like fries with that?

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Rusty Russell]

In message <20030110015203.A16268@twiddle.net> you write:
> On Wed, Jan 08, 2003 at 10:56:51PM +1100, Rusty Russell wrote:
> > +		char sym_name[strlen(obsparm[i].name) + 2];
> 
> Are you really intending to use variable sized allocation
> on the kernel stack?

Yep.  Maximum length of obsolete parameter name in current kernel:
seq_default_timer_resolution (28 chars).

It's far more likely that someone will hit the unchecked kmalloc
allocations in arch/alpha/kernel/modules.c 8)

Hope that helps,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Richard Henderson]

On Wed, Jan 08, 2003 at 10:56:51PM +1100, Rusty Russell wrote:
> +		char sym_name[strlen(obsparm[i].name) + 2];

Are you really intending to use variable sized allocation
on the kernel stack?


r~

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Miles Bader]

Rusty Russell <rusty@rustcorp.com.au> writes:
> I removed the spaces between the funcname and the brackets tho.

Curses, foiled again!

-Miles
-- 
`The suburb is an obsolete and contradictory form of human settlement'

Re: [PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Rusty Russell]

In message <20030107063239.F1ED73745@mcspd15.ucom.lsi.nec.co.jp> you write:
> Since these are just symbols in the module object, they need symbol name
> munging to find the symbol from the parameter name.

Good point.  Linus, please apply.

> [I guess using the stack is bad in general, but parameter names should be
> very short, and hey if they're obsolete, it seems pointless to spend
> much effort.]

Should be fine here.  I removed the spaces between the funcname and
the brackets tho.

Thanks!
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: Make obsolete module parameters work with MODULE_SYMBOL_PREFIX
Author: Miles Bader
Status: Trivial

D: Since these are just symbols in the module object, they need symbol name
D: munging to find the symbol from the parameter name.

diff -ruN -X../cludes linux-2.5.54-moo.orig/kernel/module.c linux-2.5.54-moo/kernel/module.c
--- linux-2.5.54-moo.orig/kernel/module.c	2003-01-06 10:51:20.000000000 +0900
+++ linux-2.5.54-moo/kernel/module.c	2003-01-07 14:31:53.000000000 +0900
@@ -666,13 +666,18 @@
 		       num, obsparm[i].name, obsparm[i].type);
 
 	for (i = 0; i < num; i++) {
+		char sym_name[strlen(obsparm[i].name) + 2];
+
+		strcpy(sym_name, MODULE_SYMBOL_PREFIX);
+		strcat(sym_name, obsparm[i].name);
+
 		kp[i].name = obsparm[i].name;
 		kp[i].perm = 000;
 		kp[i].set = set_obsolete;
 		kp[i].get = NULL;
 		obsparm[i].addr
 			= (void *)find_local_symbol(sechdrs, symindex, strtab,
-						    obsparm[i].name);
+						    sym_name);
 		if (!obsparm[i].addr) {
 			printk("%s: falsely claims to have parameter %s\n",
 			       name, obsparm[i].name);

[PATCH] Make `obsolete params' work correctly if MODULE_SYMBOL_PREFIX is non-empty

[Miles Bader]

Since these are just symbols in the module object, they need symbol name
munging to find the symbol from the parameter name.

[I guess using the stack is bad in general, but parameter names should be
very short, and hey if they're obsolete, it seems pointless to spend
much effort.]

diff -ruN -X../cludes linux-2.5.54-moo.orig/kernel/module.c linux-2.5.54-moo/kernel/module.c
--- linux-2.5.54-moo.orig/kernel/module.c	2003-01-06 10:51:20.000000000 +0900
+++ linux-2.5.54-moo/kernel/module.c	2003-01-07 14:31:53.000000000 +0900
@@ -666,13 +666,18 @@
 		       num, obsparm[i].name, obsparm[i].type);
 
 	for (i = 0; i < num; i++) {
+		char sym_name[strlen (obsparm[i].name) + 2];
+
+		strcpy (sym_name, MODULE_SYMBOL_PREFIX);
+		strcat (sym_name, obsparm[i].name);
+
 		kp[i].name = obsparm[i].name;
 		kp[i].perm = 000;
 		kp[i].set = set_obsolete;
 		kp[i].get = NULL;
 		obsparm[i].addr
 			= (void *)find_local_symbol(sechdrs, symindex, strtab,
-						    obsparm[i].name);
+						    sym_name);
 		if (!obsparm[i].addr) {
 			printk("%s: falsely claims to have parameter %s\n",
 			       name, obsparm[i].name);