lkml.org 
[lkml]   [2008]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] kprobes: disable preempt for module_text_address() and kernel_text_address()
    __register_kprobe() can be preempted after checking probing address
    but before module_text_address() or try_module_get(), and in this
    interval the module can be unloaded. In that case,
    try_module_get(probed_mod) will access to invalid address,
    or kprobe will probe invalid address.

    this patch uses preempt_disable() to protect it and use
    __module_text_address() and __kernel_text_address().

    Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
    Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
    ---

    >> @@ -718,9 +725,11 @@ static void __kprobes __unregister_kprob
    >> struct kprobe *old_p;
    >>
    >> if (p->mod_refcounted) {
    >> - mod = module_text_address((unsigned long)p->addr);
    >> + preempt_disable();
    >> + mod = __module_text_address((unsigned long)p->addr);
    >> if (mod)
    >> module_put(mod);
    >> + preempt_enable();
    >
    > this is bad fix, we have had a reference to mod. we don't need
    > preempt_disable() before module_put(mod).

    Hmm, Indeed.
    So let's add a comment there.

    kernel/kprobes.c | 21 ++++++++++++++++-----
    1 file changed, 16 insertions(+), 5 deletions(-)

    Index: 2.6.28-rc3/kernel/kprobes.c
    ===================================================================
    --- 2.6.28-rc3.orig/kernel/kprobes.c
    +++ 2.6.28-rc3/kernel/kprobes.c
    @@ -613,30 +613,37 @@ static int __kprobes __register_kprobe(s
    return -EINVAL;
    p->addr = addr;

    - if (!kernel_text_address((unsigned long) p->addr) ||
    - in_kprobes_functions((unsigned long) p->addr))
    + preempt_disable();
    + if (!__kernel_text_address((unsigned long) p->addr) ||
    + in_kprobes_functions((unsigned long) p->addr)) {
    + preempt_enable();
    return -EINVAL;
    + }

    p->mod_refcounted = 0;

    /*
    * Check if are we probing a module.
    */
    - probed_mod = module_text_address((unsigned long) p->addr);
    + probed_mod = __module_text_address((unsigned long) p->addr);
    if (probed_mod) {
    - struct module *calling_mod = module_text_address(called_from);
    + struct module *calling_mod;
    + calling_mod = __module_text_address(called_from);
    /*
    * We must allow modules to probe themself and in this case
    * avoid incrementing the module refcount, so as to allow
    * unloading of self probing modules.
    */
    if (calling_mod && calling_mod != probed_mod) {
    - if (unlikely(!try_module_get(probed_mod)))
    + if (unlikely(!try_module_get(probed_mod))) {
    + preempt_enable();
    return -EINVAL;
    + }
    p->mod_refcounted = 1;
    } else
    probed_mod = NULL;
    }
    + preempt_enable();

    p->nmissed = 0;
    INIT_LIST_HEAD(&p->list);
    @@ -718,6 +725,10 @@ static void __kprobes __unregister_kprob
    struct kprobe *old_p;

    if (p->mod_refcounted) {
    + /*
    + * Since we've already incremented refcount,
    + * we don't need to disable preemption.
    + */
    mod = module_text_address((unsigned long)p->addr);
    if (mod)
    module_put(mod);
    --
    Masami Hiramatsu

    Software Engineer
    Hitachi Computer Products (America) Inc.
    Software Solutions Division

    e-mail: mhiramat@redhat.com



    \
     
     \ /
      Last update: 2008-11-06 16:41    [W:0.027 / U:0.572 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site