* Re: two 2.5 modules bugs
@ 2002-12-30 11:39 Rusty Russell
0 siblings, 0 replies; 6+ messages in thread
From: Rusty Russell @ 2002-12-30 11:39 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: linux-kernel, torvalds
> In message <200212292018.VAA25446@harpo.it.uu.se> you write:
> > >long". The obvious fix (untested) is:
> > >
> > Tested. This patch makes the parport_pc module work again. Thanks.
>
> Linus, please apply. Mikael, thanks for the excellent bug report!
>
> Rusty.
> --
> Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
>
> Name: Fix MODULE_PARM for arrays of s.
And this as well. We restore the ","s after parsing: if expect to
keep pointers to this stuff, we must not do that.
Linus, please apply.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .5399-linux-2.5-bk/kernel/params.c .5399-linux-2.5-bk.updated/kernel/params.c
--- .5399-linux-2.5-bk/kernel/params.c 2002-12-26 15:41:06.000000000 +1100
+++ .5399-linux-2.5-bk.updated/kernel/params.c 2002-12-30 20:30:38.000000000 +1100
@@ -233,6 +233,7 @@ int param_array(const char *name,
int ret;
unsigned int count = 0;
struct kernel_param kp;
+ char save;
/* Get the name right for errors. */
kp.name = name;
@@ -247,7 +248,6 @@ int param_array(const char *name,
/* We expect a comma-separated list of values. */
do {
int len;
- char save;
if (count > max) {
printk(KERN_ERR "%s: can only take %i arguments\n",
@@ -256,18 +256,17 @@ int param_array(const char *name,
}
len = strcspn(val, ",");
- /* Temporarily nul-terminate and parse */
+ /* nul-terminate and parse */
save = val[len];
((char *)val)[len] = '\0';
ret = set(val, &kp);
- ((char *)val)[len] = save;
if (ret != 0)
return ret;
kp.arg += elemsize;
val += len+1;
count++;
- } while (val[-1] == ',');
+ } while (save == ',');
if (count < min) {
printk(KERN_ERR "%s: needs at least %i arguments\n",
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: two 2.5 modules bugs
2002-12-29 20:18 Mikael Pettersson
@ 2002-12-30 6:13 ` Rusty Russell
0 siblings, 0 replies; 6+ messages in thread
From: Rusty Russell @ 2002-12-30 6:13 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: linux-kernel, torvalds
In message <200212292018.VAA25446@harpo.it.uu.se> you write:
> >long". The obvious fix (untested) is:
> >
> Tested. This patch makes the parport_pc module work again. Thanks.
Linus, please apply. Mikael, thanks for the excellent bug report!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Name: Fix MODULE_PARM for arrays of s.
Author: Rusty Russell
Status: Tested on 2.5.53 (by Mikael Pettersson)
D: I interpreted "1-10s" to mean a string of 1-10 chars. It actually
D: means 1-10 comma-separated strings.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .748-linux-2.5-bk/kernel/module.c .748-linux-2.5-bk.updated/kernel/module.c
--- .748-linux-2.5-bk/kernel/module.c 2002-12-30 15:30:15.000000000 +1100
+++ .748-linux-2.5-bk.updated/kernel/module.c 2002-12-30 17:11:37.000000000 +1100
@@ -569,20 +569,6 @@ static int param_set_byte(const char *va
return 0;
}
-static int param_string(const char *name, const char *val,
- unsigned int min, unsigned int max,
- char *dest)
-{
- if (strlen(val) < min || strlen(val) > max) {
- printk(KERN_ERR
- "Parameter %s length must be %u-%u characters\n",
- name, min, max);
- return -EINVAL;
- }
- strcpy(dest, val);
- return 0;
-}
-
extern int set_obsolete(const char *val, struct kernel_param *kp)
{
unsigned int min, max;
@@ -618,7 +604,8 @@ extern int set_obsolete(const char *val,
return param_array(kp->name, val, min, max, obsparm->addr,
sizeof(long), param_set_long);
case 's':
- return param_string(kp->name, val, min, max, obsparm->addr);
+ return param_array(kp->name, val, min, max, obsparm->addr,
+ sizeof(char *), param_set_charp);
}
printk(KERN_ERR "Unknown obsolete parameter type %s\n", obsparm->type);
return -EINVAL;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: two 2.5 modules bugs
@ 2002-12-29 20:18 Mikael Pettersson
2002-12-30 6:13 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Mikael Pettersson @ 2002-12-29 20:18 UTC (permalink / raw)
To: rusty; +Cc: linux-kernel
On Sat, 28 Dec 2002 21:37:22 +1100, Rusty Russell wrote:
>> 1. With kernel 2.5.53 and module-init-tools-0.9.6, "modprobe tulip"
>> fails and goes into an infinite CPU-consuming loop. The problem
>> appears to be related to the dependency from tulip to crc32. If I
>> manually modprobe crc32 before modprobe tulip, it works. If crc32
>> isn't loaded, modprobe tulip first loads crc32 and then loops.
>>
>> module-init-tools-0.9.5 did not have this problem.
>
>This should be fixed in 0.9.6: a double free caused all kinds of wierd
>behavior. Please tell me if this fixes it.
module-init-tools-0.9.7 fixed the problem.
>Ew. I horribly misinterpreted "1-16s" to mean "a string 1-16 chars
>long". The obvious fix (untested) is:
>
>diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.53/kernel/module.c working-2.5.53-sparam/kernel/module.c
>--- linux-2.5.53/kernel/module.c 2002-12-26 15:41:06.000000000 +1100
>+++ working-2.5.53-sparam/kernel/module.c 2002-12-28 21:32:34.000000000 +1100
>@@ -604,7 +604,8 @@ extern int set_obsolete(const char *val,
> return param_array(kp->name, val, min, max, obsparm->addr,
> sizeof(long), param_set_long);
> case 's':
>- return param_string(kp->name, val, min, max, obsparm->addr);
>+ return param_array(kp->name, val, min, max, obsparm->addr,
>+ sizeof(char *), param_set_charp);
> }
> printk(KERN_ERR "Unknown obsolete parameter type %s\n", obsparm->type);
> return -EINVAL;
Tested. This patch makes the parport_pc module work again. Thanks.
/Mikael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: two 2.5 modules bugs
2002-12-27 16:16 Mikael Pettersson
2002-12-27 23:24 ` Petr Vandrovec
@ 2002-12-28 10:37 ` Rusty Russell
1 sibling, 0 replies; 6+ messages in thread
From: Rusty Russell @ 2002-12-28 10:37 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: linux-kernel
In message <200212271616.RAA03356@harpo.it.uu.se> you write:
> 1. With kernel 2.5.53 and module-init-tools-0.9.6, "modprobe tulip"
> fails and goes into an infinite CPU-consuming loop. The problem
> appears to be related to the dependency from tulip to crc32. If I
> manually modprobe crc32 before modprobe tulip, it works. If crc32
> isn't loaded, modprobe tulip first loads crc32 and then loops.
>
> module-init-tools-0.9.5 did not have this problem.
This should be fixed in 0.9.6: a double free caused all kinds of wierd
behavior. Please tell me if this fixes it.
> 2. The implementation of old-style MODULE_PARMs with type "1-16s"
> is broken. Instead of splicing the parameter at the commas and
> storing pointers to the substrings in consecutive array elements,
> the whole string is stored in the array instead.
>
> Consider parport_pc.c, which contains (simplified):
>
> static const char *irq[16];
> MODULE_PARM(irq, "1-16s");
>
> "modprobe parport_pc irq=007" should store a pointer to "007" in
> irq[0], but instead (unsigned int)irq[0] == 0x00373030, the ASCII
> representation of "007" in little-endian. (Kernel 2.5.53 on x86,
> with module-init-tools-0.9.[56].)
Ew. I horribly misinterpreted "1-16s" to mean "a string 1-16 chars
long". The obvious fix (untested) is:
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.53/kernel/module.c working-2.5.53-sparam/kernel/module.c
--- linux-2.5.53/kernel/module.c 2002-12-26 15:41:06.000000000 +1100
+++ working-2.5.53-sparam/kernel/module.c 2002-12-28 21:32:34.000000000 +1100
@@ -604,7 +604,8 @@ extern int set_obsolete(const char *val,
return param_array(kp->name, val, min, max, obsparm->addr,
sizeof(long), param_set_long);
case 's':
- return param_string(kp->name, val, min, max, obsparm->addr);
+ return param_array(kp->name, val, min, max, obsparm->addr,
+ sizeof(char *), param_set_charp);
}
printk(KERN_ERR "Unknown obsolete parameter type %s\n", obsparm->type);
return -EINVAL;
I'll test this tomorrow...
Thanks for the bug report!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: two 2.5 modules bugs
2002-12-27 16:16 Mikael Pettersson
@ 2002-12-27 23:24 ` Petr Vandrovec
2002-12-28 10:37 ` Rusty Russell
1 sibling, 0 replies; 6+ messages in thread
From: Petr Vandrovec @ 2002-12-27 23:24 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: rusty, linux-kernel
On Fri, Dec 27, 2002 at 05:16:35PM +0100, Mikael Pettersson wrote:
> 1. With kernel 2.5.53 and module-init-tools-0.9.6, "modprobe tulip"
> fails and goes into an infinite CPU-consuming loop. The problem
> appears to be related to the dependency from tulip to crc32. If I
> manually modprobe crc32 before modprobe tulip, it works. If crc32
> isn't loaded, modprobe tulip first loads crc32 and then loops.
>
> module-init-tools-0.9.5 did not have this problem.
Load modprobe with MALLOC_CHECK_=1. It will reveal that it tries to
free() some non-mallocated area.
Try patch below, insmod() can realloc/free its second argument, so
we can doublefree it, and glibc's malloc goes wild. It fixes problems
I had with modprobe ipx (which depends on psnap/p8022 which depends
on llc).
--- 0.9.6-1/modprobe.c.dist 2002-12-26 10:32:22.000000000 +0100
+++ 0.9.6-1/modprobe.c 2002-12-28 00:12:16.000000000 +0100
@@ -582,7 +582,7 @@
char *baseopts = NOFAIL(strdup(""));
insmod(list, baseopts, NULL, 0, dry_run, verbose, options,
commands, 0);
- free(baseopts);
+// free(baseopts);
}
/* Did config file override command or add options? */
Petr Vandrovec
vandrove@vc.cvut.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
* two 2.5 modules bugs
@ 2002-12-27 16:16 Mikael Pettersson
2002-12-27 23:24 ` Petr Vandrovec
2002-12-28 10:37 ` Rusty Russell
0 siblings, 2 replies; 6+ messages in thread
From: Mikael Pettersson @ 2002-12-27 16:16 UTC (permalink / raw)
To: rusty; +Cc: linux-kernel
1. With kernel 2.5.53 and module-init-tools-0.9.6, "modprobe tulip"
fails and goes into an infinite CPU-consuming loop. The problem
appears to be related to the dependency from tulip to crc32. If I
manually modprobe crc32 before modprobe tulip, it works. If crc32
isn't loaded, modprobe tulip first loads crc32 and then loops.
module-init-tools-0.9.5 did not have this problem.
2. The implementation of old-style MODULE_PARMs with type "1-16s"
is broken. Instead of splicing the parameter at the commas and
storing pointers to the substrings in consecutive array elements,
the whole string is stored in the array instead.
Consider parport_pc.c, which contains (simplified):
static const char *irq[16];
MODULE_PARM(irq, "1-16s");
"modprobe parport_pc irq=007" should store a pointer to "007" in
irq[0], but instead (unsigned int)irq[0] == 0x00373030, the ASCII
representation of "007" in little-endian. (Kernel 2.5.53 on x86,
with module-init-tools-0.9.[56].)
/Mikael
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-12-30 11:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-12-30 11:39 two 2.5 modules bugs Rusty Russell
-- strict thread matches above, loose matches on Subject: below --
2002-12-29 20:18 Mikael Pettersson
2002-12-30 6:13 ` Rusty Russell
2002-12-27 16:16 Mikael Pettersson
2002-12-27 23:24 ` Petr Vandrovec
2002-12-28 10:37 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome