(2013/07/04 12:33), Steven Rostedt wrote:
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 7d85429..90cf243 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -391,6 +391,28 @@ static void __get_system_dir(struct ftrace_subsystem_dir
> *dir)
> __get_system(dir->subsystem);
> }
>
> +static int ftrace_event_call_get(struct ftrace_event_call *call)
> +{
> + int ret = 0;
> +
> + mutex_lock(&event_mutex);
> + if ((call->flags & TRACE_EVENT_FL_REF_MASK) == TRACE_EVENT_FL_REF_MAX -
> 1)
> + ret = -EBUSY;
> + else
> + call->flags++;
> + mutex_unlock(&event_mutex);
> +
> + return ret;
> +}
> +
> +static void ftrace_event_call_put(struct ftrace_event_call *call)
> +{
> + mutex_lock(&event_mutex);
> + if (!WARN_ON_ONCE(!(call->flags & TRACE_EVENT_FL_REF_MASK)))
> + call->flags--;
> + mutex_unlock(&event_mutex);
> +}
Hmm, I might misunderstand, but it seems that there is a small unsafe
time slot.
> @@ -424,7 +446,15 @@ static int tracing_open_generic_file(struct inode
> *inode, struct file *filp)
>
> ret = tracing_open_generic(inode, filp);
> if (ret < 0)
> - trace_array_put(tr);
> + goto fail;
> +
> + ret = ftrace_event_call_get(file->event_call);
> + if (ret < 0)
> + goto fail;
> +
> + return 0;
> + fail:
> + trace_array_put(tr);
> return ret;
> }
>
> @@ -433,12 +463,40 @@ static int tracing_release_generic_file(struct inode
> *inode, struct file *filp)
> struct ftrace_event_file *file = inode->i_private;
> struct trace_array *tr = file->tr;
>
> + ftrace_event_call_put(file->event_call);
> trace_array_put(tr);
>
> return 0;
> }
Here, at first we get an event_file from inode->i_private, and then locks
event_mutex and get/put refcount. This should be safe if we get (find) the
object
from some list of event_file under the mutex, but we just use inode->i_private.
This can cause a race as below scenario,
CPU0 CPU1
open(kprobe_events)
trace_remove_event_call() open(enable)
lock event_mutex refer event_file
event_remove() wait for unlock event_mutex
...
free event_file
unlock event_mutex
lock event_mutex
add refcount of event_file->call (*)
So, at (*) point, the event_file is already freed. Thus there still be
an unsafe time slot. Or, did I miss something (possibly..)?
Thank you,
--
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/
