(2013/07/04 12:33), Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <[email protected]>
> 
> When one of the event files is opened, we need to prevent them from
> being removed. Modules do with with the module owner set (automated
> from the VFS layer).  The ftrace buffer instances have a ref count added
> to the trace_array when the enabled file is opened (the others are not
> that big of a deal, as they only reference the event calls which
> still exist when an instance disappears). But kprobes and uprobes
> do not have any protection.
> 
>  # cd /sys/kernel/debug/tracing
>  # echo 'p:sigprocmask sigprocmask' > kprobe_events || exit -1
>  # enable_probe() {
>     sleep 10
>     echo 1
>  }
>  # file=events/kprobes/sigprocmask/enable
>  # enable_probe > $file &
>  > kprobe_events
> 
> The above will corrupt the kprobe system, as the write to the enable
> file will happen after the kprobe was deleted.
> 
> Trying to create the probe again fails:
> 
>  # echo 'p:sigprocmask sigprocmask' > kprobe_events
>  # cat kprobe_events
> p:kprobes/sigprocmask sigprocmask
>  # ls events/kprobes/
> enable  filter
> 
> Have the unregister probe fail when the event files are open, in use
> are used by perf.
> 

Now, since the commit a232e270d makes sure that disable_trace_probe()
waits until all running handlers are done, this patch has no problem.

Acked-by: Masami Hiramatsu <[email protected]>

Thank you!


> Signed-off-by: Steven Rostedt <[email protected]>
> ---
>  kernel/trace/trace_kprobe.c |   21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 7ed6976..ffcaf42 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -95,7 +95,7 @@ static __kprobes bool trace_probe_is_on_module(struct 
> trace_probe *tp)
>  }
>  
>  static int register_probe_event(struct trace_probe *tp);
> -static void unregister_probe_event(struct trace_probe *tp);
> +static int unregister_probe_event(struct trace_probe *tp);
>  
>  static DEFINE_MUTEX(probe_lock);
>  static LIST_HEAD(probe_list);
> @@ -340,9 +340,12 @@ static int unregister_trace_probe(struct trace_probe *tp)
>       if (trace_probe_is_enabled(tp))
>               return -EBUSY;
>  
> +     /* Will fail if probe is being used by ftrace or perf */
> +     if (unregister_probe_event(tp))
> +             return -EBUSY;
> +
>       __unregister_trace_probe(tp);
>       list_del(&tp->list);
> -     unregister_probe_event(tp);
>  
>       return 0;
>  }
> @@ -621,7 +624,9 @@ static int release_all_trace_probes(void)
>       /* TODO: Use batch unregistration */
>       while (!list_empty(&probe_list)) {
>               tp = list_entry(probe_list.next, struct trace_probe, list);
> -             unregister_trace_probe(tp);
> +             ret = unregister_trace_probe(tp);
> +             if (ret)
> +                     goto end;
>               free_trace_probe(tp);
>       }
>  
> @@ -1242,11 +1247,15 @@ static int register_probe_event(struct trace_probe 
> *tp)
>       return ret;
>  }
>  
> -static void unregister_probe_event(struct trace_probe *tp)
> +static int unregister_probe_event(struct trace_probe *tp)
>  {
> +     int ret;
> +
>       /* tp->event is unregistered in trace_remove_event_call() */
> -     trace_remove_event_call(&tp->call);
> -     kfree(tp->call.print_fmt);
> +     ret = trace_remove_event_call(&tp->call);
> +     if (!ret)
> +             kfree(tp->call.print_fmt);
> +     return ret;
>  }
>  
>  /* Make a debugfs interface for controlling probe points */
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to