param: fix charp parameters set via sysfs

Impact: fix crash on reading from /sys/module/.../ieee80211_default_rc_algo The module_param type "charp" simply sets a char * pointer in the module to the parameter in the commandline string: this is why we keep the (mangled) module command line around. But when set via sysfs (as about 11 charp parameters can be) this memory is freed on the way out of the write(). Future reads hit random mem. So we kstrdup instead: we have to check we're not in early commandline parsing, and we have to note when we've used it so we can reliably kfree the parameter when it's next overwritten, and also on module unload. (Thanks to Randy Dunlap for CONFIG_SYSFS=n fixes) Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com> Diagnosed-by: Frederic Weisbecker <fweisbec@gmail.com> Tested-by: Frederic Weisbecker <fweisbec@gmail.com> Tested-by: Christof Schmitt <christof.schmitt@de.ibm.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

param: fix charp parameters set via sysfs
Impact: fix crash on reading from /sys/module/.../ieee80211_default_rc_algo The module_param type "charp" simply sets a char * pointer in the module to the parameter in the commandline string: this is why we keep the (mangled) module command line around. But when set via sysfs (as about 11 charp parameters can be) this memory is freed on the way out of the write(). Future reads hit random mem. So we kstrdup instead: we have to check we're not in early commandline parsing, and we have to note when we've used it so we can reliably kfree the parameter when it's next overwritten, and also on module unload. (Thanks to Randy Dunlap for CONFIG_SYSFS=n fixes) Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com> Diagnosed-by: Frederic Weisbecker <fweisbec@gmail.com> Tested-by: Frederic Weisbecker <fweisbec@gmail.com> Tested-by: Christof Schmitt <christof.schmitt@de.ibm.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
1 parent 15f7176eb1
Showing 4 changed files with 47 additions and 7 deletions Side-by-side Diff
include/linux/module.h
include/linux/moduleparam.h
kernel/module.c
kernel/params.c
@@ -248,6 +248,10 @@
 	const unsigned long *crcs;
 	unsigned int num_syms;
  
+	/* Kernel parameters. */
+	struct kernel_param *kp;
+	unsigned int num_kp;
+
 	/* GPL-only exported symbols. */
 	unsigned int num_gpl_syms;
 	const struct kernel_symbol *gpl_syms;
@@ -138,6 +138,16 @@
 		      unsigned num,
 		      int (*unknown)(char *param, char *val));
  
+/* Called by module remove. */
+#ifdef CONFIG_SYSFS
+extern void destroy_params(const struct kernel_param *params, unsigned num);
+#else
+static inline void destroy_params(const struct kernel_param *params,
+				  unsigned num)
+{
+}
+#endif /* !CONFIG_SYSFS */
+
 /* All the helper functions */
 /* The macros to do compile-time type checking stolen from Jakub
    Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
@@ -1491,6 +1491,9 @@
 	/* Module unload stuff */
 	module_unload_free(mod);
  
+	/* Free any allocated parameters. */
+	destroy_params(mod->kp, mod->num_kp);
+
 	/* release any pointers to mcount in this module */
 	ftrace_release(mod->module_core, mod->core_size);
  
@@ -1898,8 +1901,7 @@
 	unsigned int symindex = 0;
 	unsigned int strindex = 0;
 	unsigned int modindex, versindex, infoindex, pcpuindex;
-	unsigned int num_kp, num_mcount;
-	struct kernel_param *kp;
+	unsigned int num_mcount;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -2144,8 +2146,8 @@
  
 	/* Now we've got everything in the final locations, we can
 	 * find optional sections. */
-	kp = section_objs(hdr, sechdrs, secstrings, "__param", sizeof(*kp),
-			  &num_kp);
+	mod->kp = section_objs(hdr, sechdrs, secstrings, "__param",
+			       sizeof(*mod->kp), &mod->num_kp);
 	mod->syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab",
 				 sizeof(*mod->syms), &mod->num_syms);
 	mod->crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab");
  
@@ -2291,11 +2293,11 @@
 	 */
 	list_add_rcu(&mod->list, &modules);
  
-	err = parse_args(mod->name, mod->args, kp, num_kp, NULL);
+	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
 	if (err < 0)
 		goto unlink;
  
-	err = mod_sysfs_setup(mod, kp, num_kp);
+	err = mod_sysfs_setup(mod, mod->kp, mod->num_kp);
 	if (err < 0)
 		goto unlink;
 	add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
@@ -24,6 +24,9 @@
 #include <linux/err.h>
 #include <linux/slab.h>
  
+/* We abuse the high bits of "perm" to record whether we kmalloc'ed. */
+#define KPARAM_KMALLOCED	0x80000000
+
 #if 0
 #define DEBUGP printk
 #else
@@ -217,7 +220,19 @@
 		return -ENOSPC;
 	}
  
-	*(char **)kp->arg = (char *)val;
+	if (kp->perm & 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->perm |= KPARAM_KMALLOCED;
+		*(char **)kp->arg = kstrdup(val, GFP_KERNEL);
+		if (!kp->arg)
+			return -ENOMEM;
+	} else
+		*(const char **)kp->arg = val;
+
 	return 0;
 }
  
@@ -570,6 +585,15 @@
 	}
 }
 #endif
+
+void destroy_params(const struct kernel_param *params, unsigned num)
+{
+	unsigned int i;
+
+	for (i = 0; i < num; i++)
+		if (params[i].perm & KPARAM_KMALLOCED)
+			kfree(*(char **)params[i].arg);
+}
  
 static void __init kernel_add_sysfs_param(const char *name,
 					  struct kernel_param *kparam,
...	...	@@ -248,6 +248,10 @@
248	248	const unsigned long *crcs;
249	249	unsigned int num_syms;
250	250
	251	+ /* Kernel parameters. */
	252	+ struct kernel_param *kp;
	253	+ unsigned int num_kp;
	254	+
251	255	/* GPL-only exported symbols. */
252	256	unsigned int num_gpl_syms;
253	257	const struct kernel_symbol *gpl_syms;
...	...	@@ -138,6 +138,16 @@
138	138	unsigned num,
139	139	int (unknown)(char param, char *val));
140	140
	141	+/* Called by module remove. */
	142	+#ifdef CONFIG_SYSFS
	143	+extern void destroy_params(const struct kernel_param *params, unsigned num);
	144	+#else
	145	+static inline void destroy_params(const struct kernel_param *params,
	146	+ unsigned num)
	147	+{
	148	+}
	149	+#endif /* !CONFIG_SYSFS */
	150	+
141	151	/* All the helper functions */
142	152	/* The macros to do compile-time type checking stolen from Jakub
143	153	Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
...	...	@@ -1491,6 +1491,9 @@
1491	1491	/* Module unload stuff */
1492	1492	module_unload_free(mod);
1493	1493
	1494	+ /* Free any allocated parameters. */
	1495	+ destroy_params(mod->kp, mod->num_kp);
	1496	+
1494	1497	/* release any pointers to mcount in this module */
1495	1498	ftrace_release(mod->module_core, mod->core_size);
1496	1499
...	...	@@ -1898,8 +1901,7 @@
1898	1901	unsigned int symindex = 0;
1899	1902	unsigned int strindex = 0;
1900	1903	unsigned int modindex, versindex, infoindex, pcpuindex;
1901		- unsigned int num_kp, num_mcount;
1902		- struct kernel_param *kp;
	1904	+ unsigned int num_mcount;
1903	1905	struct module *mod;
1904	1906	long err = 0;
1905	1907	void percpu = NULL, ptr = NULL; /* Stops spurious gcc warning */
...	...	@@ -2144,8 +2146,8 @@
2144	2146
2145	2147	/* Now we've got everything in the final locations, we can
2146	2148	* find optional sections. */
2147		- kp = section_objs(hdr, sechdrs, secstrings, "__param", sizeof(*kp),
2148		- &num_kp);
	2149	+ mod->kp = section_objs(hdr, sechdrs, secstrings, "__param",
	2150	+ sizeof(*mod->kp), &mod->num_kp);
2149	2151	mod->syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab",
2150	2152	sizeof(*mod->syms), &mod->num_syms);
2151	2153	mod->crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab");
2152	2154
...	...	@@ -2291,11 +2293,11 @@
2291	2293	*/
2292	2294	list_add_rcu(&mod->list, &modules);
2293	2295
2294		- err = parse_args(mod->name, mod->args, kp, num_kp, NULL);
	2296	+ err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL);
2295	2297	if (err < 0)
2296	2298	goto unlink;
2297	2299
2298		- err = mod_sysfs_setup(mod, kp, num_kp);
	2300	+ err = mod_sysfs_setup(mod, mod->kp, mod->num_kp);
2299	2301	if (err < 0)
2300	2302	goto unlink;
2301	2303	add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
...	...	@@ -24,6 +24,9 @@
24	24	#include <linux/err.h>
25	25	#include <linux/slab.h>
26	26
	27	+/* We abuse the high bits of "perm" to record whether we kmalloc'ed. */
	28	+#define KPARAM_KMALLOCED 0x80000000
	29	+
27	30	#if 0
28	31	#define DEBUGP printk
29	32	#else
...	...	@@ -217,7 +220,19 @@
217	220	return -ENOSPC;
218	221	}
219	222
220		- (char )kp->arg = (char )val;
	223	+ if (kp->perm & KPARAM_KMALLOCED)
	224	+ kfree((char *)kp->arg);
	225	+
	226	+ /* This is a hack. We can't need to strdup in early boot, and we
	227	+ * don't need to; this mangled commandline is preserved. */
	228	+ if (slab_is_available()) {
	229	+ kp->perm \|= KPARAM_KMALLOCED;
	230	+ (char *)kp->arg = kstrdup(val, GFP_KERNEL);
	231	+ if (!kp->arg)
	232	+ return -ENOMEM;
	233	+ } else
	234	+ (const char *)kp->arg = val;
	235	+
221	236	return 0;
222	237	}
223	238
...	...	@@ -570,6 +585,15 @@
570	585	}
571	586	}
572	587	#endif
	588	+
	589	+void destroy_params(const struct kernel_param *params, unsigned num)
	590	+{
	591	+ unsigned int i;
	592	+
	593	+ for (i = 0; i < num; i++)
	594	+ if (params[i].perm & KPARAM_KMALLOCED)
	595	+ kfree((char *)params[i].arg);
	596	+}
573	597
574	598	static void __init kernel_add_sysfs_param(const char *name,
575	599	struct kernel_param *kparam,