Messages in this thread Patch in this message | | | Date | Tue, 16 Feb 2010 10:52:43 -0500 | From | Neil Horman <> | Subject | [PATCH] Fix up a few missed minor rcu errors in uevent_helper |
| |
Fix up a few remaining nits in uevent_helper
Fixes the remaining missed lock points in uevent_helper, as reported by Paul McKenney in his review. Specifically adds a spin lock to guard the write side of usevent_helper from concurrent writes, and fixes two remaining missed read side points that failed to use rcu_read_lock properly. Also fixe up a minor string parsing issue in uvent_helper_set (was terminating the string on the wrong buffer).
I've done minimal testing on this (as the things this fixes were reported by code audit, not any actual oops or other failure). I did run this for a few hours though, with several processes reading/writing different values to /sys/kernel/uevent_helper, while issued a few module loads that required issuing udev events (which, when overlapping with a proper write to /sys/kernel/uevent), triggered a fork in the appropriate binary. Nothing has fallen over in that time, so I'm satisfied with this. Applies as an incremental on top of the latest -mm Neil
Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Signed-off-by: Neil Horman <nhorman@tuxdriver.com> Tested-by: Neil Horman <nhorman@tuxdriver.com>
kernel/ksysfs.c | 17 ++++++++++++----- lib/kobject_uevent.c | 3 ++- 2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c index 66d1e5b..401d1f0 100644 --- a/kernel/ksysfs.c +++ b/kernel/ksysfs.c @@ -16,6 +16,7 @@ #include <linux/kexec.h> #include <linux/profile.h> #include <linux/sched.h> +#include <linux/spinlock.h> #define KERNEL_ATTR_RO(_name) \ static struct kobj_attribute _name##_attr = __ATTR_RO(_name) @@ -37,13 +38,18 @@ KERNEL_ATTR_RO(uevent_seqnum); static ssize_t uevent_helper_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { - return sprintf(buf, "%s\n", rcu_dereference(uevent_helper)); + ssize_t count; + rcu_read_lock(); + count = sprintf(buf, "%s\n", rcu_dereference(uevent_helper)); + rcu_read_unlock(); + return count; } struct uevent_helper_rcu { char *oldptr; struct rcu_head rcu; }; +static DEFINE_SPINLOCK(uevent_helper_lock); static void free_old_uevent_ptr(struct rcu_head *list) { @@ -68,15 +74,16 @@ static ssize_t uevent_helper_store(struct kobject *kobj, kbuf = kstrndup(buf, UEVENT_HELPER_PATH_LEN, GFP_KERNEL); if (!kbuf) return -ENOMEM; - uevent_helper[count] = '\0'; - if (count && uevent_helper[count-1] == '\n') - uevent_helper[count-1] = '\0'; + kbuf[count] = '\0'; + if (count && kbuf[count-1] == '\n') + kbuf[count-1] = '\0'; old = kmalloc(sizeof(struct uevent_helper_rcu), GFP_KERNEL); if (!old) goto out_free; - + spin_lock(&uevent_helper_lock); old->oldptr = rcu_dereference(uevent_helper); rcu_assign_pointer(uevent_helper, kbuf); + spin_unlock(&uevent_helper_lock); call_rcu(&old->rcu, free_old_uevent_ptr); return count; diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c index 211f846..a7520d0 100644 --- a/lib/kobject_uevent.c +++ b/lib/kobject_uevent.c @@ -273,10 +273,11 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, #endif /* call uevent_helper, usually only enabled during early boot */ + rcu_read_lock(); helper = rcu_dereference(uevent_helper); if (helper[0]) retval = uevent_call_helper(subsystem, env); - + rcu_read_unlock(); exit: kfree(devpath); kfree(env);
| |