Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-25 Thread Steven Rostedt
On Wed, 25 Jul 2018 11:01:22 -0500 Tom Zanussi wrote: > > First we have this: > > > > ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file); > > /* > > * The above returns on success the # of functions enabled, > > * but if it didn't find any functions it returns zero. > >

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-25 Thread Tom Zanussi
Hi Steve, On 7/24/2018 4:30 PM, Steven Rostedt wrote: On Tue, 24 Jul 2018 16:49:59 -0400 Steven Rostedt wrote: Hmm it seems we should review the register_trigger() implementation. It should return the return value of trace_event_trigger_enable_disable(), shouldn't it? Yeah, that's not do

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-25 Thread Masami Hiramatsu
On Tue, 24 Jul 2018 22:41:49 -0400 Steven Rostedt wrote: > On Wed, 25 Jul 2018 10:16:53 +0900 > Masami Hiramatsu wrote: > > > Hm, as far as I can see, when register_trigger() returns >= 0, it already > > calls ->init the trigger_data. This means its refcount++, and in that > > case, below patch

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-24 Thread Steven Rostedt
On Wed, 25 Jul 2018 10:16:53 +0900 Masami Hiramatsu wrote: > Hm, as far as I can see, when register_trigger() returns >= 0, it already > calls ->init the trigger_data. This means its refcount++, and in that > case, below patch will miss to free the trigger_data. > How about below for tentative fi

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-24 Thread Masami Hiramatsu
On Tue, 24 Jul 2018 19:13:31 -0400 Steven Rostedt wrote: > On Tue, 24 Jul 2018 17:30:08 -0400 > Steven Rostedt wrote: > > > I don't see where ->reg() would return anything but 1 on success. Maybe > > I'm missing something. I'll look some more, but I'm thinking of changing > > ->reg() to return

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-24 Thread Masami Hiramatsu
On Tue, 24 Jul 2018 16:49:59 -0400 Steven Rostedt wrote: > On Wed, 25 Jul 2018 00:09:09 +0900 > Masami Hiramatsu wrote: > > > Hmm, your patch seems to leak a memory since event_trigger_init() will > > be called twice on same trigger_data (Note that event_trigger_init() > > does not init ref cou

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-24 Thread Steven Rostedt
On Tue, 24 Jul 2018 17:30:08 -0400 Steven Rostedt wrote: > I don't see where ->reg() would return anything but 1 on success. Maybe > I'm missing something. I'll look some more, but I'm thinking of changing > ->reg() to return zero on all success, and negative on all errors and > just check thos

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-24 Thread Steven Rostedt
On Tue, 24 Jul 2018 16:49:59 -0400 Steven Rostedt wrote: > > Hmm it seems we should review the register_trigger() implementation. > > It should return the return value of trace_event_trigger_enable_disable(), > > shouldn't it? > > > > Yeah, that's not done well. I'll fix it up. > > Thanks for

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-24 Thread Steven Rostedt
On Wed, 25 Jul 2018 00:09:09 +0900 Masami Hiramatsu wrote: > Hmm, your patch seems to leak a memory since event_trigger_init() will > be called twice on same trigger_data (Note that event_trigger_init() > does not init ref counter, but increment it.) So we should decrement > it when we find it is

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-24 Thread Masami Hiramatsu
On Mon, 23 Jul 2018 22:10:06 -0400 Steven Rostedt wrote: > On Sat, 14 Jul 2018 01:27:47 +0900 > Masami Hiramatsu wrote: > > > Fix a double free bug of event_trigger_data caused by > > calling unregister_trigger() from register_snapshot_trigger(). > > This kicks a kernel BUG if double free check

Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-23 Thread Steven Rostedt
On Sat, 14 Jul 2018 01:27:47 +0900 Masami Hiramatsu wrote: > Fix a double free bug of event_trigger_data caused by > calling unregister_trigger() from register_snapshot_trigger(). > This kicks a kernel BUG if double free checker is enabled > as below; > > kernel BUG at /home/mhiramat/ksrc/linux

[PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger_data

2018-07-13 Thread Masami Hiramatsu
Fix a double free bug of event_trigger_data caused by calling unregister_trigger() from register_snapshot_trigger(). This kicks a kernel BUG if double free checker is enabled as below; kernel BUG at /home/mhiramat/ksrc/linux/mm/slub.c:296! invalid opcode: [#1] SMP PTI CPU: 2 PID: 4312 Comm