[PATCH] kernel/module.c - fix warning and reduce stack usage - reintroduction of mistakenly dropped patch

[PATCH] kernel/module.c - fix warning and reduce stack usage - reintroduction of mistakenly dropped patch

[Jesper Juhl]

Hi Andrew,

Reading your 2.6.12-rc1-mm4 announce text I see

...
-figure-out-who-is-inserting-bogus-modules-warning-fix.patch

 Folded into figure-out-who-is-inserting-bogus-modules.patch
...
figure-out-who-is-inserting-bogus-modules.patch
  Figure out who is inserting bogus modules
...

However, it seems you did *not* roll 
figure-out-who-is-inserting-bogus-modules-warning-fix.patch into 
figure-out-who-is-inserting-bogus-modules.patch but instead just dropped 
the patch.

It also turns out I had a small boundary error (off-by-one) in my original 
patch which Yum Rayan spotted and fixed in a patch he wrote that also 
reduces the stack usage of the function (by dynamically allocating the 
needed mem for 'args' instead of using a static 512 byte array - see the 
LKML thread with subject "[PATCH] Reduce stack usage in module.c"), so 
below you'll find an updated patch that reintroduce 
figure-out-who-is-inserting-bogus-modules-warning-fix.patch on top of 
2.6.12-rc1-mm4 and includes Yum Rayan's fix for the off-by-one and also 
adopts his use of kmalloc() instead of large static array.

Yum Rayan's patch does more than what I've included below, I've only 
included his changes to the who_is_doing_it() function, if you want the 
rest of his changes then please see the original thread for his full 
patch.

Here's the updated figure-out-who-is-inserting-bogus-modules-warning-fix.patch

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

--- linux-2.6.12-rc1-mm4-orig/kernel/module.c	2005-03-31 21:20:07.000000000 +0200
+++ linux-2.6.12-rc1-mm4/kernel/module.c	2005-04-03 01:54:30.000000000 +0200
@@ -1399,10 +1399,19 @@ static inline void add_kallsyms(struct m
 static void who_is_doing_it(void)
 {
 	/* Print out all the args. */
-	char args[512];
-	unsigned int i, len = current->mm->arg_end - current->mm->arg_start;
+	char *args;
+	unsigned long i, len = current->mm->arg_end - current->mm->arg_start;
 
-	copy_from_user(args, (void *)current->mm->arg_start, len);
+	if (len > 512)
+		len = 512;
+
+	args = kmalloc(len + 1, GFP_KERNEL);
+	if (!args) {
+		printk(KERN_WARNING "Unable to allocate memory, can't print who's inserting bogus modules\n");
+		return;
+	}
+
+	len -= copy_from_user(args, (void *)current->mm->arg_start, len);
 
 	for (i = 0; i < len; i++) {
 		if (args[i] == '\0')
@@ -1410,6 +1419,7 @@ static void who_is_doing_it(void)
 	}
 	args[i] = 0;
 	printk("ARGS: %s\n", args);
+	kfree(args);
 }
 
 /* Allocate and load the module: note that size of section 0 is always

Re: [PATCH] kernel/module.c - fix warning and reduce stack usage - reintroduction of mistakenly dropped patch

[Andrew Morton]

Jesper Juhl <juhl-lkml@dif.dk> wrote:
>
> However, it seems you did *not* roll 
>  figure-out-who-is-inserting-bogus-modules-warning-fix.patch into 
>  figure-out-who-is-inserting-bogus-modules.patch but instead just dropped 
>  the patch.

I meant to drop it - it was just a debug thing, and we fixed the bug ages
ago and people kept on trying to fix the debug patch.

Re: [PATCH] kernel/module.c - fix warning and reduce stack usage - reintroduction of mistakenly dropped patch

[Jesper Juhl]

On Sat, 2 Apr 2005, Andrew Morton wrote:

> Jesper Juhl <juhl-lkml@dif.dk> wrote:
> >
> > However, it seems you did *not* roll 
> >  figure-out-who-is-inserting-bogus-modules-warning-fix.patch into 
> >  figure-out-who-is-inserting-bogus-modules.patch but instead just dropped 
> >  the patch.
> 
> I meant to drop it - it was just a debug thing, and we fixed the bug ages
> ago and people kept on trying to fix the debug patch.
> 
Ohh, I see. Then please disregard the mail I just send with an alternative 
version.

-- 
Jesper

Re: [PATCH] kernel/module.c - fix warning and reduce stack usage - reintroduction of mistakenly dropped patch

[Jesper Juhl]

On Sun, 3 Apr 2005, Jesper Juhl wrote:

> 
> Hi Andrew,
> 
> Reading your 2.6.12-rc1-mm4 announce text I see
> 
> ...
> -figure-out-who-is-inserting-bogus-modules-warning-fix.patch
> 
>  Folded into figure-out-who-is-inserting-bogus-modules.patch
> ...
> figure-out-who-is-inserting-bogus-modules.patch
>   Figure out who is inserting bogus modules
> ...
> 
> However, it seems you did *not* roll 
> figure-out-who-is-inserting-bogus-modules-warning-fix.patch into 
> figure-out-who-is-inserting-bogus-modules.patch but instead just dropped 
> the patch.
> 
> It also turns out I had a small boundary error (off-by-one) in my original 
> patch which Yum Rayan spotted and fixed in a patch he wrote that also 
> reduces the stack usage of the function (by dynamically allocating the 
> needed mem for 'args' instead of using a static 512 byte array - see the 
> LKML thread with subject "[PATCH] Reduce stack usage in module.c"), so 
> below you'll find an updated patch that reintroduce 
> figure-out-who-is-inserting-bogus-modules-warning-fix.patch on top of 
> 2.6.12-rc1-mm4 and includes Yum Rayan's fix for the off-by-one and also 
> adopts his use of kmalloc() instead of large static array.
> 
> Yum Rayan's patch does more than what I've included below, I've only 
> included his changes to the who_is_doing_it() function, if you want the 
> rest of his changes then please see the original thread for his full 
> patch.
> 
> Here's the updated figure-out-who-is-inserting-bogus-modules-warning-fix.patch
> 
In case you prefer the static array, so the function will never fail due 
to OOM, then here's an alternative version of my original patch /with/ the 
static array but /without/ the off-by-one error  :

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

--- linux-2.6.12-rc1-mm4-orig/kernel/module.c	2005-03-31 21:20:07.000000000 +0200
+++ linux-2.6.12-rc1-mm4/kernel/module.c	2005-04-03 02:04:39.000000000 +0200
@@ -1400,9 +1400,12 @@ static void who_is_doing_it(void)
 {
 	/* Print out all the args. */
 	char args[512];
-	unsigned int i, len = current->mm->arg_end - current->mm->arg_start;
+	unsigned long i, len = current->mm->arg_end - current->mm->arg_start;
 
-	copy_from_user(args, (void *)current->mm->arg_start, len);
+	if (len > 511)
+		len = 511;
+
+	len -= copy_from_user(args, (void *)current->mm->arg_start, len);
 
 	for (i = 0; i < len; i++) {
 		if (args[i] == '\0')