From d2800800d795350435936b08afb402ed9aab1e66 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:08 -0600 Subject: [PATCH 01/22] documentation: fix erroneous email address. Hey, at least it has both l's. Reported-by: Marin Mitov Signed-off-by: Rusty Russell --- Documentation/cpu-hotplug.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt index a99d7031cdf9..45d5a217484f 100644 --- a/Documentation/cpu-hotplug.txt +++ b/Documentation/cpu-hotplug.txt @@ -2,7 +2,7 @@ Maintainers: CPU Hotplug Core: - Rusty Russell + Rusty Russell Srivatsa Vaddagiri i386: Zwane Mwaikambo From 2e9fb9953df91ef6310da22182ca8f4496907502 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:10 -0600 Subject: [PATCH 02/22] params: don't hand NULL values to param.set callbacks. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An audit by Dongdong Deng revealed that most driver-author-written param calls don't handle val == NULL (which happens when parameters are specified with no =, eg "foo" instead of "foo=1"). The only real case to use this is boolean, so handle it specially for that case and remove a source of bugs for everyone else. Signed-off-by: Rusty Russell Cc: Dongdong Deng Cc: Américo Wang --- kernel/params.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/kernel/params.c b/kernel/params.c index 0b30ecd53a52..3c4a9f1b095e 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -58,6 +58,9 @@ static int parse_one(char *param, /* Find parameter */ for (i = 0; i < num_params; i++) { if (parameq(param, params[i].name)) { + /* Noone handled NULL, so do it here. */ + if (!val && params[i].set != param_set_bool) + return -EINVAL; DEBUGP("They are equal! Calling %p\n", params[i].set); return params[i].set(val, ¶ms[i]); @@ -181,7 +184,6 @@ int parse_args(const char *name, tmptype l; \ int ret; \ \ - if (!val) return -EINVAL; \ ret = strtolfn(val, 0, &l); \ if (ret == -EINVAL || ((type)l != l)) \ return -EINVAL; \ @@ -203,12 +205,6 @@ STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul); int param_set_charp(const char *val, struct kernel_param *kp) { - if (!val) { - printk(KERN_ERR "%s: string parameter expected\n", - kp->name); - return -EINVAL; - } - if (strlen(val) > 1024) { printk(KERN_ERR "%s: string parameter too long\n", kp->name); @@ -309,12 +305,6 @@ static int param_array(const char *name, kp.arg = elem; kp.flags = flags; - /* No equals sign? */ - if (!val) { - printk(KERN_ERR "%s: expects arguments\n", name); - return -EINVAL; - } - *num = 0; /* We expect a comma-separated list of values. */ do { @@ -381,10 +371,6 @@ int param_set_copystring(const char *val, struct kernel_param *kp) { const struct kparam_string *kps = kp->str; - if (!val) { - printk(KERN_ERR "%s: missing param set value\n", kp->name); - return -EINVAL; - } if (strlen(val)+1 > kps->maxlen) { printk(KERN_ERR "%s: string doesn't fit in %u chars.\n", kp->name, kps->maxlen-1); From a14fe249a8f74269c9e636bcbaa78f5bdb354ce3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:11 -0600 Subject: [PATCH 03/22] param: move the EXPORT_SYMBOL to after the definitions. This is modern style, and good to do before we start changing things. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Tested-by: Phil Carmody --- kernel/params.c | 39 +++++++++++++-------------------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/kernel/params.c b/kernel/params.c index 3c4a9f1b095e..3e78fdb445e7 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -193,7 +193,9 @@ int parse_args(const char *name, int param_get_##name(char *buffer, struct kernel_param *kp) \ { \ return sprintf(buffer, format, *((type *)kp->arg)); \ - } + } \ + EXPORT_SYMBOL(param_set_##name); \ + EXPORT_SYMBOL(param_get_##name) STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, strict_strtoul); STANDARD_PARAM_DEF(short, short, "%hi", long, strict_strtol); @@ -222,11 +224,13 @@ int param_set_charp(const char *val, struct kernel_param *kp) return 0; } +EXPORT_SYMBOL(param_set_charp); int param_get_charp(char *buffer, struct kernel_param *kp) { return sprintf(buffer, "%s", *((char **)kp->arg)); } +EXPORT_SYMBOL(param_get_charp); /* Actually could be a bool or an int, for historical reasons. */ int param_set_bool(const char *val, struct kernel_param *kp) @@ -254,6 +258,7 @@ int param_set_bool(const char *val, struct kernel_param *kp) *(int *)kp->arg = v; return 0; } +EXPORT_SYMBOL(param_set_bool); int param_get_bool(char *buffer, struct kernel_param *kp) { @@ -266,6 +271,7 @@ int param_get_bool(char *buffer, struct kernel_param *kp) /* Y and N chosen as being relatively non-coder friendly */ return sprintf(buffer, "%c", val ? 'Y' : 'N'); } +EXPORT_SYMBOL(param_get_bool); /* This one must be bool. */ int param_set_invbool(const char *val, struct kernel_param *kp) @@ -281,11 +287,13 @@ int param_set_invbool(const char *val, struct kernel_param *kp) *(bool *)kp->arg = !boolval; return ret; } +EXPORT_SYMBOL(param_set_invbool); int param_get_invbool(char *buffer, struct kernel_param *kp) { return sprintf(buffer, "%c", (*(bool *)kp->arg) ? 'N' : 'Y'); } +EXPORT_SYMBOL(param_get_invbool); /* We break the rule and mangle the string. */ static int param_array(const char *name, @@ -346,6 +354,7 @@ int param_array_set(const char *val, struct kernel_param *kp) arr->elemsize, arr->set, kp->flags, arr->num ?: &temp_num); } +EXPORT_SYMBOL(param_array_set); int param_array_get(char *buffer, struct kernel_param *kp) { @@ -366,6 +375,7 @@ int param_array_get(char *buffer, struct kernel_param *kp) buffer[off] = '\0'; return off; } +EXPORT_SYMBOL(param_array_get); int param_set_copystring(const char *val, struct kernel_param *kp) { @@ -379,12 +389,14 @@ int param_set_copystring(const char *val, struct kernel_param *kp) strcpy(kps->string, val); return 0; } +EXPORT_SYMBOL(param_set_copystring); int param_get_string(char *buffer, struct kernel_param *kp) { const struct kparam_string *kps = kp->str; return strlcpy(buffer, kps->string, kps->maxlen); } +EXPORT_SYMBOL(param_get_string); /* sysfs output in /sys/modules/XYZ/parameters/ */ #define to_module_attr(n) container_of(n, struct module_attribute, attr) @@ -754,28 +766,3 @@ static int __init param_sysfs_init(void) subsys_initcall(param_sysfs_init); #endif /* CONFIG_SYSFS */ - -EXPORT_SYMBOL(param_set_byte); -EXPORT_SYMBOL(param_get_byte); -EXPORT_SYMBOL(param_set_short); -EXPORT_SYMBOL(param_get_short); -EXPORT_SYMBOL(param_set_ushort); -EXPORT_SYMBOL(param_get_ushort); -EXPORT_SYMBOL(param_set_int); -EXPORT_SYMBOL(param_get_int); -EXPORT_SYMBOL(param_set_uint); -EXPORT_SYMBOL(param_get_uint); -EXPORT_SYMBOL(param_set_long); -EXPORT_SYMBOL(param_get_long); -EXPORT_SYMBOL(param_set_ulong); -EXPORT_SYMBOL(param_get_ulong); -EXPORT_SYMBOL(param_set_charp); -EXPORT_SYMBOL(param_get_charp); -EXPORT_SYMBOL(param_set_bool); -EXPORT_SYMBOL(param_get_bool); -EXPORT_SYMBOL(param_set_invbool); -EXPORT_SYMBOL(param_get_invbool); -EXPORT_SYMBOL(param_array_set); -EXPORT_SYMBOL(param_array_get); -EXPORT_SYMBOL(param_set_copystring); -EXPORT_SYMBOL(param_get_string); From 9bbb9e5a33109b2832e2e63dcc7a132924ab374b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:12 -0600 Subject: [PATCH 04/22] param: use ops in struct kernel_param, rather than get and set fns directly This is more kernel-ish, saves some space, and also allows us to expand the ops without breaking all the callers who are happy for the new members to be NULL. The few places which defined their own param types are changed to the new scheme (more which crept in recently fixed in following patches). Since we're touching them anyway, we change get() and set() to take a const struct kernel_param (which they really are). This causes some harmless warnings until we fix them (in following patches). To reduce churn, module_param_call creates the ops struct so the callers don't have to change (and casts the functions to reduce warnings). The modern version which takes an ops struct is called module_param_cb. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Tested-by: Phil Carmody Cc: "David S. Miller" Cc: Ville Syrjala Cc: Dmitry Torokhov Cc: Alessandro Rubini Cc: Michal Januszewski Cc: Trond Myklebust Cc: "J. Bruce Fields" Cc: Neil Brown Cc: linux-kernel@vger.kernel.org Cc: linux-input@vger.kernel.org Cc: linux-fbdev-devel@lists.sourceforge.net Cc: linux-nfs@vger.kernel.org Cc: netdev@vger.kernel.org --- drivers/input/misc/ati_remote2.c | 26 +++--- drivers/input/mouse/psmouse-base.c | 14 ++-- drivers/video/uvesafb.c | 7 +- fs/nfs/callback.c | 11 ++- include/linux/moduleparam.h | 123 ++++++++++++++++++----------- kernel/params.c | 90 ++++++++++++++------- net/sunrpc/xprtsock.c | 26 +++--- 7 files changed, 186 insertions(+), 111 deletions(-) diff --git a/drivers/input/misc/ati_remote2.c b/drivers/input/misc/ati_remote2.c index e148749b5851..23257652b8e8 100644 --- a/drivers/input/misc/ati_remote2.c +++ b/drivers/input/misc/ati_remote2.c @@ -38,7 +38,8 @@ enum { }; static int ati_remote2_set_mask(const char *val, - struct kernel_param *kp, unsigned int max) + const struct kernel_param *kp, + unsigned int max) { unsigned long mask; int ret; @@ -59,28 +60,31 @@ static int ati_remote2_set_mask(const char *val, } static int ati_remote2_set_channel_mask(const char *val, - struct kernel_param *kp) + const struct kernel_param *kp) { pr_debug("%s()\n", __func__); return ati_remote2_set_mask(val, kp, ATI_REMOTE2_MAX_CHANNEL_MASK); } -static int ati_remote2_get_channel_mask(char *buffer, struct kernel_param *kp) +static int ati_remote2_get_channel_mask(char *buffer, + const struct kernel_param *kp) { pr_debug("%s()\n", __func__); return sprintf(buffer, "0x%04x", *(unsigned int *)kp->arg); } -static int ati_remote2_set_mode_mask(const char *val, struct kernel_param *kp) +static int ati_remote2_set_mode_mask(const char *val, + const struct kernel_param *kp) { pr_debug("%s()\n", __func__); return ati_remote2_set_mask(val, kp, ATI_REMOTE2_MAX_MODE_MASK); } -static int ati_remote2_get_mode_mask(char *buffer, struct kernel_param *kp) +static int ati_remote2_get_mode_mask(char *buffer, + const struct kernel_param *kp) { pr_debug("%s()\n", __func__); @@ -89,15 +93,19 @@ static int ati_remote2_get_mode_mask(char *buffer, struct kernel_param *kp) static unsigned int channel_mask = ATI_REMOTE2_MAX_CHANNEL_MASK; #define param_check_channel_mask(name, p) __param_check(name, p, unsigned int) -#define param_set_channel_mask ati_remote2_set_channel_mask -#define param_get_channel_mask ati_remote2_get_channel_mask +static struct kernel_param_ops param_ops_channel_mask = { + .set = ati_remote2_set_channel_mask, + .get = ati_remote2_get_channel_mask, +}; module_param(channel_mask, channel_mask, 0644); MODULE_PARM_DESC(channel_mask, "Bitmask of channels to accept <15:Channel16>...<1:Channel2><0:Channel1>"); static unsigned int mode_mask = ATI_REMOTE2_MAX_MODE_MASK; #define param_check_mode_mask(name, p) __param_check(name, p, unsigned int) -#define param_set_mode_mask ati_remote2_set_mode_mask -#define param_get_mode_mask ati_remote2_get_mode_mask +static struct kernel_param_ops param_ops_mode_mask = { + .set = ati_remote2_set_mode_mask, + .get = ati_remote2_get_mode_mask, +}; module_param(mode_mask, mode_mask, 0644); MODULE_PARM_DESC(mode_mask, "Bitmask of modes to accept <4:PC><3:AUX4><2:AUX3><1:AUX2><0:AUX1>"); diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c index 979c50215282..73a7af2542a8 100644 --- a/drivers/input/mouse/psmouse-base.c +++ b/drivers/input/mouse/psmouse-base.c @@ -39,11 +39,13 @@ MODULE_DESCRIPTION(DRIVER_DESC); MODULE_LICENSE("GPL"); static unsigned int psmouse_max_proto = PSMOUSE_AUTO; -static int psmouse_set_maxproto(const char *val, struct kernel_param *kp); -static int psmouse_get_maxproto(char *buffer, struct kernel_param *kp); +static int psmouse_set_maxproto(const char *val, const struct kernel_param *); +static int psmouse_get_maxproto(char *buffer, const struct kernel_param *kp); +static struct kernel_param_ops param_ops_proto_abbrev = { + .set = psmouse_set_maxproto, + .get = psmouse_get_maxproto, +}; #define param_check_proto_abbrev(name, p) __param_check(name, p, unsigned int) -#define param_set_proto_abbrev psmouse_set_maxproto -#define param_get_proto_abbrev psmouse_get_maxproto module_param_named(proto, psmouse_max_proto, proto_abbrev, 0644); MODULE_PARM_DESC(proto, "Highest protocol extension to probe (bare, imps, exps, any). Useful for KVM switches."); @@ -1679,7 +1681,7 @@ static ssize_t psmouse_attr_set_resolution(struct psmouse *psmouse, void *data, } -static int psmouse_set_maxproto(const char *val, struct kernel_param *kp) +static int psmouse_set_maxproto(const char *val, const struct kernel_param *kp) { const struct psmouse_protocol *proto; @@ -1696,7 +1698,7 @@ static int psmouse_set_maxproto(const char *val, struct kernel_param *kp) return 0; } -static int psmouse_get_maxproto(char *buffer, struct kernel_param *kp) +static int psmouse_get_maxproto(char *buffer, const struct kernel_param *kp) { int type = *((unsigned int *)kp->arg); diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c index 7b8839ebf3c4..52ec0959d462 100644 --- a/drivers/video/uvesafb.c +++ b/drivers/video/uvesafb.c @@ -1977,8 +1977,7 @@ static void __devexit uvesafb_exit(void) module_exit(uvesafb_exit); -#define param_get_scroll NULL -static int param_set_scroll(const char *val, struct kernel_param *kp) +static int param_set_scroll(const char *val, const struct kernel_param *kp) { ypan = 0; @@ -1993,7 +1992,9 @@ static int param_set_scroll(const char *val, struct kernel_param *kp) return 0; } - +static struct kernel_param_ops param_ops_scroll = { + .set = param_set_scroll, +}; #define param_check_scroll(name, p) __param_check(name, p, void) module_param_named(scroll, ypan, scroll, 0); diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index 36dfdae95123..e17b49e2eabd 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c @@ -45,7 +45,7 @@ unsigned short nfs_callback_tcpport; unsigned short nfs_callback_tcpport6; #define NFS_CALLBACK_MAXPORTNR (65535U) -static int param_set_portnr(const char *val, struct kernel_param *kp) +static int param_set_portnr(const char *val, const struct kernel_param *kp) { unsigned long num; int ret; @@ -58,11 +58,10 @@ static int param_set_portnr(const char *val, struct kernel_param *kp) *((unsigned int *)kp->arg) = num; return 0; } - -static int param_get_portnr(char *buffer, struct kernel_param *kp) -{ - return param_get_uint(buffer, kp); -} +static struct kernel_param_ops param_ops_portnr = { + .set = param_set_portnr, + .get = param_get_uint, +}; #define param_check_portnr(name, p) __param_check(name, p, unsigned int); module_param_named(callback_tcpport, nfs_callback_set_tcpport, portnr, 0644); diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 82a9124f7d75..02e5090ce32f 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -31,20 +31,21 @@ static const char __module_cat(name,__LINE__)[] \ struct kernel_param; -/* Returns 0, or -errno. arg is in kp->arg. */ -typedef int (*param_set_fn)(const char *val, struct kernel_param *kp); -/* Returns length written or -errno. Buffer is 4k (ie. be short!) */ -typedef int (*param_get_fn)(char *buffer, struct kernel_param *kp); +struct kernel_param_ops { + /* Returns 0, or -errno. arg is in kp->arg. */ + int (*set)(const char *val, const struct kernel_param *kp); + /* Returns length written or -errno. Buffer is 4k (ie. be short!) */ + int (*get)(char *buffer, const struct kernel_param *kp); +}; /* Flag bits for kernel_param.flags */ #define KPARAM_ISBOOL 2 struct kernel_param { const char *name; + const struct kernel_param_ops *ops; u16 perm; u16 flags; - param_set_fn set; - param_get_fn get; union { void *arg; const struct kparam_string *str; @@ -63,8 +64,7 @@ struct kparam_array { unsigned int max; unsigned int *num; - param_set_fn set; - param_get_fn get; + const struct kernel_param_ops *ops; unsigned int elemsize; void *elem; }; @@ -83,7 +83,7 @@ struct kparam_array parameters. perm sets the visibility in sysfs: 000 means it's not there, read bits mean it's readable, write bits mean it's writable. */ -#define __module_param_call(prefix, name, set, get, arg, isbool, perm) \ +#define __module_param_call(prefix, name, ops, arg, isbool, perm) \ /* Default value instead of permissions? */ \ static int __param_perm_check_##name __attribute__((unused)) = \ BUILD_BUG_ON_ZERO((perm) < 0 || (perm) > 0777 || ((perm) & 2)) \ @@ -92,20 +92,37 @@ struct kparam_array static struct kernel_param __moduleparam_const __param_##name \ __used \ __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \ - = { __param_str_##name, perm, isbool ? KPARAM_ISBOOL : 0, \ - set, get, { arg } } + = { __param_str_##name, ops, perm, isbool ? KPARAM_ISBOOL : 0, \ + { arg } } -#define module_param_call(name, set, get, arg, perm) \ +/* Obsolete - use module_param_cb() */ +#define module_param_call(name, set, get, arg, perm) \ + static struct kernel_param_ops __param_ops_##name = \ + { (void *)set, (void *)get }; \ + __module_param_call(MODULE_PARAM_PREFIX, \ + name, &__param_ops_##name, arg, \ + __same_type(*(arg), bool), \ + (perm) + sizeof(__check_old_set_param(set))*0) + +/* We don't get oldget: it's often a new-style param_get_uint, etc. */ +static inline int +__check_old_set_param(int (*oldset)(const char *, struct kernel_param *)) +{ + return 0; +} + +#define module_param_cb(name, ops, arg, perm) \ __module_param_call(MODULE_PARAM_PREFIX, \ - name, set, get, arg, \ - __same_type(*(arg), bool), perm) + name, ops, arg, __same_type(*(arg), bool), perm) -/* Helper functions: type is byte, short, ushort, int, uint, long, - ulong, charp, bool or invbool, or XXX if you define param_get_XXX, - param_set_XXX and param_check_XXX. */ +/* + * Helper functions: type is byte, short, ushort, int, uint, long, + * ulong, charp, bool or invbool, or XXX if you define param_ops_XXX + * and param_check_XXX. + */ #define module_param_named(name, value, type, perm) \ param_check_##type(name, &(value)); \ - module_param_call(name, param_set_##type, param_get_##type, &value, perm); \ + module_param_cb(name, ¶m_ops_##type, &value, perm); \ __MODULE_PARM_TYPE(name, #type) #define module_param(name, type, perm) \ @@ -126,7 +143,7 @@ struct kparam_array */ #define core_param(name, var, type, perm) \ param_check_##type(name, &(var)); \ - __module_param_call("", name, param_set_##type, param_get_##type, \ + __module_param_call("", name, ¶m_ops_##type, \ &var, __same_type(var, bool), perm) #endif /* !MODULE */ @@ -135,7 +152,7 @@ struct kparam_array static const struct kparam_string __param_string_##name \ = { len, string }; \ __module_param_call(MODULE_PARAM_PREFIX, name, \ - param_set_copystring, param_get_string, \ + ¶m_ops_string, \ .str = &__param_string_##name, 0, perm); \ __MODULE_PARM_TYPE(name, "string") @@ -162,41 +179,50 @@ static inline void destroy_params(const struct kernel_param *params, #define __param_check(name, p, type) \ static inline type *__check_##name(void) { return(p); } -extern int param_set_byte(const char *val, struct kernel_param *kp); -extern int param_get_byte(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_byte; +extern int param_set_byte(const char *val, const struct kernel_param *kp); +extern int param_get_byte(char *buffer, const struct kernel_param *kp); #define param_check_byte(name, p) __param_check(name, p, unsigned char) -extern int param_set_short(const char *val, struct kernel_param *kp); -extern int param_get_short(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_short; +extern int param_set_short(const char *val, const struct kernel_param *kp); +extern int param_get_short(char *buffer, const struct kernel_param *kp); #define param_check_short(name, p) __param_check(name, p, short) -extern int param_set_ushort(const char *val, struct kernel_param *kp); -extern int param_get_ushort(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_ushort; +extern int param_set_ushort(const char *val, const struct kernel_param *kp); +extern int param_get_ushort(char *buffer, const struct kernel_param *kp); #define param_check_ushort(name, p) __param_check(name, p, unsigned short) -extern int param_set_int(const char *val, struct kernel_param *kp); -extern int param_get_int(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_int; +extern int param_set_int(const char *val, const struct kernel_param *kp); +extern int param_get_int(char *buffer, const struct kernel_param *kp); #define param_check_int(name, p) __param_check(name, p, int) -extern int param_set_uint(const char *val, struct kernel_param *kp); -extern int param_get_uint(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_uint; +extern int param_set_uint(const char *val, const struct kernel_param *kp); +extern int param_get_uint(char *buffer, const struct kernel_param *kp); #define param_check_uint(name, p) __param_check(name, p, unsigned int) -extern int param_set_long(const char *val, struct kernel_param *kp); -extern int param_get_long(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_long; +extern int param_set_long(const char *val, const struct kernel_param *kp); +extern int param_get_long(char *buffer, const struct kernel_param *kp); #define param_check_long(name, p) __param_check(name, p, long) -extern int param_set_ulong(const char *val, struct kernel_param *kp); -extern int param_get_ulong(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_ulong; +extern int param_set_ulong(const char *val, const struct kernel_param *kp); +extern int param_get_ulong(char *buffer, const struct kernel_param *kp); #define param_check_ulong(name, p) __param_check(name, p, unsigned long) -extern int param_set_charp(const char *val, struct kernel_param *kp); -extern int param_get_charp(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_charp; +extern int param_set_charp(const char *val, const struct kernel_param *kp); +extern int param_get_charp(char *buffer, const struct kernel_param *kp); #define param_check_charp(name, p) __param_check(name, p, char *) /* For historical reasons "bool" parameters can be (unsigned) "int". */ -extern int param_set_bool(const char *val, struct kernel_param *kp); -extern int param_get_bool(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_bool; +extern int param_set_bool(const char *val, const struct kernel_param *kp); +extern int param_get_bool(char *buffer, const struct kernel_param *kp); #define param_check_bool(name, p) \ static inline void __check_##name(void) \ { \ @@ -205,17 +231,18 @@ extern int param_get_bool(char *buffer, struct kernel_param *kp); !__same_type(*(p), int)); \ } -extern int param_set_invbool(const char *val, struct kernel_param *kp); -extern int param_get_invbool(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_invbool; +extern int param_set_invbool(const char *val, const struct kernel_param *kp); +extern int param_get_invbool(char *buffer, const struct kernel_param *kp); #define param_check_invbool(name, p) __param_check(name, p, bool) /* Comma-separated array: *nump is set to number they actually specified. */ #define module_param_array_named(name, array, type, nump, perm) \ static const struct kparam_array __param_arr_##name \ - = { ARRAY_SIZE(array), nump, param_set_##type, param_get_##type,\ + = { ARRAY_SIZE(array), nump, ¶m_ops_##type, \ sizeof(array[0]), array }; \ __module_param_call(MODULE_PARAM_PREFIX, name, \ - param_array_set, param_array_get, \ + ¶m_array_ops, \ .arr = &__param_arr_##name, \ __same_type(array[0], bool), perm); \ __MODULE_PARM_TYPE(name, "array of " #type) @@ -223,11 +250,11 @@ extern int param_get_invbool(char *buffer, struct kernel_param *kp); #define module_param_array(name, type, nump, perm) \ module_param_array_named(name, name, type, nump, perm) -extern int param_array_set(const char *val, struct kernel_param *kp); -extern int param_array_get(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_array_ops; -extern int param_set_copystring(const char *val, struct kernel_param *kp); -extern int param_get_string(char *buffer, struct kernel_param *kp); +extern struct kernel_param_ops param_ops_string; +extern int param_set_copystring(const char *val, const struct kernel_param *); +extern int param_get_string(char *buffer, const struct kernel_param *kp); /* for exporting parameters in /sys/parameters */ @@ -235,13 +262,13 @@ struct module; #if defined(CONFIG_SYSFS) && defined(CONFIG_MODULES) extern int module_param_sysfs_setup(struct module *mod, - struct kernel_param *kparam, + const struct kernel_param *kparam, unsigned int num_params); extern void module_param_sysfs_remove(struct module *mod); #else static inline int module_param_sysfs_setup(struct module *mod, - struct kernel_param *kparam, + const struct kernel_param *kparam, unsigned int num_params) { return 0; diff --git a/kernel/params.c b/kernel/params.c index 3e78fdb445e7..a550698ae02d 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -59,11 +59,11 @@ static int parse_one(char *param, for (i = 0; i < num_params; i++) { if (parameq(param, params[i].name)) { /* Noone handled NULL, so do it here. */ - if (!val && params[i].set != param_set_bool) + if (!val && params[i].ops->set != param_set_bool) return -EINVAL; DEBUGP("They are equal! Calling %p\n", - params[i].set); - return params[i].set(val, ¶ms[i]); + params[i].ops->set); + return params[i].ops->set(val, ¶ms[i]); } } @@ -179,7 +179,7 @@ int parse_args(const char *name, /* Lazy bastard, eh? */ #define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn) \ - int param_set_##name(const char *val, struct kernel_param *kp) \ + int param_set_##name(const char *val, const struct kernel_param *kp) \ { \ tmptype l; \ int ret; \ @@ -190,12 +190,18 @@ int parse_args(const char *name, *((type *)kp->arg) = l; \ return 0; \ } \ - int param_get_##name(char *buffer, struct kernel_param *kp) \ + int param_get_##name(char *buffer, const struct kernel_param *kp) \ { \ return sprintf(buffer, format, *((type *)kp->arg)); \ } \ + struct kernel_param_ops param_ops_##name = { \ + .set = param_set_##name, \ + .get = param_get_##name, \ + }; \ EXPORT_SYMBOL(param_set_##name); \ - EXPORT_SYMBOL(param_get_##name) + EXPORT_SYMBOL(param_get_##name); \ + EXPORT_SYMBOL(param_ops_##name) + STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, strict_strtoul); STANDARD_PARAM_DEF(short, short, "%hi", long, strict_strtol); @@ -205,7 +211,7 @@ STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, strict_strtoul); STANDARD_PARAM_DEF(long, long, "%li", long, strict_strtol); STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul); -int param_set_charp(const char *val, struct kernel_param *kp) +int param_set_charp(const char *val, const struct kernel_param *kp) { if (strlen(val) > 1024) { printk(KERN_ERR "%s: string parameter too long\n", @@ -226,14 +232,20 @@ int param_set_charp(const char *val, struct kernel_param *kp) } EXPORT_SYMBOL(param_set_charp); -int param_get_charp(char *buffer, struct kernel_param *kp) +int param_get_charp(char *buffer, const struct kernel_param *kp) { return sprintf(buffer, "%s", *((char **)kp->arg)); } EXPORT_SYMBOL(param_get_charp); +struct kernel_param_ops param_ops_charp = { + .set = param_set_charp, + .get = param_get_charp, +}; +EXPORT_SYMBOL(param_ops_charp); + /* Actually could be a bool or an int, for historical reasons. */ -int param_set_bool(const char *val, struct kernel_param *kp) +int param_set_bool(const char *val, const struct kernel_param *kp) { bool v; @@ -260,7 +272,7 @@ int param_set_bool(const char *val, struct kernel_param *kp) } EXPORT_SYMBOL(param_set_bool); -int param_get_bool(char *buffer, struct kernel_param *kp) +int param_get_bool(char *buffer, const struct kernel_param *kp) { bool val; if (kp->flags & KPARAM_ISBOOL) @@ -273,8 +285,14 @@ int param_get_bool(char *buffer, struct kernel_param *kp) } EXPORT_SYMBOL(param_get_bool); +struct kernel_param_ops param_ops_bool = { + .set = param_set_bool, + .get = param_get_bool, +}; +EXPORT_SYMBOL(param_ops_bool); + /* This one must be bool. */ -int param_set_invbool(const char *val, struct kernel_param *kp) +int param_set_invbool(const char *val, const struct kernel_param *kp) { int ret; bool boolval; @@ -289,18 +307,24 @@ int param_set_invbool(const char *val, struct kernel_param *kp) } EXPORT_SYMBOL(param_set_invbool); -int param_get_invbool(char *buffer, struct kernel_param *kp) +int param_get_invbool(char *buffer, const struct kernel_param *kp) { return sprintf(buffer, "%c", (*(bool *)kp->arg) ? 'N' : 'Y'); } EXPORT_SYMBOL(param_get_invbool); +struct kernel_param_ops param_ops_invbool = { + .set = param_set_invbool, + .get = param_get_invbool, +}; +EXPORT_SYMBOL(param_ops_invbool); + /* We break the rule and mangle the string. */ static int param_array(const char *name, const char *val, unsigned int min, unsigned int max, void *elem, int elemsize, - int (*set)(const char *, struct kernel_param *kp), + int (*set)(const char *, const struct kernel_param *kp), u16 flags, unsigned int *num) { @@ -345,18 +369,17 @@ static int param_array(const char *name, return 0; } -int param_array_set(const char *val, struct kernel_param *kp) +static int param_array_set(const char *val, const struct kernel_param *kp) { const struct kparam_array *arr = kp->arr; unsigned int temp_num; return param_array(kp->name, val, 1, arr->max, arr->elem, - arr->elemsize, arr->set, kp->flags, + arr->elemsize, arr->ops->set, kp->flags, arr->num ?: &temp_num); } -EXPORT_SYMBOL(param_array_set); -int param_array_get(char *buffer, struct kernel_param *kp) +static int param_array_get(char *buffer, const struct kernel_param *kp) { int i, off, ret; const struct kparam_array *arr = kp->arr; @@ -367,7 +390,7 @@ int param_array_get(char *buffer, struct kernel_param *kp) if (i) buffer[off++] = ','; p.arg = arr->elem + arr->elemsize * i; - ret = arr->get(buffer + off, &p); + ret = arr->ops->get(buffer + off, &p); if (ret < 0) return ret; off += ret; @@ -375,9 +398,14 @@ int param_array_get(char *buffer, struct kernel_param *kp) buffer[off] = '\0'; return off; } -EXPORT_SYMBOL(param_array_get); -int param_set_copystring(const char *val, struct kernel_param *kp) +struct kernel_param_ops param_array_ops = { + .set = param_array_set, + .get = param_array_get, +}; +EXPORT_SYMBOL(param_array_ops); + +int param_set_copystring(const char *val, const struct kernel_param *kp) { const struct kparam_string *kps = kp->str; @@ -391,13 +419,19 @@ int param_set_copystring(const char *val, struct kernel_param *kp) } EXPORT_SYMBOL(param_set_copystring); -int param_get_string(char *buffer, struct kernel_param *kp) +int param_get_string(char *buffer, const struct kernel_param *kp) { const struct kparam_string *kps = kp->str; return strlcpy(buffer, kps->string, kps->maxlen); } EXPORT_SYMBOL(param_get_string); +struct kernel_param_ops param_ops_string = { + .set = param_set_copystring, + .get = param_get_string, +}; +EXPORT_SYMBOL(param_ops_string); + /* sysfs output in /sys/modules/XYZ/parameters/ */ #define to_module_attr(n) container_of(n, struct module_attribute, attr) #define to_module_kobject(n) container_of(n, struct module_kobject, kobj) @@ -407,7 +441,7 @@ extern struct kernel_param __start___param[], __stop___param[]; struct param_attribute { struct module_attribute mattr; - struct kernel_param *param; + const struct kernel_param *param; }; struct module_param_attrs @@ -426,10 +460,10 @@ static ssize_t param_attr_show(struct module_attribute *mattr, int count; struct param_attribute *attribute = to_param_attr(mattr); - if (!attribute->param->get) + if (!attribute->param->ops->get) return -EPERM; - count = attribute->param->get(buf, attribute->param); + count = attribute->param->ops->get(buf, attribute->param); if (count > 0) { strcat(buf, "\n"); ++count; @@ -445,10 +479,10 @@ static ssize_t param_attr_store(struct module_attribute *mattr, int err; struct param_attribute *attribute = to_param_attr(mattr); - if (!attribute->param->set) + if (!attribute->param->ops->set) return -EPERM; - err = attribute->param->set(buf, attribute->param); + err = attribute->param->ops->set(buf, attribute->param); if (!err) return len; return err; @@ -473,7 +507,7 @@ static ssize_t param_attr_store(struct module_attribute *mattr, * if there's an error. */ static __modinit int add_sysfs_param(struct module_kobject *mk, - struct kernel_param *kp, + const struct kernel_param *kp, const char *name) { struct module_param_attrs *new; @@ -555,7 +589,7 @@ static void free_module_param_attrs(struct module_kobject *mk) * /sys/module/[mod->name]/parameters/ */ int module_param_sysfs_setup(struct module *mod, - struct kernel_param *kparam, + const struct kernel_param *kparam, unsigned int num_params) { int i, err; diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index 7ca65c7005ea..49a62f0c4b87 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -2577,7 +2577,8 @@ void cleanup_socket_xprt(void) xprt_unregister_transport(&xs_bc_tcp_transport); } -static int param_set_uint_minmax(const char *val, struct kernel_param *kp, +static int param_set_uint_minmax(const char *val, + const struct kernel_param *kp, unsigned int min, unsigned int max) { unsigned long num; @@ -2592,34 +2593,37 @@ static int param_set_uint_minmax(const char *val, struct kernel_param *kp, return 0; } -static int param_set_portnr(const char *val, struct kernel_param *kp) +static int param_set_portnr(const char *val, const struct kernel_param *kp) { return param_set_uint_minmax(val, kp, RPC_MIN_RESVPORT, RPC_MAX_RESVPORT); } -static int param_get_portnr(char *buffer, struct kernel_param *kp) -{ - return param_get_uint(buffer, kp); -} +static struct kernel_param_ops param_ops_portnr = { + .set = param_set_portnr, + .get = param_get_uint, +}; + #define param_check_portnr(name, p) \ __param_check(name, p, unsigned int); module_param_named(min_resvport, xprt_min_resvport, portnr, 0644); module_param_named(max_resvport, xprt_max_resvport, portnr, 0644); -static int param_set_slot_table_size(const char *val, struct kernel_param *kp) +static int param_set_slot_table_size(const char *val, + const struct kernel_param *kp) { return param_set_uint_minmax(val, kp, RPC_MIN_SLOT_TABLE, RPC_MAX_SLOT_TABLE); } -static int param_get_slot_table_size(char *buffer, struct kernel_param *kp) -{ - return param_get_uint(buffer, kp); -} +static struct kernel_param_ops param_ops_slot_table_size = { + .set = param_set_slot_table_size, + .get = param_get_uint, +}; + #define param_check_slot_table_size(name, p) \ __param_check(name, p, unsigned int); From 101d6c826fa03266f8538ea4f6a459190e6863e8 Mon Sep 17 00:00:00 2001 From: Stephen Rothwell Date: Mon, 2 Aug 2010 12:00:43 +1000 Subject: [PATCH 05/22] AppArmor: update for module_param_named API change Fixes these build errors: security/apparmor/lsm.c:701: error: 'param_ops_aabool' undeclared here (not in a function) security/apparmor/lsm.c:721: error: 'param_ops_aalockpolicy' undeclared here (not in a function) security/apparmor/lsm.c:729: error: 'param_ops_aauint' undeclared here (not in a function) Signed-off-by: Stephen Rothwell Signed-off-by: John Johansen Signed-off-by: Rusty Russell --- security/apparmor/lsm.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 8db33a8b50c4..d5666d3cc21b 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -667,17 +667,29 @@ static struct security_operations apparmor_ops = { * AppArmor sysfs module parameters */ -static int param_set_aabool(const char *val, struct kernel_param *kp); -static int param_get_aabool(char *buffer, struct kernel_param *kp); +static int param_set_aabool(const char *val, const struct kernel_param *kp); +static int param_get_aabool(char *buffer, const struct kernel_param *kp); #define param_check_aabool(name, p) __param_check(name, p, int) +static struct kernel_param_ops param_ops_aabool = { + .set = param_set_aabool, + .get = param_get_aabool +}; -static int param_set_aauint(const char *val, struct kernel_param *kp); -static int param_get_aauint(char *buffer, struct kernel_param *kp); +static int param_set_aauint(const char *val, const struct kernel_param *kp); +static int param_get_aauint(char *buffer, const struct kernel_param *kp); #define param_check_aauint(name, p) __param_check(name, p, int) +static struct kernel_param_ops param_ops_aauint = { + .set = param_set_aauint, + .get = param_get_aauint +}; -static int param_set_aalockpolicy(const char *val, struct kernel_param *kp); -static int param_get_aalockpolicy(char *buffer, struct kernel_param *kp); +static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp); +static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp); #define param_check_aalockpolicy(name, p) __param_check(name, p, int) +static struct kernel_param_ops param_ops_aalockpolicy = { + .set = param_set_aalockpolicy, + .get = param_get_aalockpolicy +}; static int param_set_audit(const char *val, struct kernel_param *kp); static int param_get_audit(char *buffer, struct kernel_param *kp); @@ -751,7 +763,7 @@ static int __init apparmor_enabled_setup(char *str) __setup("apparmor=", apparmor_enabled_setup); /* set global flag turning off the ability to load policy */ -static int param_set_aalockpolicy(const char *val, struct kernel_param *kp) +static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp) { if (!capable(CAP_MAC_ADMIN)) return -EPERM; @@ -760,35 +772,35 @@ static int param_set_aalockpolicy(const char *val, struct kernel_param *kp) return param_set_bool(val, kp); } -static int param_get_aalockpolicy(char *buffer, struct kernel_param *kp) +static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp) { if (!capable(CAP_MAC_ADMIN)) return -EPERM; return param_get_bool(buffer, kp); } -static int param_set_aabool(const char *val, struct kernel_param *kp) +static int param_set_aabool(const char *val, const struct kernel_param *kp) { if (!capable(CAP_MAC_ADMIN)) return -EPERM; return param_set_bool(val, kp); } -static int param_get_aabool(char *buffer, struct kernel_param *kp) +static int param_get_aabool(char *buffer, const struct kernel_param *kp) { if (!capable(CAP_MAC_ADMIN)) return -EPERM; return param_get_bool(buffer, kp); } -static int param_set_aauint(const char *val, struct kernel_param *kp) +static int param_set_aauint(const char *val, const struct kernel_param *kp) { if (!capable(CAP_MAC_ADMIN)) return -EPERM; return param_set_uint(val, kp); } -static int param_get_aauint(char *buffer, struct kernel_param *kp) +static int param_get_aauint(char *buffer, const struct kernel_param *kp) { if (!capable(CAP_MAC_ADMIN)) return -EPERM; From 8e4e15d44a817e9f02cb04a6cd6c0ee77ed3fbd8 Mon Sep 17 00:00:00 2001 From: Stephen Rothwell Date: Wed, 4 Aug 2010 12:11:22 +1000 Subject: [PATCH 06/22] nfs: update for module_param_named API change After merging the rr tree, today's linux-next build (powerpc ppc64_defconfig) failed like this: net/sunrpc/auth.c:74: error: 'param_ops_hashtbl_sz' undeclared here (not in a function) Caused by commit 0685652df0929cec7d78efa85127f6eb34962132 ("param:param_ops") interacting with commit f8f853ab19fcc415b6eadd273373edc424916212 ("SUNRPC: Make the credential cache hashtable size configurable") from the nfs tree. Signed-off-by: Stephen Rothwell Signed-off-by: Rusty Russell --- net/sunrpc/auth.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c index 880d0de3f50f..36cb66022a27 100644 --- a/net/sunrpc/auth.c +++ b/net/sunrpc/auth.c @@ -39,7 +39,7 @@ static LIST_HEAD(cred_unused); static unsigned long number_cred_unused; #define MAX_HASHTABLE_BITS (10) -static int param_set_hashtbl_sz(const char *val, struct kernel_param *kp) +static int param_set_hashtbl_sz(const char *val, const struct kernel_param *kp) { unsigned long num; unsigned int nbits; @@ -61,7 +61,7 @@ out_inval: return -EINVAL; } -static int param_get_hashtbl_sz(char *buffer, struct kernel_param *kp) +static int param_get_hashtbl_sz(char *buffer, const struct kernel_param *kp) { unsigned int nbits; @@ -71,6 +71,11 @@ static int param_get_hashtbl_sz(char *buffer, struct kernel_param *kp) #define param_check_hashtbl_sz(name, p) __param_check(name, p, unsigned int); +static struct kernel_param_ops param_ops_hashtbl_sz = { + .set = param_set_hashtbl_sz, + .get = param_get_hashtbl_sz, +}; + module_param_named(auth_hashtable_size, auth_hashbits, hashtbl_sz, 0644); MODULE_PARM_DESC(auth_hashtable_size, "RPC credential cache hashtable size"); From 549a8a030693912d5ec4106c2f538593c482a1e4 Mon Sep 17 00:00:00 2001 From: Sachin Sant Date: Tue, 17 Nov 2009 18:51:03 +0530 Subject: [PATCH 07/22] Add param ops struct for hvc_iucv driver. Today's next 20091117 build failed on s390 with drivers/char/hvc_iucv.c:1331: error: 'param_ops_vmidfilter' undeclared here (not in a function) make[2]: *** [drivers/char/hvc_iucv.o] Error 1 Most probably caused by commit 684a6d340b8a5767db4670031b0f39455346018a (param:param_ops) which introduced a param_ops structure. The following compile tested patch adds a param_ops structure for hvc_iucv. Signed-off-by: Sachin Sant Acked-by: Heiko Carstens Tested-by: Phil Carmody Signed-off-by: Rusty Russell --- drivers/char/hvc_iucv.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/char/hvc_iucv.c b/drivers/char/hvc_iucv.c index 5a80ad68ef22..7b01bc609de3 100644 --- a/drivers/char/hvc_iucv.c +++ b/drivers/char/hvc_iucv.c @@ -1149,7 +1149,7 @@ out_err: * Note: If it is called early in the boot process, @val is stored and * parsed later in hvc_iucv_init(). */ -static int param_set_vmidfilter(const char *val, struct kernel_param *kp) +static int param_set_vmidfilter(const char *val, const struct kernel_param *kp) { int rc; @@ -1176,7 +1176,7 @@ static int param_set_vmidfilter(const char *val, struct kernel_param *kp) * The function stores the filter as a comma-separated list of z/VM user IDs * in @buffer. Typically, sysfs routines call this function for attr show. */ -static int param_get_vmidfilter(char *buffer, struct kernel_param *kp) +static int param_get_vmidfilter(char *buffer, const struct kernel_param *kp) { int rc; size_t index, len; @@ -1203,6 +1203,11 @@ static int param_get_vmidfilter(char *buffer, struct kernel_param *kp) #define param_check_vmidfilter(name, p) __param_check(name, p, void) +static struct kernel_param_ops param_ops_vmidfilter = { + .set = param_set_vmidfilter, + .get = param_get_vmidfilter, +}; + /** * hvc_iucv_init() - z/VM IUCV HVC device driver initialization */ From 6a841528d288ac420052f5c98fd426b0fcdc5b52 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:16 -0600 Subject: [PATCH 08/22] param: silence .init.text references from param ops Ideally, we'd check that it was only the "set" function which was __init, and that the permissions were r/o. But that's a little hard. Signed-off-by: Rusty Russell Acked-by: Sam Ravnborg Tested-by: Phil Carmody --- scripts/mod/modpost.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 1ce655dde99e..b16044002d91 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1033,6 +1033,13 @@ static const struct sectioncheck *section_mismatch( * fromsec = .data* * atsym =__param* * + * Pattern 1a: + * module_param_call() ops can refer to __init set function if permissions=0 + * The pattern is identified by: + * tosec = .init.text + * fromsec = .data* + * atsym = __param_ops_* + * * Pattern 2: * Many drivers utilise a *driver container with references to * add, remove, probe functions etc. @@ -1067,6 +1074,12 @@ static int secref_whitelist(const struct sectioncheck *mismatch, (strncmp(fromsym, "__param", strlen("__param")) == 0)) return 0; + /* Check for pattern 1a */ + if (strcmp(tosec, ".init.text") == 0 && + match(fromsec, data_sections) && + (strncmp(fromsym, "__param_ops_", strlen("__param_ops_")) == 0)) + return 0; + /* Check for pattern 2 */ if (match(tosec, init_exit_sections) && match(fromsec, data_sections) && From e6df34a4429b77fdffb6e05adf263468a3dcda33 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:17 -0600 Subject: [PATCH 09/22] param: add a free hook to kernel_param_ops. This allows us to generalize the KPARAM_KMALLOCED flag, by calling a function on every parameter when a module is unloaded. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Tested-by: Phil Carmody --- include/linux/moduleparam.h | 2 ++ kernel/params.c | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 02e5090ce32f..9f51568f51c8 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -36,6 +36,8 @@ struct kernel_param_ops { int (*set)(const char *val, const struct kernel_param *kp); /* Returns length written or -errno. Buffer is 4k (ie. be short!) */ int (*get)(char *buffer, const struct kernel_param *kp); + /* Optional function to free kp->arg when module unloaded. */ + void (*free)(void *arg); }; /* Flag bits for kernel_param.flags */ diff --git a/kernel/params.c b/kernel/params.c index a550698ae02d..458a09b886c4 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -399,9 +399,20 @@ static int param_array_get(char *buffer, const struct kernel_param *kp) return off; } +static void param_array_free(void *arg) +{ + unsigned int i; + const struct kparam_array *arr = arg; + + if (arr->ops->free) + for (i = 0; i < (arr->num ? *arr->num : arr->max); i++) + arr->ops->free(arr->elem + arr->elemsize * i); +} + struct kernel_param_ops param_array_ops = { .set = param_array_set, .get = param_array_get, + .free = param_array_free, }; EXPORT_SYMBOL(param_array_ops); @@ -634,7 +645,11 @@ void module_param_sysfs_remove(struct module *mod) void destroy_params(const struct kernel_param *params, unsigned num) { - /* FIXME: This should free kmalloced charp parameters. It doesn't. */ + unsigned int i; + + for (i = 0; i < num; i++) + if (params[i].ops->free) + params[i].ops->free(params[i].arg); } static void __init kernel_add_sysfs_param(const char *name, From a1054322afc8120ea5a50bc84e5beeda54571862 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:18 -0600 Subject: [PATCH 10/22] param: use free hook for charp (fix leak of charp parameters) Instead of using a "I kmalloced this" flag, we keep track of the kmalloced strings and use that list to check if we need to kfree (in practice, the list is very short). This means that kparams can be const again, and plugs a leak. This is important for drivers/usb/gadget/nokia.c which gets modprobe/rmmod'ed frequently on the N9000. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Cc: Artem Bityutskiy Tested-by: Phil Carmody --- kernel/params.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/kernel/params.c b/kernel/params.c index 458a09b886c4..ef60db14fae0 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -31,6 +31,45 @@ #define DEBUGP(fmt, a...) #endif +/* This just allows us to keep track of which parameters are kmalloced. */ +struct kmalloced_param { + struct list_head list; + char val[]; +}; +static DEFINE_MUTEX(param_lock); +static LIST_HEAD(kmalloced_params); + +static void *kmalloc_parameter(unsigned int size) +{ + struct kmalloced_param *p; + + p = kmalloc(sizeof(*p) + size, GFP_KERNEL); + if (!p) + return NULL; + + mutex_lock(¶m_lock); + list_add(&p->list, &kmalloced_params); + mutex_unlock(¶m_lock); + + return p->val; +} + +/* Does nothing if parameter wasn't kmalloced above. */ +static void maybe_kfree_parameter(void *param) +{ + struct kmalloced_param *p; + + mutex_lock(¶m_lock); + list_for_each_entry(p, &kmalloced_params, list) { + if (p->val == param) { + list_del(&p->list); + kfree(p); + break; + } + } + mutex_unlock(¶m_lock); +} + static inline char dash2underscore(char c) { if (c == '-') @@ -219,12 +258,15 @@ int param_set_charp(const char *val, const struct kernel_param *kp) return -ENOSPC; } - /* This is a hack. We can't need to strdup in early boot, and we + maybe_kfree_parameter(*(char **)kp->arg); + + /* This is a hack. We can't kmalloc in early boot, and we * don't need to; this mangled commandline is preserved. */ if (slab_is_available()) { - *(char **)kp->arg = kstrdup(val, GFP_KERNEL); + *(char **)kp->arg = kmalloc_parameter(strlen(val)+1); if (!*(char **)kp->arg) return -ENOMEM; + strcpy(*(char **)kp->arg, val); } else *(const char **)kp->arg = val; @@ -238,9 +280,15 @@ int param_get_charp(char *buffer, const struct kernel_param *kp) } EXPORT_SYMBOL(param_get_charp); +static void param_free_charp(void *arg) +{ + maybe_kfree_parameter(*((char **)arg)); +} + struct kernel_param_ops param_ops_charp = { .set = param_set_charp, .get = param_get_charp, + .free = param_free_charp, }; EXPORT_SYMBOL(param_ops_charp); From 914dcaa84c53f2c3efa6016efcae13fd92a8a17c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:18 -0600 Subject: [PATCH 11/22] param: make param sections const. Since this section can be read-only (they're in .rodata), they should always have been const. Minor flow-through various functions. Signed-off-by: Rusty Russell Tested-by: Phil Carmody --- include/linux/moduleparam.h | 2 +- init/main.c | 8 ++++---- kernel/params.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 9f51568f51c8..6d48831fe7d2 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -161,7 +161,7 @@ __check_old_set_param(int (*oldset)(const char *, struct kernel_param *)) /* Called on module insert or kernel boot */ extern int parse_args(const char *name, char *args, - struct kernel_param *params, + const struct kernel_param *params, unsigned num, int (*unknown)(char *param, char *val)); diff --git a/init/main.c b/init/main.c index 86cbfd085b01..22d61cb06f98 100644 --- a/init/main.c +++ b/init/main.c @@ -201,11 +201,11 @@ static char * argv_init[MAX_INIT_ARGS+2] = { "init", NULL, }; char * envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, }; static const char *panic_later, *panic_param; -extern struct obs_kernel_param __setup_start[], __setup_end[]; +extern const struct obs_kernel_param __setup_start[], __setup_end[]; static int __init obsolete_checksetup(char *line) { - struct obs_kernel_param *p; + const struct obs_kernel_param *p; int had_early_param = 0; p = __setup_start; @@ -458,7 +458,7 @@ static noinline void __init_refok rest_init(void) /* Check for early params. */ static int __init do_early_param(char *param, char *val) { - struct obs_kernel_param *p; + const struct obs_kernel_param *p; for (p = __setup_start; p < __setup_end; p++) { if ((p->early && strcmp(param, p->str) == 0) || @@ -536,7 +536,7 @@ static void __init mm_init(void) asmlinkage void __init start_kernel(void) { char * command_line; - extern struct kernel_param __start___param[], __stop___param[]; + extern const struct kernel_param __start___param[], __stop___param[]; smp_setup_processor_id(); diff --git a/kernel/params.c b/kernel/params.c index ef60db14fae0..a3eeeefc9472 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -88,7 +88,7 @@ static inline int parameq(const char *input, const char *paramname) static int parse_one(char *param, char *val, - struct kernel_param *params, + const struct kernel_param *params, unsigned num_params, int (*handle_unknown)(char *param, char *val)) { @@ -170,7 +170,7 @@ static char *next_arg(char *args, char **param, char **val) /* Args looks like "foo=bar,bar2 baz=fuz wiz". */ int parse_args(const char *name, char *args, - struct kernel_param *params, + const struct kernel_param *params, unsigned num, int (*unknown)(char *param, char *val)) { From 907b29eb41aa604477a655bff7345731da94514d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:19 -0600 Subject: [PATCH 12/22] param: locking for kernel parameters There may be cases (most obviously, sysfs-writable charp parameters) where a module needs to prevent sysfs access to parameters. Rather than express this in terms of a big lock, the functions are expressed in terms of what they protect against. This is clearer, esp. if the implementation changes to a module-level or even param-level lock. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Tested-by: Phil Carmody --- include/linux/moduleparam.h | 56 +++++++++++++++++++++++++++++++++++++ kernel/params.c | 33 +++++++++++++++++----- 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 6d48831fe7d2..ca74a3402d63 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -130,6 +130,62 @@ __check_old_set_param(int (*oldset)(const char *, struct kernel_param *)) #define module_param(name, type, perm) \ module_param_named(name, name, type, perm) +/** + * kparam_block_sysfs_write - make sure a parameter isn't written via sysfs. + * @name: the name of the parameter + * + * There's no point blocking write on a paramter that isn't writable via sysfs! + */ +#define kparam_block_sysfs_write(name) \ + do { \ + BUG_ON(!(__param_##name.perm & 0222)); \ + __kernel_param_lock(); \ + } while (0) + +/** + * kparam_unblock_sysfs_write - allows sysfs to write to a parameter again. + * @name: the name of the parameter + */ +#define kparam_unblock_sysfs_write(name) \ + do { \ + BUG_ON(!(__param_##name.perm & 0222)); \ + __kernel_param_unlock(); \ + } while (0) + +/** + * kparam_block_sysfs_read - make sure a parameter isn't read via sysfs. + * @name: the name of the parameter + * + * This also blocks sysfs writes. + */ +#define kparam_block_sysfs_read(name) \ + do { \ + BUG_ON(!(__param_##name.perm & 0444)); \ + __kernel_param_lock(); \ + } while (0) + +/** + * kparam_unblock_sysfs_read - allows sysfs to read a parameter again. + * @name: the name of the parameter + */ +#define kparam_unblock_sysfs_read(name) \ + do { \ + BUG_ON(!(__param_##name.perm & 0444)); \ + __kernel_param_unlock(); \ + } while (0) + +#ifdef CONFIG_SYSFS +extern void __kernel_param_lock(void); +extern void __kernel_param_unlock(void); +#else +static inline void __kernel_param_lock(void) +{ +} +static inline void __kernel_param_unlock(void) +{ +} +#endif + #ifndef MODULE /** * core_param - define a historical core kernel parameter. diff --git a/kernel/params.c b/kernel/params.c index a3eeeefc9472..08107d181758 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -31,12 +31,14 @@ #define DEBUGP(fmt, a...) #endif +/* Protects all parameters, and incidentally kmalloced_param list. */ +static DEFINE_MUTEX(param_lock); + /* This just allows us to keep track of which parameters are kmalloced. */ struct kmalloced_param { struct list_head list; char val[]; }; -static DEFINE_MUTEX(param_lock); static LIST_HEAD(kmalloced_params); static void *kmalloc_parameter(unsigned int size) @@ -47,10 +49,7 @@ static void *kmalloc_parameter(unsigned int size) if (!p) return NULL; - mutex_lock(¶m_lock); list_add(&p->list, &kmalloced_params); - mutex_unlock(¶m_lock); - return p->val; } @@ -59,7 +58,6 @@ static void maybe_kfree_parameter(void *param) { struct kmalloced_param *p; - mutex_lock(¶m_lock); list_for_each_entry(p, &kmalloced_params, list) { if (p->val == param) { list_del(&p->list); @@ -67,7 +65,6 @@ static void maybe_kfree_parameter(void *param) break; } } - mutex_unlock(¶m_lock); } static inline char dash2underscore(char c) @@ -93,6 +90,7 @@ static int parse_one(char *param, int (*handle_unknown)(char *param, char *val)) { unsigned int i; + int err; /* Find parameter */ for (i = 0; i < num_params; i++) { @@ -102,7 +100,10 @@ static int parse_one(char *param, return -EINVAL; DEBUGP("They are equal! Calling %p\n", params[i].ops->set); - return params[i].ops->set(val, ¶ms[i]); + mutex_lock(¶m_lock); + err = params[i].ops->set(val, ¶ms[i]); + mutex_unlock(¶m_lock); + return err; } } @@ -400,6 +401,7 @@ static int param_array(const char *name, /* nul-terminate and parse */ save = val[len]; ((char *)val)[len] = '\0'; + BUG_ON(!mutex_is_locked(¶m_lock)); ret = set(val, &kp); if (ret != 0) @@ -438,6 +440,7 @@ static int param_array_get(char *buffer, const struct kernel_param *kp) if (i) buffer[off++] = ','; p.arg = arr->elem + arr->elemsize * i; + BUG_ON(!mutex_is_locked(¶m_lock)); ret = arr->ops->get(buffer + off, &p); if (ret < 0) return ret; @@ -522,7 +525,9 @@ static ssize_t param_attr_show(struct module_attribute *mattr, if (!attribute->param->ops->get) return -EPERM; + mutex_lock(¶m_lock); count = attribute->param->ops->get(buf, attribute->param); + mutex_unlock(¶m_lock); if (count > 0) { strcat(buf, "\n"); ++count; @@ -541,7 +546,9 @@ static ssize_t param_attr_store(struct module_attribute *mattr, if (!attribute->param->ops->set) return -EPERM; + mutex_lock(¶m_lock); err = attribute->param->ops->set(buf, attribute->param); + mutex_unlock(¶m_lock); if (!err) return len; return err; @@ -555,6 +562,18 @@ static ssize_t param_attr_store(struct module_attribute *mattr, #endif #ifdef CONFIG_SYSFS +void __kernel_param_lock(void) +{ + mutex_lock(¶m_lock); +} +EXPORT_SYMBOL(__kernel_param_lock); + +void __kernel_param_unlock(void) +{ + mutex_unlock(¶m_lock); +} +EXPORT_SYMBOL(__kernel_param_unlock); + /* * add_sysfs_param - add a parameter to sysfs * @mk: struct module_kobject From 546970bc6afc7fb37447fbac09b82c7884662c21 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:20 -0600 Subject: [PATCH 13/22] param: add kerneldoc to moduleparam.h Also reorders the macros with the most common ones at the top. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Tested-by: Phil Carmody --- include/linux/moduleparam.h | 121 ++++++++++++++++++++++++++++-------- 1 file changed, 95 insertions(+), 26 deletions(-) diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index ca74a3402d63..893549c04265 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -71,6 +71,62 @@ struct kparam_array void *elem; }; +/** + * module_param - typesafe helper for a module/cmdline parameter + * @value: the variable to alter, and exposed parameter name. + * @type: the type of the parameter + * @perm: visibility in sysfs. + * + * @value becomes the module parameter, or (prefixed by KBUILD_MODNAME and a + * ".") the kernel commandline parameter. Note that - is changed to _, so + * the user can use "foo-bar=1" even for variable "foo_bar". + * + * @perm is 0 if the the variable is not to appear in sysfs, or 0444 + * for world-readable, 0644 for root-writable, etc. Note that if it + * is writable, you may need to use kparam_block_sysfs_write() around + * accesses (esp. charp, which can be kfreed when it changes). + * + * The @type is simply pasted to refer to a param_ops_##type and a + * param_check_##type: for convenience many standard types are provided but + * you can create your own by defining those variables. + * + * Standard types are: + * byte, short, ushort, int, uint, long, ulong + * charp: a character pointer + * bool: a bool, values 0/1, y/n, Y/N. + * invbool: the above, only sense-reversed (N = true). + */ +#define module_param(name, type, perm) \ + module_param_named(name, name, type, perm) + +/** + * module_param_named - typesafe helper for a renamed module/cmdline parameter + * @name: a valid C identifier which is the parameter name. + * @value: the actual lvalue to alter. + * @type: the type of the parameter + * @perm: visibility in sysfs. + * + * Usually it's a good idea to have variable names and user-exposed names the + * same, but that's harder if the variable must be non-static or is inside a + * structure. This allows exposure under a different name. + */ +#define module_param_named(name, value, type, perm) \ + param_check_##type(name, &(value)); \ + module_param_cb(name, ¶m_ops_##type, &value, perm); \ + __MODULE_PARM_TYPE(name, #type) + +/** + * module_param_cb - general callback for a module/cmdline parameter + * @name: a valid C identifier which is the parameter name. + * @ops: the set & get operations for this parameter. + * @perm: visibility in sysfs. + * + * The ops can have NULL set or get functions. + */ +#define module_param_cb(name, ops, arg, perm) \ + __module_param_call(MODULE_PARAM_PREFIX, \ + name, ops, arg, __same_type(*(arg), bool), perm) + /* On alpha, ia64 and ppc64 relocations to global data cannot go into read-only sections (which is part of respective UNIX ABI on these platforms). So 'const' makes no sense and even causes compile failures @@ -82,9 +138,7 @@ struct kparam_array #endif /* This is the fundamental function for registering boot/module - parameters. perm sets the visibility in sysfs: 000 means it's - not there, read bits mean it's readable, write bits mean it's - writable. */ + parameters. */ #define __module_param_call(prefix, name, ops, arg, isbool, perm) \ /* Default value instead of permissions? */ \ static int __param_perm_check_##name __attribute__((unused)) = \ @@ -113,23 +167,6 @@ __check_old_set_param(int (*oldset)(const char *, struct kernel_param *)) return 0; } -#define module_param_cb(name, ops, arg, perm) \ - __module_param_call(MODULE_PARAM_PREFIX, \ - name, ops, arg, __same_type(*(arg), bool), perm) - -/* - * Helper functions: type is byte, short, ushort, int, uint, long, - * ulong, charp, bool or invbool, or XXX if you define param_ops_XXX - * and param_check_XXX. - */ -#define module_param_named(name, value, type, perm) \ - param_check_##type(name, &(value)); \ - module_param_cb(name, ¶m_ops_##type, &value, perm); \ - __MODULE_PARM_TYPE(name, #type) - -#define module_param(name, type, perm) \ - module_param_named(name, name, type, perm) - /** * kparam_block_sysfs_write - make sure a parameter isn't written via sysfs. * @name: the name of the parameter @@ -191,7 +228,7 @@ static inline void __kernel_param_unlock(void) * core_param - define a historical core kernel parameter. * @name: the name of the cmdline and sysfs parameter (often the same as var) * @var: the variable - * @type: the type (for param_set_##type and param_get_##type) + * @type: the type of the parameter * @perm: visibility in sysfs * * core_param is just like module_param(), but cannot be modular and @@ -205,7 +242,16 @@ static inline void __kernel_param_unlock(void) &var, __same_type(var, bool), perm) #endif /* !MODULE */ -/* Actually copy string: maxlen param is usually sizeof(string). */ +/** + * module_param_string - a char array parameter + * @name: the name of the parameter + * @string: the string variable + * @len: the maximum length of the string, incl. terminator + * @perm: visibility in sysfs. + * + * This actually copies the string when it's set (unlike type charp). + * @len is usually just sizeof(string). + */ #define module_param_string(name, string, len, perm) \ static const struct kparam_string __param_string_##name \ = { len, string }; \ @@ -294,7 +340,33 @@ extern int param_set_invbool(const char *val, const struct kernel_param *kp); extern int param_get_invbool(char *buffer, const struct kernel_param *kp); #define param_check_invbool(name, p) __param_check(name, p, bool) -/* Comma-separated array: *nump is set to number they actually specified. */ +/** + * module_param_array - a parameter which is an array of some type + * @name: the name of the array variable + * @type: the type, as per module_param() + * @nump: optional pointer filled in with the number written + * @perm: visibility in sysfs + * + * Input and output are as comma-separated values. Commas inside values + * don't work properly (eg. an array of charp). + * + * ARRAY_SIZE(@name) is used to determine the number of elements in the + * array, so the definition must be visible. + */ +#define module_param_array(name, type, nump, perm) \ + module_param_array_named(name, name, type, nump, perm) + +/** + * module_param_array_named - renamed parameter which is an array of some type + * @name: a valid C identifier which is the parameter name + * @array: the name of the array variable + * @type: the type, as per module_param() + * @nump: optional pointer filled in with the number written + * @perm: visibility in sysfs + * + * This exposes a different name than the actual variable name. See + * module_param_named() for why this might be necessary. + */ #define module_param_array_named(name, array, type, nump, perm) \ static const struct kparam_array __param_arr_##name \ = { ARRAY_SIZE(array), nump, ¶m_ops_##type, \ @@ -305,9 +377,6 @@ extern int param_get_invbool(char *buffer, const struct kernel_param *kp); __same_type(array[0], bool), perm); \ __MODULE_PARM_TYPE(name, "array of " #type) -#define module_param_array(name, type, nump, perm) \ - module_param_array_named(name, name, type, nump, perm) - extern struct kernel_param_ops param_array_ops; extern struct kernel_param_ops param_ops_string; From dca41306395eab37e222ff9e72765e692fcc7251 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:21 -0600 Subject: [PATCH 14/22] param: remove unnecessary writable charp sysfs-writable charp arguments need to be locked against modification (since the old ones may be kfreed underneath us). String arguments are much simpler, so use them for small strings (eg. IFNAMSIZ). lkdtm only uses the parameters at module initialization time, so there's not much point making them writable. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Tested-by: Phil Carmody Cc: Andrew Morton Cc: M. Mohan Kumar Cc: Greg Kroah-Hartman Cc: Bartlomiej Zolnierkiewicz Cc: Jeff Mahoney Cc: Julia Lawall Cc: devel@driverdev.osuosl.org --- drivers/misc/lkdtm.c | 4 ++-- drivers/staging/rtl8187se/r8180_core.c | 6 +++--- drivers/staging/rtl8192e/r8192E_core.c | 6 +++--- drivers/staging/rtl8192su/r8192U_core.c | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/misc/lkdtm.c b/drivers/misc/lkdtm.c index 5bfb2a2041b8..ef34de7a8026 100644 --- a/drivers/misc/lkdtm.c +++ b/drivers/misc/lkdtm.c @@ -124,9 +124,9 @@ static int count = DEFAULT_COUNT; module_param(recur_count, int, 0644); MODULE_PARM_DESC(recur_count, " Recursion level for the stack overflow test, "\ "default is 10"); -module_param(cpoint_name, charp, 0644); +module_param(cpoint_name, charp, 0444); MODULE_PARM_DESC(cpoint_name, " Crash Point, where kernel is to be crashed"); -module_param(cpoint_type, charp, 0644); +module_param(cpoint_type, charp, 0444); MODULE_PARM_DESC(cpoint_type, " Crash Point Type, action to be taken on "\ "hitting the crash point"); module_param(cpoint_count, int, 0644); diff --git a/drivers/staging/rtl8187se/r8180_core.c b/drivers/staging/rtl8187se/r8180_core.c index 49ab9fa9ffa7..ed7457bc24ea 100644 --- a/drivers/staging/rtl8187se/r8180_core.c +++ b/drivers/staging/rtl8187se/r8180_core.c @@ -61,7 +61,7 @@ static struct pci_device_id rtl8180_pci_id_tbl[] __devinitdata = { }; -static char *ifname = "wlan%d"; +static char ifname[IFNAMSIZ] = "wlan%d"; static int hwseqnum = 0; static int hwwep = 0; static int channels = 0x3fff; @@ -72,7 +72,7 @@ MODULE_AUTHOR("Andrea Merello "); MODULE_DESCRIPTION("Linux driver for Realtek RTL8180 / RTL8185 WiFi cards"); -module_param(ifname, charp, S_IRUGO|S_IWUSR); +module_param_string(ifname, ifname, sizeof(ifname), S_IRUGO|S_IWUSR); module_param(hwseqnum, int, S_IRUGO|S_IWUSR); module_param(hwwep, int, S_IRUGO|S_IWUSR); module_param(channels, int, S_IRUGO|S_IWUSR); @@ -3609,7 +3609,7 @@ static int __devinit rtl8180_pci_probe(struct pci_dev *pdev, if (dev_alloc_name(dev, ifname) < 0) { DMESG("Oops: devname already taken! Trying wlan%%d...\n"); - ifname = "wlan%d"; + strcpy(ifname, "wlan%d"); dev_alloc_name(dev, ifname); } diff --git a/drivers/staging/rtl8192e/r8192E_core.c b/drivers/staging/rtl8192e/r8192E_core.c index 4cd071adf84b..17a806f9ee77 100644 --- a/drivers/staging/rtl8192e/r8192E_core.c +++ b/drivers/staging/rtl8192e/r8192E_core.c @@ -112,7 +112,7 @@ static const struct pci_device_id rtl8192_pci_id_tbl[] __devinitdata = { {} }; -static char* ifname = "wlan%d"; +static char ifname[IFNAMSIZ] = "wlan%d"; static int hwwep = 1; //default use hw. set 0 to use software security static int channels = 0x3fff; @@ -123,7 +123,7 @@ MODULE_DEVICE_TABLE(pci, rtl8192_pci_id_tbl); MODULE_DESCRIPTION("Linux driver for Realtek RTL819x WiFi cards"); -module_param(ifname, charp, S_IRUGO|S_IWUSR ); +module_param_string(ifname, ifname, sizeof(ifname), S_IRUGO|S_IWUSR); //module_param(hwseqnum,int, S_IRUGO|S_IWUSR); module_param(hwwep,int, S_IRUGO|S_IWUSR); module_param(channels,int, S_IRUGO|S_IWUSR); @@ -6446,7 +6446,7 @@ static int __devinit rtl8192_pci_probe(struct pci_dev *pdev, if (dev_alloc_name(dev, ifname) < 0){ RT_TRACE(COMP_INIT, "Oops: devname already taken! Trying wlan%%d...\n"); - ifname = "wlan%d"; + strcpy(ifname, "wlan%d"); dev_alloc_name(dev, ifname); } diff --git a/drivers/staging/rtl8192su/r8192U_core.c b/drivers/staging/rtl8192su/r8192U_core.c index 6970c97713d8..df5b52baf893 100644 --- a/drivers/staging/rtl8192su/r8192U_core.c +++ b/drivers/staging/rtl8192su/r8192U_core.c @@ -144,13 +144,13 @@ MODULE_VERSION("V 1.1"); MODULE_DEVICE_TABLE(usb, rtl8192_usb_id_tbl); MODULE_DESCRIPTION("Linux driver for Realtek RTL8192 USB WiFi cards"); -static char* ifname = "wlan%d"; +static char ifname[IFNAMSIZ] = "wlan%d"; static int hwwep = 1; //default use hw. set 0 to use software security static int channels = 0x3fff; -module_param(ifname, charp, S_IRUGO|S_IWUSR ); +module_param_string(ifname, ifname, sizeof(ifname), S_IRUGO|S_IWUSR); //module_param(hwseqnum,int, S_IRUGO|S_IWUSR); module_param(hwwep,int, S_IRUGO|S_IWUSR); module_param(channels,int, S_IRUGO|S_IWUSR); @@ -7406,7 +7406,7 @@ static int __devinit rtl8192_usb_probe(struct usb_interface *intf, if (dev_alloc_name(dev, ifname) < 0){ RT_TRACE(COMP_INIT, "Oops: devname already taken! Trying wlan%%d...\n"); - ifname = "wlan%d"; + strcpy(ifname, "wlan%d"); dev_alloc_name(dev, ifname); } From d6d1b650ae6acce73d55dd0246de22180303ae73 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:27 -0600 Subject: [PATCH 15/22] param: simple locking for sysfs-writable charp parameters Since the writing to sysfs can free the old one, we need to block that when we access the charp variables. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Tested-by: Phil Carmody Cc: Jeff Dike Cc: Dan Williams Cc: John W. Linville Cc: Jing Huang Cc: James E.J. Bottomley Cc: Greg Kroah-Hartman Cc: Johannes Berg Cc: David S. Miller Cc: user-mode-linux-devel@lists.sourceforge.net Cc: libertas-dev@lists.infradead.org Cc: linux-wireless@vger.kernel.org Cc: netdev@vger.kernel.org Cc: linux-scsi@vger.kernel.org Cc: linux-usb@vger.kernel.org --- arch/um/drivers/hostaudio_kern.c | 10 ++++++++++ drivers/net/wireless/libertas/if_usb.c | 3 +++ drivers/net/wireless/libertas_tf/if_usb.c | 3 +++ drivers/scsi/bfa/bfad.c | 2 ++ drivers/usb/atm/ueagle-atm.c | 2 ++ drivers/video/vt8623fb.c | 2 ++ net/mac80211/rate.c | 2 ++ 7 files changed, 24 insertions(+) diff --git a/arch/um/drivers/hostaudio_kern.c b/arch/um/drivers/hostaudio_kern.c index 68142df76608..0c46e398cd8f 100644 --- a/arch/um/drivers/hostaudio_kern.c +++ b/arch/um/drivers/hostaudio_kern.c @@ -187,7 +187,9 @@ static int hostaudio_open(struct inode *inode, struct file *file) int ret; #ifdef DEBUG + kparam_block_sysfs_write(dsp); printk(KERN_DEBUG "hostaudio: open called (host: %s)\n", dsp); + kparam_unblock_sysfs_write(dsp); #endif state = kmalloc(sizeof(struct hostaudio_state), GFP_KERNEL); @@ -199,9 +201,11 @@ static int hostaudio_open(struct inode *inode, struct file *file) if (file->f_mode & FMODE_WRITE) w = 1; + kparam_block_sysfs_write(dsp); lock_kernel(); ret = os_open_file(dsp, of_set_rw(OPENFLAGS(), r, w), 0); unlock_kernel(); + kparam_unblock_sysfs_write(dsp); if (ret < 0) { kfree(state); @@ -258,13 +262,17 @@ static int hostmixer_open_mixdev(struct inode *inode, struct file *file) if (file->f_mode & FMODE_WRITE) w = 1; + kparam_block_sysfs_write(mixer); lock_kernel(); ret = os_open_file(mixer, of_set_rw(OPENFLAGS(), r, w), 0); unlock_kernel(); + kparam_unblock_sysfs_write(mixer); if (ret < 0) { + kparam_block_sysfs_write(dsp); printk(KERN_ERR "hostaudio_open_mixdev failed to open '%s', " "err = %d\n", dsp, -ret); + kparam_unblock_sysfs_write(dsp); kfree(state); return ret; } @@ -320,8 +328,10 @@ MODULE_LICENSE("GPL"); static int __init hostaudio_init_module(void) { + __kernel_param_lock(); printk(KERN_INFO "UML Audio Relay (host dsp = %s, host mixer = %s)\n", dsp, mixer); + __kernel_param_unlock(); module_data.dev_audio = register_sound_dsp(&hostaudio_fops, -1); if (module_data.dev_audio < 0) { diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c index 07ece9d26c63..3ff61063671a 100644 --- a/drivers/net/wireless/libertas/if_usb.c +++ b/drivers/net/wireless/libertas/if_usb.c @@ -289,10 +289,13 @@ static int if_usb_probe(struct usb_interface *intf, } /* Upload firmware */ + kparam_block_sysfs_write(fw_name); if (__if_usb_prog_firmware(cardp, lbs_fw_name, BOOT_CMD_FW_BY_USB)) { + kparam_unblock_sysfs_write(fw_name); lbs_deb_usbd(&udev->dev, "FW upload failed\n"); goto err_prog_firmware; } + kparam_unblock_sysfs_write(fw_name); if (!(priv = lbs_add_card(cardp, &udev->dev))) goto err_prog_firmware; diff --git a/drivers/net/wireless/libertas_tf/if_usb.c b/drivers/net/wireless/libertas_tf/if_usb.c index b172f5d87a3b..41a4f214ade1 100644 --- a/drivers/net/wireless/libertas_tf/if_usb.c +++ b/drivers/net/wireless/libertas_tf/if_usb.c @@ -811,12 +811,15 @@ static int if_usb_prog_firmware(struct if_usb_card *cardp) lbtf_deb_enter(LBTF_DEB_USB); + kparam_block_sysfs_write(fw_name); ret = request_firmware(&cardp->fw, lbtf_fw_name, &cardp->udev->dev); if (ret < 0) { pr_err("request_firmware() failed with %#x\n", ret); pr_err("firmware %s not found\n", lbtf_fw_name); + kparam_unblock_sysfs_write(fw_name); goto done; } + kparam_unblock_sysfs_write(fw_name); if (check_fwfile_format(cardp->fw->data, cardp->fw->size)) goto release_fw; diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c index 915a29d6c7ad..ca04cc9d332f 100644 --- a/drivers/scsi/bfa/bfad.c +++ b/drivers/scsi/bfa/bfad.c @@ -788,6 +788,7 @@ bfad_drv_init(struct bfad_s *bfad) memset(&driver_info, 0, sizeof(driver_info)); strncpy(driver_info.version, BFAD_DRIVER_VERSION, sizeof(driver_info.version) - 1); + __kernel_param_lock(); if (host_name) strncpy(driver_info.host_machine_name, host_name, sizeof(driver_info.host_machine_name) - 1); @@ -797,6 +798,7 @@ bfad_drv_init(struct bfad_s *bfad) if (os_patch) strncpy(driver_info.host_os_patch, os_patch, sizeof(driver_info.host_os_patch) - 1); + __kernel_param_unlock(); strncpy(driver_info.os_device_name, bfad->pci_name, sizeof(driver_info.os_device_name - 1)); diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c index 5b3f555e01c9..ea071a5b6eee 100644 --- a/drivers/usb/atm/ueagle-atm.c +++ b/drivers/usb/atm/ueagle-atm.c @@ -1577,6 +1577,7 @@ static void cmvs_file_name(struct uea_softc *sc, char *const cmv_name, int ver) char file_arr[] = "CMVxy.bin"; char *file; + kparam_block_sysfs_write(cmv_file); /* set proper name corresponding modem version and line type */ if (cmv_file[sc->modem_index] == NULL) { if (UEA_CHIP_VERSION(sc) == ADI930) @@ -1595,6 +1596,7 @@ static void cmvs_file_name(struct uea_softc *sc, char *const cmv_name, int ver) strlcat(cmv_name, file, UEA_FW_NAME_MAX); if (ver == 2) strlcat(cmv_name, ".v2", UEA_FW_NAME_MAX); + kparam_unblock_sysfs_write(cmv_file); } static int request_cmvs_old(struct uea_softc *sc, diff --git a/drivers/video/vt8623fb.c b/drivers/video/vt8623fb.c index d31dc96f838a..85d76ec4c63e 100644 --- a/drivers/video/vt8623fb.c +++ b/drivers/video/vt8623fb.c @@ -726,7 +726,9 @@ static int __devinit vt8623_pci_probe(struct pci_dev *dev, const struct pci_devi /* Prepare startup mode */ + kparam_block_sysfs_write(mode_option); rc = fb_find_mode(&(info->var), info, mode_option, NULL, 0, NULL, 8); + kparam_unblock_sysfs_write(mode_option); if (! ((rc == 1) || (rc == 2))) { rc = -EINVAL; dev_err(info->device, "mode %s not found\n", mode_option); diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c index 6d0bd198af19..be04d46110fe 100644 --- a/net/mac80211/rate.c +++ b/net/mac80211/rate.c @@ -103,6 +103,7 @@ ieee80211_rate_control_ops_get(const char *name) struct rate_control_ops *ops; const char *alg_name; + kparam_block_sysfs_write(ieee80211_default_rc_algo); if (!name) alg_name = ieee80211_default_rc_algo; else @@ -120,6 +121,7 @@ ieee80211_rate_control_ops_get(const char *name) /* try built-in one if specific alg requested but not found */ if (!ops && strlen(CONFIG_MAC80211_RC_DEFAULT)) ops = ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT); + kparam_unblock_sysfs_write(ieee80211_default_rc_algo); return ops; } From 7d3510356b066bcfa9898ec3f90c9c7810ba6ed7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:31 -0600 Subject: [PATCH 16/22] param: lock myri10ge_fw_name against sysfs changes. Since it can be changed via sysfs, we need to make a copy. This most generic way of doing this is to keep a flag so we know when to free it. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Cc: Andrew Gallatin Cc: Brice Goglin Cc: netdev@vger.kernel.org --- drivers/net/myri10ge/myri10ge.c | 54 +++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c index d771d1650d60..fb2c0927d3cc 100644 --- a/drivers/net/myri10ge/myri10ge.c +++ b/drivers/net/myri10ge/myri10ge.c @@ -239,6 +239,7 @@ struct myri10ge_priv { int watchdog_resets; int watchdog_pause; int pause; + bool fw_name_allocated; char *fw_name; char eeprom_strings[MYRI10GE_EEPROM_STRINGS_SIZE]; char *product_code_string; @@ -271,6 +272,7 @@ MODULE_FIRMWARE("myri10ge_eth_z8e.dat"); MODULE_FIRMWARE("myri10ge_rss_ethp_z8e.dat"); MODULE_FIRMWARE("myri10ge_rss_eth_z8e.dat"); +/* Careful: must be accessed under kparam_block_sysfs_write */ static char *myri10ge_fw_name = NULL; module_param(myri10ge_fw_name, charp, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(myri10ge_fw_name, "Firmware image name"); @@ -376,6 +378,14 @@ static inline void put_be32(__be32 val, __be32 __iomem * p) static struct net_device_stats *myri10ge_get_stats(struct net_device *dev); +static void set_fw_name(struct myri10ge_priv *mgp, char *name, bool allocated) +{ + if (mgp->fw_name_allocated) + kfree(mgp->fw_name); + mgp->fw_name = name; + mgp->fw_name_allocated = allocated; +} + static int myri10ge_send_cmd(struct myri10ge_priv *mgp, u32 cmd, struct myri10ge_cmd *data, int atomic) @@ -747,7 +757,7 @@ static int myri10ge_load_firmware(struct myri10ge_priv *mgp, int adopt) dev_warn(&mgp->pdev->dev, "via hotplug\n"); } - mgp->fw_name = "adopted"; + set_fw_name(mgp, "adopted", false); mgp->tx_boundary = 2048; myri10ge_dummy_rdma(mgp, 1); status = myri10ge_get_firmware_capabilities(mgp); @@ -3233,7 +3243,7 @@ static void myri10ge_firmware_probe(struct myri10ge_priv *mgp) * load the optimized firmware (which assumes aligned PCIe * completions) in order to see if it works on this host. */ - mgp->fw_name = myri10ge_fw_aligned; + set_fw_name(mgp, myri10ge_fw_aligned, false); status = myri10ge_load_firmware(mgp, 1); if (status != 0) { goto abort; @@ -3261,7 +3271,7 @@ static void myri10ge_firmware_probe(struct myri10ge_priv *mgp) abort: /* fall back to using the unaligned firmware */ mgp->tx_boundary = 2048; - mgp->fw_name = myri10ge_fw_unaligned; + set_fw_name(mgp, myri10ge_fw_unaligned, false); } @@ -3284,7 +3294,7 @@ static void myri10ge_select_firmware(struct myri10ge_priv *mgp) dev_info(&mgp->pdev->dev, "PCIE x%d Link\n", link_width); mgp->tx_boundary = 4096; - mgp->fw_name = myri10ge_fw_aligned; + set_fw_name(mgp, myri10ge_fw_aligned, false); } else { myri10ge_firmware_probe(mgp); } @@ -3293,22 +3303,29 @@ static void myri10ge_select_firmware(struct myri10ge_priv *mgp) dev_info(&mgp->pdev->dev, "Assuming aligned completions (forced)\n"); mgp->tx_boundary = 4096; - mgp->fw_name = myri10ge_fw_aligned; + set_fw_name(mgp, myri10ge_fw_aligned, false); } else { dev_info(&mgp->pdev->dev, "Assuming unaligned completions (forced)\n"); mgp->tx_boundary = 2048; - mgp->fw_name = myri10ge_fw_unaligned; + set_fw_name(mgp, myri10ge_fw_unaligned, false); } } + + kparam_block_sysfs_write(myri10ge_fw_name); if (myri10ge_fw_name != NULL) { - overridden = 1; - mgp->fw_name = myri10ge_fw_name; + char *fw_name = kstrdup(myri10ge_fw_name, GFP_KERNEL); + if (fw_name) { + overridden = 1; + set_fw_name(mgp, fw_name, true); + } } + kparam_unblock_sysfs_write(myri10ge_fw_name); + if (mgp->board_number < MYRI10GE_MAX_BOARDS && myri10ge_fw_names[mgp->board_number] != NULL && strlen(myri10ge_fw_names[mgp->board_number])) { - mgp->fw_name = myri10ge_fw_names[mgp->board_number]; + set_fw_name(mgp, myri10ge_fw_names[mgp->board_number], false); overridden = 1; } if (overridden) @@ -3660,6 +3677,7 @@ static void myri10ge_probe_slices(struct myri10ge_priv *mgp) struct myri10ge_cmd cmd; struct pci_dev *pdev = mgp->pdev; char *old_fw; + bool old_allocated; int i, status, ncpus, msix_cap; mgp->num_slices = 1; @@ -3672,17 +3690,23 @@ static void myri10ge_probe_slices(struct myri10ge_priv *mgp) /* try to load the slice aware rss firmware */ old_fw = mgp->fw_name; + old_allocated = mgp->fw_name_allocated; + /* don't free old_fw if we override it. */ + mgp->fw_name_allocated = false; + if (myri10ge_fw_name != NULL) { dev_info(&mgp->pdev->dev, "overriding rss firmware to %s\n", myri10ge_fw_name); - mgp->fw_name = myri10ge_fw_name; + set_fw_name(mgp, myri10ge_fw_name, false); } else if (old_fw == myri10ge_fw_aligned) - mgp->fw_name = myri10ge_fw_rss_aligned; + set_fw_name(mgp, myri10ge_fw_rss_aligned, false); else - mgp->fw_name = myri10ge_fw_rss_unaligned; + set_fw_name(mgp, myri10ge_fw_rss_unaligned, false); status = myri10ge_load_firmware(mgp, 0); if (status != 0) { dev_info(&pdev->dev, "Rss firmware not found\n"); + if (old_allocated) + kfree(old_fw); return; } @@ -3747,6 +3771,8 @@ static void myri10ge_probe_slices(struct myri10ge_priv *mgp) mgp->num_slices); if (status == 0) { pci_disable_msix(pdev); + if (old_allocated) + kfree(old_fw); return; } if (status > 0) @@ -3763,7 +3789,7 @@ disable_msix: abort_with_fw: mgp->num_slices = 1; - mgp->fw_name = old_fw; + set_fw_name(mgp, old_fw, old_allocated); myri10ge_load_firmware(mgp, 0); } @@ -3993,6 +4019,7 @@ abort_with_enabled: pci_disable_device(pdev); abort_with_netdev: + set_fw_name(mgp, NULL, false); free_netdev(netdev); return status; } @@ -4037,6 +4064,7 @@ static void myri10ge_remove(struct pci_dev *pdev) dma_free_coherent(&pdev->dev, sizeof(*mgp->cmd), mgp->cmd, mgp->cmd_bus); + set_fw_name(mgp, NULL, false); free_netdev(netdev); pci_disable_device(pdev); pci_set_drvdata(pdev, NULL); From 886275ce41a9751117367fb387ed171049eb6148 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:33 -0600 Subject: [PATCH 17/22] param: lock if_sdio's lbs_helper_name and lbs_fw_name against sysfs changes. Since it can be changed via sysfs, we need to make a copy. This most generic way of doing this is to keep a flag so we know when to free it. Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai Cc: Dan Williams Cc: John W. Linville Cc: libertas-dev@lists.infradead.org Cc: linux-wireless@vger.kernel.org Cc: netdev@vger.kernel.org --- drivers/net/wireless/libertas/if_sdio.c | 32 +++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c index 6e71346a7550..ba854c70ab94 100644 --- a/drivers/net/wireless/libertas/if_sdio.c +++ b/drivers/net/wireless/libertas/if_sdio.c @@ -125,6 +125,8 @@ struct if_sdio_card { const char *helper; const char *firmware; + bool helper_allocated; + bool firmware_allocated; u8 buffer[65536]; @@ -984,16 +986,34 @@ static int if_sdio_probe(struct sdio_func *func, card->helper = if_sdio_models[i].helper; card->firmware = if_sdio_models[i].firmware; + kparam_block_sysfs_write(helper_name); if (lbs_helper_name) { + char *helper = kstrdup(lbs_helper_name, GFP_KERNEL); + if (!helper) { + kparam_unblock_sysfs_write(helper_name); + ret = -ENOMEM; + goto free; + } lbs_deb_sdio("overriding helper firmware: %s\n", lbs_helper_name); - card->helper = lbs_helper_name; + card->helper = helper; + card->helper_allocated = true; } + kparam_unblock_sysfs_write(helper_name); + kparam_block_sysfs_write(fw_name); if (lbs_fw_name) { + char *fw_name = kstrdup(lbs_fw_name, GFP_KERNEL); + if (!fw_name) { + kparam_unblock_sysfs_write(fw_name); + ret = -ENOMEM; + goto free; + } lbs_deb_sdio("overriding firmware: %s\n", lbs_fw_name); - card->firmware = lbs_fw_name; + card->firmware = fw_name; + card->firmware_allocated = true; } + kparam_unblock_sysfs_write(fw_name); sdio_claim_host(func); @@ -1127,6 +1147,10 @@ free: kfree(packet); } + if (card->helper_allocated) + kfree(card->helper); + if (card->firmware_allocated) + kfree(card->firmware); kfree(card); goto out; @@ -1177,6 +1201,10 @@ static void if_sdio_remove(struct sdio_func *func) kfree(packet); } + if (card->helper_allocated) + kfree(card->helper); + if (card->firmware_allocated) + kfree(card->firmware); kfree(card); lbs_deb_leave(LBS_DEB_SDIO); From c8ba6c52e19c13c2b6fb9ca9e5188799c753914c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:37 -0600 Subject: [PATCH 18/22] param: update drivers/char/ipmi/ipmi_watchdog.c to new scheme This is one of the most interesting users of module parameters in the tree, so weaning it off the old-style non-const module_param_call scheme is a useful exercise. I was confused by set_param_int/get_param_int (vs. the normal param_set_int and param_get_int), so I renamed set_param_int to set_param_timeout, and re-used param_get_int directly instead of get_param_int. I also implemented param_check_wdog_ifnum and param_check_timeout, so now the ifnum_to_use and timeout/pretimeout parameters can just use plain module_param(). Signed-off-by: Rusty Russell Cc: Corey Minyard Cc: openipmi-developer@lists.sourceforge.net --- drivers/char/ipmi/ipmi_watchdog.c | 42 ++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c index 82bcdb262a3a..654d566ca57c 100644 --- a/drivers/char/ipmi/ipmi_watchdog.c +++ b/drivers/char/ipmi/ipmi_watchdog.c @@ -196,7 +196,7 @@ static void ipmi_unregister_watchdog(int ipmi_intf); */ static int start_now; -static int set_param_int(const char *val, struct kernel_param *kp) +static int set_param_timeout(const char *val, const struct kernel_param *kp) { char *endp; int l; @@ -215,10 +215,11 @@ static int set_param_int(const char *val, struct kernel_param *kp) return rv; } -static int get_param_int(char *buffer, struct kernel_param *kp) -{ - return sprintf(buffer, "%i", *((int *)kp->arg)); -} +static struct kernel_param_ops param_ops_timeout = { + .set = set_param_timeout, + .get = param_get_int, +}; +#define param_check_timeout param_check_int typedef int (*action_fn)(const char *intval, char *outval); @@ -227,7 +228,7 @@ static int preaction_op(const char *inval, char *outval); static int preop_op(const char *inval, char *outval); static void check_parms(void); -static int set_param_str(const char *val, struct kernel_param *kp) +static int set_param_str(const char *val, const struct kernel_param *kp) { action_fn fn = (action_fn) kp->arg; int rv = 0; @@ -251,7 +252,7 @@ static int set_param_str(const char *val, struct kernel_param *kp) return rv; } -static int get_param_str(char *buffer, struct kernel_param *kp) +static int get_param_str(char *buffer, const struct kernel_param *kp) { action_fn fn = (action_fn) kp->arg; int rv; @@ -263,7 +264,7 @@ static int get_param_str(char *buffer, struct kernel_param *kp) } -static int set_param_wdog_ifnum(const char *val, struct kernel_param *kp) +static int set_param_wdog_ifnum(const char *val, const struct kernel_param *kp) { int rv = param_set_int(val, kp); if (rv) @@ -276,27 +277,38 @@ static int set_param_wdog_ifnum(const char *val, struct kernel_param *kp) return 0; } -module_param_call(ifnum_to_use, set_param_wdog_ifnum, get_param_int, - &ifnum_to_use, 0644); +static struct kernel_param_ops param_ops_wdog_ifnum = { + .set = set_param_wdog_ifnum, + .get = param_get_int, +}; + +#define param_check_wdog_ifnum param_check_int + +static struct kernel_param_ops param_ops_str = { + .set = set_param_str, + .get = get_param_str, +}; + +module_param(ifnum_to_use, wdog_ifnum, 0644); MODULE_PARM_DESC(ifnum_to_use, "The interface number to use for the watchdog " "timer. Setting to -1 defaults to the first registered " "interface"); -module_param_call(timeout, set_param_int, get_param_int, &timeout, 0644); +module_param(timeout, timeout, 0644); MODULE_PARM_DESC(timeout, "Timeout value in seconds."); -module_param_call(pretimeout, set_param_int, get_param_int, &pretimeout, 0644); +module_param(pretimeout, timeout, 0644); MODULE_PARM_DESC(pretimeout, "Pretimeout value in seconds."); -module_param_call(action, set_param_str, get_param_str, action_op, 0644); +module_param_cb(action, ¶m_ops_str, action_op, 0644); MODULE_PARM_DESC(action, "Timeout action. One of: " "reset, none, power_cycle, power_off."); -module_param_call(preaction, set_param_str, get_param_str, preaction_op, 0644); +module_param_cb(preaction, ¶m_ops_str, preaction_op, 0644); MODULE_PARM_DESC(preaction, "Pretimeout action. One of: " "pre_none, pre_smi, pre_nmi, pre_int."); -module_param_call(preop, set_param_str, get_param_str, preop_op, 0644); +module_param_cb(preop, ¶m_ops_str, preop_op, 0644); MODULE_PARM_DESC(preop, "Pretimeout driver operation. One of: " "preop_none, preop_panic, preop_give_data."); From 1a8bff5b404909436fcf03cac167a76ceaaa5547 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:38 -0600 Subject: [PATCH 19/22] ide: use module_param_named rather than module_param_call It has the additional benefit of typechecking (in this case, an unsigned int). Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai --- drivers/ide/ide.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c index 3cb9c4e056ff..fa896210ed7b 100644 --- a/drivers/ide/ide.c +++ b/drivers/ide/ide.c @@ -177,7 +177,7 @@ EXPORT_SYMBOL_GPL(ide_pci_clk); module_param_named(pci_clock, ide_pci_clk, int, 0); MODULE_PARM_DESC(pci_clock, "PCI bus clock frequency (in MHz)"); -static int ide_set_dev_param_mask(const char *s, struct kernel_param *kp) +static int ide_set_dev_param_mask(const char *s, const struct kernel_param *kp) { int a, b, i, j = 1; unsigned int *dev_param_mask = (unsigned int *)kp->arg; @@ -200,34 +200,40 @@ static int ide_set_dev_param_mask(const char *s, struct kernel_param *kp) return 0; } +static struct kernel_param_ops param_ops_ide_dev_mask = { + .set = ide_set_dev_param_mask +}; + +#define param_check_ide_dev_mask(name, p) param_check_uint(name, p) + static unsigned int ide_nodma; -module_param_call(nodma, ide_set_dev_param_mask, NULL, &ide_nodma, 0); +module_param_named(nodma, ide_nodma, ide_dev_mask, 0); MODULE_PARM_DESC(nodma, "disallow DMA for a device"); static unsigned int ide_noflush; -module_param_call(noflush, ide_set_dev_param_mask, NULL, &ide_noflush, 0); +module_param_named(noflush, ide_noflush, ide_dev_mask, 0); MODULE_PARM_DESC(noflush, "disable flush requests for a device"); static unsigned int ide_nohpa; -module_param_call(nohpa, ide_set_dev_param_mask, NULL, &ide_nohpa, 0); +module_param_named(nohpa, ide_nohpa, ide_dev_mask, 0); MODULE_PARM_DESC(nohpa, "disable Host Protected Area for a device"); static unsigned int ide_noprobe; -module_param_call(noprobe, ide_set_dev_param_mask, NULL, &ide_noprobe, 0); +module_param_named(noprobe, ide_noprobe, ide_dev_mask, 0); MODULE_PARM_DESC(noprobe, "skip probing for a device"); static unsigned int ide_nowerr; -module_param_call(nowerr, ide_set_dev_param_mask, NULL, &ide_nowerr, 0); +module_param_named(nowerr, ide_nowerr, ide_dev_mask, 0); MODULE_PARM_DESC(nowerr, "ignore the ATA_DF bit for a device"); static unsigned int ide_cdroms; -module_param_call(cdrom, ide_set_dev_param_mask, NULL, &ide_cdroms, 0); +module_param_named(cdrom, ide_cdroms, ide_dev_mask, 0); MODULE_PARM_DESC(cdrom, "force device as a CD-ROM"); struct chs_geom { From 57ba4717f2fe3ed441f3225dd9e05f6a0419fb6c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:39 -0600 Subject: [PATCH 20/22] param: use module_param in drivers/message/fusion/mptbase.c No need to open code this! Signed-off-by: Rusty Russell Reviewed-by: Takashi Iwai --- drivers/message/fusion/mptbase.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c index b88a244a1edd..d8c668d64418 100644 --- a/drivers/message/fusion/mptbase.c +++ b/drivers/message/fusion/mptbase.c @@ -109,8 +109,7 @@ MODULE_PARM_DESC(mpt_debug_level, " debug level - refer to mptdebug.h \ int mpt_fwfault_debug; EXPORT_SYMBOL(mpt_fwfault_debug); -module_param_call(mpt_fwfault_debug, param_set_int, param_get_int, - &mpt_fwfault_debug, 0600); +module_param(mpt_fwfault_debug, int, 0600); MODULE_PARM_DESC(mpt_fwfault_debug, "Enable detection of Firmware fault" " and halt Firmware on fault - (default=0)"); From 4ef2db016aab27af05a95aeab1c30ad3f2fed7b9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:39 -0600 Subject: [PATCH 21/22] param: update drivers/acpi/debug.c to new scheme The new module_param_cb() uses an ops struct, and the ops take a const struct kernel_param pointer (it's in .rodata). Signed-off-by: Rusty Russell --- drivers/acpi/debug.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/acpi/debug.c b/drivers/acpi/debug.c index 146135e7a6a1..295dbfa2db9c 100644 --- a/drivers/acpi/debug.c +++ b/drivers/acpi/debug.c @@ -96,7 +96,8 @@ static const struct acpi_dlevel acpi_debug_levels[] = { /* -------------------------------------------------------------------------- FS Interface (/sys) -------------------------------------------------------------------------- */ -static int param_get_debug_layer(char *buffer, struct kernel_param *kp) { +static int param_get_debug_layer(char *buffer, const struct kernel_param *kp) +{ int result = 0; int i; @@ -118,7 +119,8 @@ static int param_get_debug_layer(char *buffer, struct kernel_param *kp) { return result; } -static int param_get_debug_level(char *buffer, struct kernel_param *kp) { +static int param_get_debug_level(char *buffer, const struct kernel_param *kp) +{ int result = 0; int i; @@ -137,8 +139,18 @@ static int param_get_debug_level(char *buffer, struct kernel_param *kp) { return result; } -module_param_call(debug_layer, param_set_uint, param_get_debug_layer, &acpi_dbg_layer, 0644); -module_param_call(debug_level, param_set_uint, param_get_debug_level, &acpi_dbg_level, 0644); +static struct kernel_param_ops acpi_debug_layer_ops = { + .set = param_set_uint, + .get = param_get_debug_layer, +}; + +static struct kernel_param_ops acpi_debug_level_ops = { + .set = param_set_uint, + .get = param_get_debug_level, +}; + +module_param_cb(debug_layer, &acpi_debug_layer_ops, &acpi_dbg_layer, 0644); +module_param_cb(debug_level, &acpi_debug_level_ops, &acpi_dbg_level, 0644); static char trace_method_name[6]; module_param_string(trace_method_name, trace_method_name, 6, 0644); @@ -147,7 +159,7 @@ module_param(trace_debug_layer, uint, 0644); static unsigned int trace_debug_level; module_param(trace_debug_level, uint, 0644); -static int param_set_trace_state(const char *val, struct kernel_param *kp) +static int param_set_trace_state(const char *val, const struct kernel_param *kp) { int result = 0; @@ -181,7 +193,7 @@ exit: return result; } -static int param_get_trace_state(char *buffer, struct kernel_param *kp) +static int param_get_trace_state(char *buffer, const struct kernel_param *kp) { if (!acpi_gbl_trace_method_name) return sprintf(buffer, "disable"); @@ -194,8 +206,12 @@ static int param_get_trace_state(char *buffer, struct kernel_param *kp) return 0; } -module_param_call(trace_state, param_set_trace_state, param_get_trace_state, - NULL, 0644); +static struct kernel_param_ops param_ops_trace_state = { + .set = param_set_trace_state, + .get = param_get_trace_state, +}; + +module_param_cb(trace_state, ¶m_ops_trace_state, NULL, 0644); /* -------------------------------------------------------------------------- DebugFS Interface From a6de51b2787012ba3ab62c7d50df1b749b83d5f0 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Aug 2010 23:04:40 -0600 Subject: [PATCH 22/22] param: don't deref arg in __same_type() checks gcc allows this when arg is a function, but sparse complains: drivers/char/ipmi/ipmi_watchdog.c:303:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:307:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:311:1: error: cannot dereference this type Reported-by: Randy Dunlap Tested-by: Randy Dunlap Signed-off-by: Rusty Russell --- include/linux/moduleparam.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 893549c04265..9d2f1837b3d8 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -125,7 +125,7 @@ struct kparam_array */ #define module_param_cb(name, ops, arg, perm) \ __module_param_call(MODULE_PARAM_PREFIX, \ - name, ops, arg, __same_type(*(arg), bool), perm) + name, ops, arg, __same_type((arg), bool *), perm) /* On alpha, ia64 and ppc64 relocations to global data cannot go into read-only sections (which is part of respective UNIX ABI on these @@ -157,7 +157,7 @@ struct kparam_array { (void *)set, (void *)get }; \ __module_param_call(MODULE_PARAM_PREFIX, \ name, &__param_ops_##name, arg, \ - __same_type(*(arg), bool), \ + __same_type(arg, bool *), \ (perm) + sizeof(__check_old_set_param(set))*0) /* We don't get oldget: it's often a new-style param_get_uint, etc. */ @@ -330,9 +330,9 @@ extern int param_get_bool(char *buffer, const struct kernel_param *kp); #define param_check_bool(name, p) \ static inline void __check_##name(void) \ { \ - BUILD_BUG_ON(!__same_type(*(p), bool) && \ - !__same_type(*(p), unsigned int) && \ - !__same_type(*(p), int)); \ + BUILD_BUG_ON(!__same_type((p), bool *) && \ + !__same_type((p), unsigned int *) && \ + !__same_type((p), int *)); \ } extern struct kernel_param_ops param_ops_invbool;