lkml.org 
[lkml]   [2010]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: oops in uevent_helper [was: mmotm 2010-01-13-12-17 uploaded]
    On Thu, Feb 11, 2010 at 09:27:08PM -0800, Andrew Morton wrote:
    > On Fri, 12 Feb 2010 06:21:26 +0100 Andi Kleen <andi@firstfloor.org> wrote:
    >
    > > > urgh, must I? That trashes Neil's
    > > > kmod-add-init-function-to-usermodehelper.patch and
    > > > kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function-and-resolve-limit.patch
    > > > and probably requires repairing other stuff and sets the testing status
    > > > back to "square one".
    > > >
    > > > If you have patches queued, please make the time to support them!
    > >
    > > Ok, understood. I'll try to look into it today.
    >
    > Ta. As I mentioned to Neil, if it looks serious then let's shelve it
    > all and revisit for 2.6.35.
    >
    > > You want incrementals?
    >
    > If convenient, please. Otherwise we can drop--and-remerge.
    >
    >


    Ok, this fixes the oops Jiri reported for me. Its been tested by me, but only
    minimally, and my rcu-foo is not the greatest, so through reviews appreciated.
    The patch is incremental against the latest mmotm as of this AM.

    Thanks!
    Neil


    Fix up remaining references to uevent_helper to play nice with Andi's
    uevent_helper/rcu changes.

    Some changes were made recently which modified uevent_helper to be an rcu
    protected pointer, rather than a static char array. This has led to a few
    missed points in which the sysfs path still assumed that:
    1) the uevent_helper symbol could still be accessed safely without
    rcu_dereference
    2) that the sysfs path could copy data to that pointer safely.

    I've fixed this by chaging the sysfs path so that it duplicates the string on
    uevent_helper_store, and freeing it (only if it doesn't point to the
    CONFIG_DEFAULT_UEVENT_HELPER string), in a call_rcu post-quiescent point. I've
    also fixed up the remaining references to the uevent_helper pointers to use
    rcu_dereference.

    Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


    kernel/ksysfs.c | 38 ++++++++++++++++++++++++++++++++++++--
    lib/kobject_uevent.c | 4 +++-
    2 files changed, 39 insertions(+), 3 deletions(-)

    diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
    index 21fe3c4..66d1e5b 100644
    --- a/kernel/ksysfs.c
    +++ b/kernel/ksysfs.c
    @@ -37,19 +37,53 @@ 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", uevent_helper);
    + return sprintf(buf, "%s\n", rcu_dereference(uevent_helper));
    }
    +
    +struct uevent_helper_rcu {
    + char *oldptr;
    + struct rcu_head rcu;
    +};
    +
    +static void free_old_uevent_ptr(struct rcu_head *list)
    +{
    + struct uevent_helper_rcu *ptr;
    + char *dfl = CONFIG_UEVENT_HELPER_PATH;
    + ptr = container_of(list, struct uevent_helper_rcu, rcu);
    + if (ptr->oldptr && (ptr->oldptr != dfl))
    + kfree(ptr->oldptr);
    +
    + kfree(ptr);
    +}
    +
    static ssize_t uevent_helper_store(struct kobject *kobj,
    struct kobj_attribute *attr,
    const char *buf, size_t count)
    {
    + char *kbuf;
    + struct uevent_helper_rcu *old;
    +
    if (count+1 > UEVENT_HELPER_PATH_LEN)
    return -ENOENT;
    - memcpy(uevent_helper, buf, count);
    + 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';
    + old = kmalloc(sizeof(struct uevent_helper_rcu), GFP_KERNEL);
    + if (!old)
    + goto out_free;
    +
    + old->oldptr = rcu_dereference(uevent_helper);
    + rcu_assign_pointer(uevent_helper, kbuf);
    + call_rcu(&old->rcu, free_old_uevent_ptr);
    +
    return count;
    +
    +out_free:
    + kfree(kbuf);
    + return -ENOMEM;
    }
    KERNEL_ATTR_RW(uevent_helper);
    #endif
    diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
    index c2383f3..211f846 100644
    --- a/lib/kobject_uevent.c
    +++ b/lib/kobject_uevent.c
    @@ -126,6 +126,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
    struct kset *kset;
    const struct kset_uevent_ops *uevent_ops;
    u64 seq;
    + const char *helper;
    int i = 0;
    int retval = 0;

    @@ -272,7 +273,8 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
    #endif

    /* call uevent_helper, usually only enabled during early boot */
    - if (uevent_helper[0])
    + helper = rcu_dereference(uevent_helper);
    + if (helper[0])
    retval = uevent_call_helper(subsystem, env);

    exit:

    \
     
     \ /
      Last update: 2010-02-12 18:09    [W:0.038 / U:33.388 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site