The following commit has been merged in the linux branch: commit 65afac7d80ab3bc9f81e75eafb71eeb92a3ebdef Author: Rusty Russell rusty@rustcorp.com.au Date: Thu Oct 29 08:56:16 2009 -0600

param: fix lots of bugs with writing charp params from sysfs, by leaking mem.

e180a6b7759a "param: fix charp parameters set via sysfs" fixed the case where charp parameters written via sysfs were freed, leaving drivers accessing random memory.

Unfortunately, storing a flag in the kparam struct was a bad idea: it's rodata so setting it causes an oops on some archs. But that's not all:

1) module_param_array() on charp doesn't work reliably, since we use an uninitialized temporary struct kernel_param. 2) there's a fundamental race if a module uses this parameter and then it's changed: they will still access the old, freed, memory.

The simplest fix (ie. for 2.6.32) is to never free the memory. This prevents all these problems, at cost of a memory leak. In practice, there are only 18 places where a charp is writable via sysfs, and all are root-only writable.

Reported-by: Takashi Iwai tiwai@suse.de Cc: Sitsofe Wheeler sitsofe@yahoo.com Cc: Frederic Weisbecker fweisbec@gmail.com Cc: Christof Schmitt christof.schmitt@de.ibm.com Signed-off-by: Rusty Russell rusty@rustcorp.com.au Cc: stable@kernel.org

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 6547c3c..82a9124 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -37,7 +37,6 @@ typedef int (*param_set_fn)(const char *val, struct kernel_param *kp); typedef int (*param_get_fn)(char *buffer, struct kernel_param *kp);

/* Flag bits for kernel_param.flags */ -#define KPARAM_KMALLOCED 1 #define KPARAM_ISBOOL 2

struct kernel_param { diff --git a/kernel/params.c b/kernel/params.c index 9da58ea..95ef27c 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -218,13 +218,9 @@ int param_set_charp(const char *val, struct kernel_param *kp) return -ENOSPC; }

- if (kp->flags & KPARAM_KMALLOCED) - kfree(*(char **)kp->arg); - /* This is a hack. We can't need to strdup in early boot, and we * don't need to; this mangled commandline is preserved. */ if (slab_is_available()) { - kp->flags |= KPARAM_KMALLOCED; *(char **)kp->arg = kstrdup(val, GFP_KERNEL); if (!kp->arg) return -ENOMEM; @@ -605,11 +601,7 @@ void module_param_sysfs_remove(struct module *mod)

void destroy_params(const struct kernel_param *params, unsigned num) { - unsigned int i; - - for (i = 0; i < num; i++) - if (params[i].flags & KPARAM_KMALLOCED) - kfree(*(char **)params[i].arg); + /* FIXME: This should free kmalloced charp parameters. It doesn't. */ }

static void __init kernel_add_sysfs_param(const char *name,