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);
In message <[email protected]> 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);
Rusty Russell <[email protected]> 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'
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~
In message <[email protected]> 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.
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?
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
In message <[email protected]> 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);
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
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 <[email protected]>
CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.
In message <[email protected]> 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.