Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 65afac7d authored by Rusty Russell's avatar Rusty Russell
Browse files

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



e180a6b7 "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: default avatarTakashi 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: default avatarRusty Russell <rusty@rustcorp.com.au>
Cc: stable@kernel.org
parent 964fe080

include/linux/moduleparam.h

+0 −1
Original line number Diff line number Diff line
@@ -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 {

kernel/params.c

+1 −9
Original line number Diff line number Diff line
@@ -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,