All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	"zhangwei(Jovi)" <jovi.zhangwei@huawei.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Ingo Molnar <mingo@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [RFC][PATCH 3/4] tracing/kprobes: Fail to unregister if probe event files are open
Date: Wed, 31 Jul 2013 22:40:03 +0200	[thread overview]
Message-ID: <20130731204003.GA30188@redhat.com> (raw)
In-Reply-To: <1375300192.5418.17.camel@gandalf.local.home>

On 07/31, Steven Rostedt wrote:
>
> On Wed, 2013-07-03 at 23:33 -0400, Steven Rostedt wrote:
> > The above will corrupt the kprobe system, as the write to the enable
> > file will happen after the kprobe was deleted.
>
> Oleg,
>
> The above no longer triggers the bug due to your changes. The race is
> much tighter now

Yes, the changelog should be updated...

> and requires a process with the enable file opened and
> races with a write to enable it where the removal of the trace file
> checks the trace disabled, sees that it is, continues, but then the
> write enables it just as it gets deleted.

This should be fine. Either event_remove() path takes event_mutex
first and then ->write() fails, or ftrace_event_enable_disable()
actually disables this even successfully.

> Do you know of a way to trigger this bug?

I'll try to think more tomorrow, but most probably no. The race is
unlikely.

We need perf_trace_event_init() or ":enable_event:this-event" right
before trace_remove_event_call() takes the mutex.

And right after the caller (kprobes) checks "disabled".

> Hmm, what happens without this patch now? If it is active, and we delete
> it? It will call back into the kprobes and access a tracepoint that does
> not exist? Would this cause a crash?

I think yes, the crash is possible.

perf or FL_SOFT_MODE, this call/file has the external references, and
we are going to free it.

Oleg.


  reply	other threads:[~2013-07-31 20:45 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-04  3:33 [RFC][PATCH 0/4] tracing/kprobes/uprobes: Fix race between opening probe event files and deleting probe Steven Rostedt
2013-07-04  3:33 ` [RFC][PATCH 1/4] tracing: Add ref count to ftrace_event_call Steven Rostedt
2013-07-04  4:22   ` Masami Hiramatsu
2013-07-04 11:55     ` [RFC PATCH] tracing: Atomically get refcounts of event_call and trace_array Masami Hiramatsu
2013-07-04 12:12       ` Masami Hiramatsu
2013-07-04 12:23         ` Steven Rostedt
2013-07-05  0:32       ` Oleg Nesterov
2013-07-05  2:32         ` Masami Hiramatsu
2013-07-09  7:55         ` [RFC PATCH V2] tracing: Check f_dentry before accessing event_file/call in inode->i_private Masami Hiramatsu
2013-07-15 18:16           ` Oleg Nesterov
2013-07-17  2:10             ` Masami Hiramatsu
2013-07-17 14:51               ` Oleg Nesterov
2013-07-18  2:20                 ` Masami Hiramatsu
2013-07-18 14:51                   ` Oleg Nesterov
2013-07-19  5:21                     ` Masami Hiramatsu
2013-07-19 13:33                       ` Oleg Nesterov
2013-07-22  9:57                         ` Masami Hiramatsu
2013-07-22 17:04                           ` Oleg Nesterov
2013-07-23 21:04                             ` Oleg Nesterov
2013-07-04  3:33 ` [RFC][PATCH 2/4] tracing: trace_remove_event_call() should fail if call/file is in use Steven Rostedt
2013-07-04 12:48   ` Masami Hiramatsu
2013-07-04  3:33 ` [RFC][PATCH 3/4] tracing/kprobes: Fail to unregister if probe event files are open Steven Rostedt
2013-07-04 12:45   ` Masami Hiramatsu
2013-07-04 18:48     ` Oleg Nesterov
2013-07-05  2:53       ` Masami Hiramatsu
2013-07-05 17:26         ` Oleg Nesterov
2013-07-08  2:36           ` Masami Hiramatsu
2013-07-08 14:25             ` Oleg Nesterov
2013-07-09  8:01               ` [RFC PATCH] tracing/kprobe: Wait for disabling all running kprobe handlers Masami Hiramatsu
2013-07-09  8:07                 ` Peter Zijlstra
2013-07-09  8:20                   ` Masami Hiramatsu
2013-07-09  8:21                     ` Peter Zijlstra
2013-07-09  8:50                       ` Masami Hiramatsu
2013-07-09  9:35                       ` [RFC PATCH V2] " Masami Hiramatsu
2013-07-15 18:20                         ` Oleg Nesterov
2013-07-18 12:07                           ` Masami Hiramatsu
2013-07-18 14:35                             ` Steven Rostedt
2013-07-30  8:15   ` [RFC][PATCH 3/4] tracing/kprobes: Fail to unregister if probe event files are open Masami Hiramatsu
2013-07-31 19:49   ` Steven Rostedt
2013-07-31 20:40     ` Oleg Nesterov [this message]
2013-07-31 22:42       ` Steven Rostedt
2013-08-01  2:07         ` Steven Rostedt
2013-08-01  2:50           ` Steven Rostedt
2013-08-01  3:48             ` Masami Hiramatsu
2013-08-01 13:34             ` Oleg Nesterov
2013-08-01 13:49               ` Oleg Nesterov
2013-08-01 14:17               ` Steven Rostedt
2013-08-01 14:33                 ` Oleg Nesterov
2013-08-01 14:45                   ` Steven Rostedt
2013-08-01 14:46                     ` Oleg Nesterov
2013-08-02  4:57               ` Masami Hiramatsu
2013-08-01 13:10         ` Oleg Nesterov
2013-07-04  3:33 ` [RFC][PATCH 4/4] tracing/uprobes: " Steven Rostedt
2013-08-01  3:40   ` Steven Rostedt
2013-08-01 14:08   ` Oleg Nesterov
2013-08-01 14:25     ` Steven Rostedt
2013-07-04  4:00 ` [RFC][PATCH 0/4] tracing/kprobes/uprobes: Fix race between opening probe event files and deleting probe Masami Hiramatsu
2013-07-04  6:14   ` Masami Hiramatsu
2013-07-12 13:09 ` Masami Hiramatsu
2013-07-12 17:53   ` Oleg Nesterov
2013-07-15 18:01 ` Oleg Nesterov
2013-07-16 16:38   ` Oleg Nesterov
2013-07-16 19:10     ` Steven Rostedt
2013-07-16 19:22       ` Oleg Nesterov
2013-07-16 19:38         ` Steven Rostedt
2013-07-17 16:03           ` Steven Rostedt
2013-07-17 17:37             ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130731204003.GA30188@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=akpm@linux-foundation.org \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=jovi.zhangwei@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=srikar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.