lkml.org 
[lkml]   [2009]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 7/7] tracing/filters: make filter preds RCU safe
    I noticed ftrace_event_call->preds and ->preds[*] are not protected
    by any lock, and I can manage to trigger kernel oops.

    This patch adds filter_mutex to protect concurrent access to
    event_subsystem->preds/preds[i] or ftrace_event_call->preds/preds[i],
    and make the fast-path filter_match_preds() RCU safe.

    Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
    ---
    kernel/trace/trace.h | 6 +-
    kernel/trace/trace_events.c | 4 +-
    kernel/trace/trace_events_filter.c | 104 +++++++++++++++++++++++++---------
    kernel/trace/trace_events_stage_3.h | 10 +++-
    4 files changed, 91 insertions(+), 33 deletions(-)

    diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
    index e685ac2..ae05fbb 100644
    --- a/kernel/trace/trace.h
    +++ b/kernel/trace/trace.h
    @@ -847,13 +847,15 @@ struct filter_pred {
    int trace_define_field(struct ftrace_event_call *call, char *type,
    char *name, int offset, int size);
    extern void filter_free_pred(struct filter_pred *pred);
    -extern void filter_print_preds(struct filter_pred **preds,
    +extern void filter_print_preds(struct ftrace_event_call *call,
    struct trace_seq *s);
    extern int filter_parse(char **pbuf, struct filter_pred *pred);
    extern int filter_add_pred(struct ftrace_event_call *call,
    struct filter_pred *pred);
    extern void filter_free_preds(struct ftrace_event_call *call);
    -extern int filter_match_preds(struct ftrace_event_call *call, void *rec);
    +extern int filter_match_preds(struct filter_pred **preds, void *rec);
    +extern void filter_print_subsystem_preds(struct event_subsystem *system,
    + struct trace_seq *s);
    extern void filter_free_subsystem_preds(struct event_subsystem *system);
    extern int filter_add_subsystem_pred(struct event_subsystem *system,
    struct filter_pred *pred);
    diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
    index 576f4fa..ca1a2b0 100644
    --- a/kernel/trace/trace_events.c
    +++ b/kernel/trace/trace_events.c
    @@ -481,7 +481,7 @@ event_filter_read(struct file *filp, char __user *ubuf, size_t cnt,

    trace_seq_init(s);

    - filter_print_preds(call->preds, s);
    + filter_print_preds(call, s);
    r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->len);

    kfree(s);
    @@ -549,7 +549,7 @@ subsystem_filter_read(struct file *filp, char __user *ubuf, size_t cnt,

    trace_seq_init(s);

    - filter_print_preds(system->preds, s);
    + filter_print_subsystem_preds(system, s);
    r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->len);

    kfree(s);
    diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
    index f16a921..8e32dd8 100644
    --- a/kernel/trace/trace_events_filter.c
    +++ b/kernel/trace/trace_events_filter.c
    @@ -22,10 +22,14 @@
    #include <linux/uaccess.h>
    #include <linux/module.h>
    #include <linux/ctype.h>
    +#include <linux/mutex.h>
    +#include <linux/rcupdate.h>

    #include "trace.h"
    #include "trace_output.h"

    +static DEFINE_MUTEX(filter_mutex);
    +
    static int filter_pred_64(struct filter_pred *pred, void *event)
    {
    u64 *addr = (u64 *)(event + pred->offset);
    @@ -82,15 +86,19 @@ static int filter_pred_string(struct filter_pred *pred, void *event)
    return match;
    }

    -/* return 1 if event matches, 0 otherwise (discard) */
    -int filter_match_preds(struct ftrace_event_call *call, void *rec)
    +/*
    + * return 1 if event matches, 0 otherwise (discard)
    + *
    + * should be protected by rcu_read_lock()
    + * */
    +int filter_match_preds(struct filter_pred **preds, void *rec)
    {
    int i, matched, and_failed = 0;
    struct filter_pred *pred;

    for (i = 0; i < MAX_FILTER_PRED; i++) {
    - if (call->preds[i]) {
    - pred = call->preds[i];
    + pred = rcu_dereference(preds[i]);
    + if (pred) {
    if (and_failed && !pred->or)
    continue;
    matched = pred->fn(pred, rec);
    @@ -109,7 +117,8 @@ int filter_match_preds(struct ftrace_event_call *call, void *rec)
    return 1;
    }

    -void filter_print_preds(struct filter_pred **preds, struct trace_seq *s)
    +static void __filter_print_preds(struct filter_pred **preds,
    + struct trace_seq *s)
    {
    char *field_name;
    struct filter_pred *pred;
    @@ -137,6 +146,21 @@ void filter_print_preds(struct filter_pred **preds, struct trace_seq *s)
    }
    }

    +void filter_print_preds(struct ftrace_event_call *call, struct trace_seq *s)
    +{
    + mutex_lock(&filter_mutex);
    + __filter_print_preds(call->preds, s);
    + mutex_unlock(&filter_mutex);
    +}
    +
    +void filter_print_subsystem_preds(struct event_subsystem *system,
    + struct trace_seq *s)
    +{
    + mutex_lock(&filter_mutex);
    + __filter_print_preds(system->preds, s);
    + mutex_unlock(&filter_mutex);
    +}
    +
    static struct ftrace_event_field *
    find_event_field(struct ftrace_event_call *call, char *name)
    {
    @@ -160,29 +184,37 @@ void filter_free_pred(struct filter_pred *pred)
    kfree(pred);
    }

    -void filter_free_preds(struct ftrace_event_call *call)
    +static void __filter_free_preds(struct filter_pred **preds)
    {
    int i;

    - if (call->preds) {
    - for (i = 0; i < MAX_FILTER_PRED; i++)
    - filter_free_pred(call->preds[i]);
    - kfree(call->preds);
    - call->preds = NULL;
    - }
    + if (!preds)
    + return;
    +
    + for (i = 0; i < MAX_FILTER_PRED; i++)
    + filter_free_pred(preds[i]);
    + kfree(preds);
    +}
    +
    +void filter_free_preds(struct ftrace_event_call *call)
    +{
    + struct filter_pred **preds = call->preds;
    +
    + mutex_lock(&filter_mutex);
    + rcu_assign_pointer(call->preds, NULL);
    + mutex_unlock(&filter_mutex);
    +
    + synchronize_rcu();
    + __filter_free_preds(preds);
    }

    void filter_free_subsystem_preds(struct event_subsystem *system)
    {
    struct ftrace_event_call *call = __start_ftrace_events;
    - int i;

    - if (system->preds) {
    - for (i = 0; i < MAX_FILTER_PRED; i++)
    - filter_free_pred(system->preds[i]);
    - kfree(system->preds);
    - system->preds = NULL;
    - }
    + mutex_lock(&filter_mutex);
    + __filter_free_preds(system->preds);
    + mutex_unlock(&filter_mutex);

    events_for_each(call) {
    if (!call->name || !call->regfunc)
    @@ -197,25 +229,35 @@ static int __filter_add_pred(struct ftrace_event_call *call,
    struct filter_pred *pred)
    {
    int i;
    + int ret = 0;
    + struct filter_pred **preds;

    - if (call->preds && !pred->compound)
    + if (!pred->compound)
    filter_free_preds(call);

    + mutex_lock(&filter_mutex);
    +
    if (!call->preds) {
    - call->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred),
    - GFP_KERNEL);
    - if (!call->preds)
    - return -ENOMEM;
    + preds = kzalloc(MAX_FILTER_PRED * sizeof(pred),
    + GFP_KERNEL);
    + if (!preds) {
    + ret = -ENOMEM;
    + goto unlock;
    + }
    + rcu_assign_pointer(call->preds, preds);
    }

    for (i = 0; i < MAX_FILTER_PRED; i++) {
    if (!call->preds[i]) {
    - call->preds[i] = pred;
    - return 0;
    + rcu_assign_pointer(call->preds[i], pred);
    + goto unlock;
    }
    }
    + ret = -ENOSPC;

    - return -ENOSPC;
    +unlock:
    + mutex_unlock(&filter_mutex);
    + return ret;
    }

    static int is_string_field(const char *type)
    @@ -304,11 +346,15 @@ int filter_add_subsystem_pred(struct event_subsystem *system,
    if (system->preds && !pred->compound)
    filter_free_subsystem_preds(system);

    + mutex_lock(&filter_mutex);
    +
    if (!system->preds) {
    system->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred),
    GFP_KERNEL);
    - if (!system->preds)
    + if (!system->preds) {
    + mutex_unlock(&filter_mutex);
    return -ENOMEM;
    + }
    }

    for (i = 0; i < MAX_FILTER_PRED; i++) {
    @@ -318,6 +364,8 @@ int filter_add_subsystem_pred(struct event_subsystem *system,
    }
    }

    + mutex_unlock(&filter_mutex);
    +
    if (i == MAX_FILTER_PRED)
    return -ENOSPC;

    diff --git a/kernel/trace/trace_events_stage_3.h b/kernel/trace/trace_events_stage_3.h
    index 9d2fa78..973941d 100644
    --- a/kernel/trace/trace_events_stage_3.h
    +++ b/kernel/trace/trace_events_stage_3.h
    @@ -207,8 +207,10 @@ static void ftrace_raw_event_##call(proto) \
    struct ftrace_event_call *call = &event_##call; \
    struct ring_buffer_event *event; \
    struct ftrace_raw_##call *entry; \
    + struct filter_pred **preds; \
    unsigned long irq_flags; \
    int pc; \
    + int matched = 1; \
    \
    local_save_flags(irq_flags); \
    pc = preempt_count(); \
    @@ -222,7 +224,13 @@ static void ftrace_raw_event_##call(proto) \
    \
    assign; \
    \
    - if (call->preds && !filter_match_preds(call, entry)) \
    + rcu_read_lock(); \
    + preds = rcu_dereference(call->preds); \
    + if (preds) \
    + matched = filter_match_preds(preds, entry); \
    + rcu_read_unlock(); \
    + \
    + if (!matched) \
    ring_buffer_event_discard(event); \
    \
    trace_nowake_buffer_unlock_commit(event, irq_flags, pc); \
    --
    1.5.4.rc3


    \
     
     \ /
      Last update: 2009-04-11 09:59    [W:0.034 / U:121.004 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site