Messages in this thread | | | Date | Mon, 28 Jan 2013 17:39:26 +0530 | From | Srikar Dronamraju <> | Subject | Re: [PATCH 2/4] uprobes: Fully initialize uprobe_trace_consumer before uprobe_register() |
| |
* Oleg Nesterov <oleg@redhat.com> [2013-01-27 20:48:37]:
> probe_event_enable() does uprobe_register() and only after that sets > utc->tu and tu->consumer/flags. This can race with uprobe_dispatcher() > which can miss these assignments or see them out of order. Nothing > really bad can happen, but this doesn't look clean/safe. > > And this does not allow to use uprobe_consumer->filter() we are going > to add, it is called by uprobe_register() and it needs utc->tu. > > Change this code to initialize everything before uprobe_register(), and > reset tu->consumer/flags if it fails. We can't race with event_disable(), > the caller holds event_mutex, and if we could the code would be wrong > anyway. > > In fact I think uprobe_trace_consumer should die, it buys nothing but > complicate the code. We can simply add uprobe_consumer into trace_uprobe. > > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> --- > kernel/trace/trace_uprobe.c | 13 +++++++------ > 1 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index 53afabe..94d4ea2 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -552,17 +552,18 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag) > return -EINTR; > > utc->cons.handler = uprobe_dispatcher; > + utc->tu = tu; > + tu->consumer = utc; > + tu->flags |= flag; > + > ret = uprobe_register(tu->inode, tu->offset, &utc->cons); > if (ret) { > + tu->consumer = NULL; > + tu->flags &= ~flag; > kfree(utc); > - return ret; > } > > - tu->flags |= flag; > - utc->tu = tu; > - tu->consumer = utc; > - > - return 0; > + return ret; > } > > static void probe_event_disable(struct trace_uprobe *tu, int flag) > -- > 1.5.5.1 >
-- Thanks and Regards Srikar Dronamraju
| |