2006-12-20 11:05:50

by Sébastien Dugué

Subject: Fix IPMI watchdog set_param_str() using kstrdup

I've wondered for some time why my IPMI watchdog stopped working. I traced it
back to the following patch:

[PATCH] ipmi: strstrip conversion

Here is a patch to fix this issue.

S?bastien.


Fix IPMI watchdog set_param_str() using kstrdup

set_param_str() cannot use kstrdup() to duplicate the parameter. That's
fine when the driver is compiled as a module but it sure is not when
built into the kernel as the kernel parameters are parsed before the
kmalloc slabs are setup.


ipmi_watchdog.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

Signed-off-by: S?bastien Dugu? <[email protected]>

Index: linux-2.6.20-rc1/drivers/char/ipmi/ipmi_watchdog.c
===================================================================
--- linux-2.6.20-rc1.orig/drivers/char/ipmi/ipmi_watchdog.c 2006-12-14 02:14:23.000000000 +0100
+++ linux-2.6.20-rc1/drivers/char/ipmi/ipmi_watchdog.c 2006-12-19 17:20:06.000000000 +0100
@@ -216,13 +216,13 @@ static int set_param_str(const char *val
{
action_fn fn = (action_fn) kp->arg;
int rv = 0;
- char *dup, *s;
+ char valcp[16];
+ char *s;

- dup = kstrdup(val, GFP_KERNEL);
- if (!dup)
- return -ENOMEM;
+ strncpy(valcp, val, 16);
+ valcp[15] = '\0';

- s = strstrip(dup);
+ s = strstrip(valcp);

down_read(&register_sem);
rv = fn(s, NULL);
@@ -235,7 +235,6 @@ static int set_param_str(const char *val

out_unlock:
up_read(&register_sem);
- kfree(dup);
return rv;
}

2006-12-20 11:19:12

by Pekka Enberg

Subject: Re: Fix IPMI watchdog set_param_str() using kstrdup

On 12/20/06, S?bastien Dugu? <[email protected]> wrote:
> set_param_str() cannot use kstrdup() to duplicate the parameter. That's
> fine when the driver is compiled as a module but it sure is not when
> built into the kernel as the kernel parameters are parsed before the
> kmalloc slabs are setup.

Aah. I wonder though, if we could defer parsing driver parameters
until kmalloc caches are up...

2006-12-20 11:32:12

by Sébastien Dugué

Subject: Re: Fix IPMI watchdog set_param_str() using kstrdup

On Wed, 20 Dec 2006 13:19:09 +0200 "Pekka Enberg" <[email protected]> wrote:

> On 12/20/06, S?bastien Dugu? <[email protected]> wrote:
> > set_param_str() cannot use kstrdup() to duplicate the parameter. That's
> > fine when the driver is compiled as a module but it sure is not when
> > built into the kernel as the kernel parameters are parsed before the
> > kmalloc slabs are setup.
>
> Aah. I wonder though, if we could defer parsing driver parameters
> until kmalloc caches are up...

From a general point of view, why not. Except that for the
watchdog case, I want it to start as soon as possible for it to be
able to reboot in case of an early crash.

S?bastien.