From 7eff2e7a8b65c25920207324e56611150eb1cd9a Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Tue, 14 Aug 2007 15:15:12 +0200 Subject: Driver core: change add_uevent_var to use a struct This changes the uevent buffer functions to use a struct instead of a long list of parameters. It does no longer require the caller to do the proper buffer termination and size accounting, which is currently wrong in some places. It fixes a known bug where parts of the uevent environment are overwritten because of wrong index calculations. Many thanks to Mathieu Desnoyers for finding bugs and improving the error handling. Signed-off-by: Kay Sievers Cc: Mathieu Desnoyers Cc: Cornelia Huck Signed-off-by: Greg Kroah-Hartman --- lib/kobject_uevent.c | 149 ++++++++++++++++++++++----------------------------- 1 file changed, 63 insertions(+), 86 deletions(-) (limited to 'lib/kobject_uevent.c') diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c index e06a8dcec0f0..7d8aeb301635 100644 --- a/lib/kobject_uevent.c +++ b/lib/kobject_uevent.c @@ -22,8 +22,6 @@ #include #include -#define BUFFER_SIZE 2048 /* buffer for the variables */ -#define NUM_ENVP 32 /* number of env pointers */ /* the strings here must match the enum in include/linux/kobject.h */ const char *kobject_actions[] = { @@ -54,31 +52,21 @@ static struct sock *uevent_sock; * corresponding error when it fails. */ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, - char *envp_ext[]) + char *envp_ext[]) { - char **envp; - char *buffer; - char *scratch; - const char *action_string; + struct kobj_uevent_env *env; + const char *action_string = kobject_actions[action]; const char *devpath = NULL; const char *subsystem; struct kobject *top_kobj; struct kset *kset; struct kset_uevent_ops *uevent_ops; u64 seq; - char *seq_buff; int i = 0; int retval = 0; - int j; pr_debug("%s\n", __FUNCTION__); - action_string = kobject_actions[action]; - if (!action_string) { - pr_debug("kobject attempted to send uevent without action_string!\n"); - return -EINVAL; - } - /* search the kset we belong to */ top_kobj = kobj; while (!top_kobj->kset && top_kobj->parent) { @@ -92,7 +80,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, kset = top_kobj->kset; uevent_ops = kset->uevent_ops; - /* skip the event, if the filter returns zero. */ + /* skip the event, if the filter returns zero. */ if (uevent_ops && uevent_ops->filter) if (!uevent_ops->filter(kset, kobj)) { pr_debug("kobject filter function caused the event to drop!\n"); @@ -109,18 +97,11 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, return 0; } - /* environment index */ - envp = kzalloc(NUM_ENVP * sizeof (char *), GFP_KERNEL); - if (!envp) + /* environment buffer */ + env = kzalloc(sizeof(struct kobj_uevent_env), GFP_KERNEL); + if (!env) return -ENOMEM; - /* environment values */ - buffer = kmalloc(BUFFER_SIZE, GFP_KERNEL); - if (!buffer) { - retval = -ENOMEM; - goto exit; - } - /* complete object path */ devpath = kobject_get_path(kobj, GFP_KERNEL); if (!devpath) { @@ -128,29 +109,29 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, goto exit; } - /* event environemnt for helper process only */ - envp[i++] = "HOME=/"; - envp[i++] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin"; - /* default keys */ - scratch = buffer; - envp [i++] = scratch; - scratch += sprintf(scratch, "ACTION=%s", action_string) + 1; - envp [i++] = scratch; - scratch += sprintf (scratch, "DEVPATH=%s", devpath) + 1; - envp [i++] = scratch; - scratch += sprintf(scratch, "SUBSYSTEM=%s", subsystem) + 1; - for (j = 0; envp_ext && envp_ext[j]; j++) - envp[i++] = envp_ext[j]; - /* just reserve the space, overwrite it after kset call has returned */ - envp[i++] = seq_buff = scratch; - scratch += strlen("SEQNUM=18446744073709551616") + 1; + retval = add_uevent_var(env, "ACTION=%s", action_string); + if (retval) + goto exit; + retval = add_uevent_var(env, "DEVPATH=%s", devpath); + if (retval) + goto exit; + retval = add_uevent_var(env, "SUBSYSTEM=%s", subsystem); + if (retval) + goto exit; + + /* keys passed in from the caller */ + if (envp_ext) { + for (i = 0; envp_ext[i]; i++) { + retval = add_uevent_var(env, envp_ext[i]); + if (retval) + goto exit; + } + } /* let the kset specific function add its stuff */ if (uevent_ops && uevent_ops->uevent) { - retval = uevent_ops->uevent(kset, kobj, - &envp[i], NUM_ENVP - i, scratch, - BUFFER_SIZE - (scratch - buffer)); + retval = uevent_ops->uevent(kset, kobj, env); if (retval) { pr_debug ("%s - uevent() returned %d\n", __FUNCTION__, retval); @@ -158,11 +139,13 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, } } - /* we will send an event, request a new sequence number */ + /* we will send an event, so request a new sequence number */ spin_lock(&sequence_lock); seq = ++uevent_seqnum; spin_unlock(&sequence_lock); - sprintf(seq_buff, "SEQNUM=%llu", (unsigned long long)seq); + retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)seq); + if (retval) + goto exit; #if defined(CONFIG_NET) /* send netlink message */ @@ -172,17 +155,19 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, /* allocate message with the maximum possible size */ len = strlen(action_string) + strlen(devpath) + 2; - skb = alloc_skb(len + BUFFER_SIZE, GFP_KERNEL); + skb = alloc_skb(len + env->buflen, GFP_KERNEL); if (skb) { + char *scratch; + /* add header */ scratch = skb_put(skb, len); sprintf(scratch, "%s@%s", action_string, devpath); /* copy keys to our continuous event payload buffer */ - for (i = 2; envp[i]; i++) { - len = strlen(envp[i]) + 1; + for (i = 0; i < env->envp_idx; i++) { + len = strlen(env->envp[i]) + 1; scratch = skb_put(skb, len); - strcpy(scratch, envp[i]); + strcpy(scratch, env->envp[i]); } NETLINK_CB(skb).dst_group = 1; @@ -198,13 +183,19 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, argv [0] = uevent_helper; argv [1] = (char *)subsystem; argv [2] = NULL; - call_usermodehelper (argv[0], argv, envp, UMH_WAIT_EXEC); + retval = add_uevent_var(env, "HOME=/"); + if (retval) + goto exit; + retval = add_uevent_var(env, "PATH=/sbin:/bin:/usr/sbin:/usr/bin"); + if (retval) + goto exit; + + call_usermodehelper (argv[0], argv, env->envp, UMH_WAIT_EXEC); } exit: kfree(devpath); - kfree(buffer); - kfree(envp); + kfree(env); return retval; } @@ -227,52 +218,38 @@ int kobject_uevent(struct kobject *kobj, enum kobject_action action) EXPORT_SYMBOL_GPL(kobject_uevent); /** - * add_uevent_var - helper for creating event variables - * @envp: Pointer to table of environment variables, as passed into - * uevent() method. - * @num_envp: Number of environment variable slots available, as - * passed into uevent() method. - * @cur_index: Pointer to current index into @envp. It should be - * initialized to 0 before the first call to add_uevent_var(), - * and will be incremented on success. - * @buffer: Pointer to buffer for environment variables, as passed - * into uevent() method. - * @buffer_size: Length of @buffer, as passed into uevent() method. - * @cur_len: Pointer to current length of space used in @buffer. - * Should be initialized to 0 before the first call to - * add_uevent_var(), and will be incremented on success. - * @format: Format for creating environment variable (of the form - * "XXX=%x") for snprintf(). + * add_uevent_var - add key value string to the environment buffer + * @env: environment buffer structure + * @format: printf format for the key=value pair * * Returns 0 if environment variable was added successfully or -ENOMEM * if no space was available. */ -int add_uevent_var(char **envp, int num_envp, int *cur_index, - char *buffer, int buffer_size, int *cur_len, - const char *format, ...) +int add_uevent_var(struct kobj_uevent_env *env, const char *format, ...) { va_list args; + int len; - /* - * We check against num_envp - 1 to make sure there is at - * least one slot left after we return, since kobject_uevent() - * needs to set the last slot to NULL. - */ - if (*cur_index >= num_envp - 1) + if (env->envp_idx >= ARRAY_SIZE(env->envp)) { + printk(KERN_ERR "add_uevent_var: too many keys\n"); + WARN_ON(1); return -ENOMEM; - - envp[*cur_index] = buffer + *cur_len; + } va_start(args, format); - *cur_len += vsnprintf(envp[*cur_index], - max(buffer_size - *cur_len, 0), - format, args) + 1; + len = vsnprintf(&env->buf[env->buflen], + sizeof(env->buf) - env->buflen, + format, args); va_end(args); - if (*cur_len > buffer_size) + if (len >= (sizeof(env->buf) - env->buflen)) { + printk(KERN_ERR "add_uevent_var: buffer size too small\n"); + WARN_ON(1); return -ENOMEM; + } - (*cur_index)++; + env->envp[env->envp_idx++] = &env->buf[env->buflen]; + env->buflen += len + 1; return 0; } EXPORT_SYMBOL_GPL(add_uevent_var); -- cgit v1.2.3-59-g8ed1b