* [PATCH] tracing: Fix double free of event_trigger_data
@ 2018-07-25 12:57 Steven Rostedt
2018-07-25 15:43 ` Masami Hiramatsu
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-07-25 12:57 UTC (permalink / raw)
To: LKML
Cc: Masami Hiramatsu, Tom Zanussi, Hiraku Toyooka, Ingo Molnar,
Andrew Morton
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Running the following:
# cd /sys/kernel/debug/tracing
# echo 500000 > buffer_size_kb
[ Or some other number that takes up most of memory ]
# echo snapshot > events/sched/sched_switch/trigger
Triggers the following bug:
------------[ cut here ]------------
kernel BUG at mm/slub.c:296!
invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC PTI
CPU: 6 PID: 6878 Comm: bash Not tainted 4.18.0-rc6-test+ #1066
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
RIP: 0010:kfree+0x16c/0x180
Code: 05 41 0f b6 72 51 5b 5d 41 5c 4c 89 d7 e9 ac b3 f8 ff 48 89 d9 48 89 da 41 b8 01 00 00 00 5b 5d 41 5c 4c 89 d6 e9 f4 f3 ff ff <0f> 0b 0f 0b 48 8b 3d d9 d8 f9 00 e9 c1 fe ff ff 0f 1f 40 00 0f 1f
RSP: 0018:ffffb654436d3d88 EFLAGS: 00010246
RAX: ffff91a9d50f3d80 RBX: ffff91a9d50f3d80 RCX: ffff91a9d50f3d80
RDX: 00000000000006a4 RSI: ffff91a9de5a60e0 RDI: ffff91a9d9803500
RBP: ffffffff8d267c80 R08: 00000000000260e0 R09: ffffffff8c1a56be
R10: fffff0d404543cc0 R11: 0000000000000389 R12: ffffffff8c1a56be
R13: ffff91a9d9930e18 R14: ffff91a98c0c2890 R15: ffffffff8d267d00
FS: 00007f363ea64700(0000) GS:ffff91a9de580000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055c1cacc8e10 CR3: 00000000d9b46003 CR4: 00000000001606e0
Call Trace:
event_trigger_callback+0xee/0x1d0
event_trigger_write+0xfc/0x1a0
__vfs_write+0x33/0x190
? handle_mm_fault+0x115/0x230
? _cond_resched+0x16/0x40
vfs_write+0xb0/0x190
ksys_write+0x52/0xc0
do_syscall_64+0x5a/0x160
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f363e16ab50
Code: 73 01 c3 48 8b 0d 38 83 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d 79 db 2c 00 00 75 10 b8 01 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 1e e3 01 00 48 89 04 24
RSP: 002b:00007fff9a4c6378 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000009 RCX: 00007f363e16ab50
RDX: 0000000000000009 RSI: 000055c1cacc8e10 RDI: 0000000000000001
RBP: 000055c1cacc8e10 R08: 00007f363e435740 R09: 00007f363ea64700
R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000009
R13: 0000000000000001 R14: 00007f363e4345e0 R15: 00007f363e4303c0
Modules linked in: ip6table_filter ip6_tables snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device i915 snd_pcm snd_timer i2c_i801 snd soundcore i2c_algo_bit drm_kms_helper
86_pkg_temp_thermal video kvm_intel kvm irqbypass wmi e1000e
---[ end trace d301afa879ddfa25 ]---
The cause is because the register_snapshot_trigger() call failed to
allocate the snapshot buffer, and then called unregister_trigger()
which freed the data that was passed to it. Then on return to the
function that called register_snapshot_trigger(), as it sees it
failed to register, it frees the trigger_data again and causes
a double free.
By calling event_trigger_init() on the trigger_data (which only ups
the reference counter for it), and then event_trigger_free() afterward,
the trigger_data would not get freed by the registering trigger function
as it would only up and lower the ref count for it. If the register
trigger function fails, then the event_trigger_free() called after it
will free the trigger data normally.
Link: http://lkml.kernel.org/r/20180724191331.738eb819@gandalf.local.home
Cc: stable@vger.kerne.org
Fixes: 93e31ffbf417 ("tracing: Add 'snapshot' event trigger command")
Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_trigger.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index d18249683682..d18ec0e58be2 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -679,6 +679,8 @@ event_trigger_callback(struct event_command *cmd_ops,
goto out_free;
out_reg:
+ /* Up the trigger_data count to make sure reg doesn't free it on failure */
+ event_trigger_init(trigger_ops, trigger_data);
ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
/*
* The above returns on success the # of functions enabled,
@@ -686,11 +688,13 @@ event_trigger_callback(struct event_command *cmd_ops,
* Consider no functions a failure too.
*/
if (!ret) {
+ cmd_ops->unreg(glob, trigger_ops, trigger_data, file);
ret = -ENOENT;
- goto out_free;
- } else if (ret < 0)
- goto out_free;
- ret = 0;
+ } else if (ret > 0)
+ ret = 0;
+
+ /* Down the counter of trigger_data or free it if not used anymore */
+ event_trigger_free(trigger_ops, trigger_data);
out:
return ret;
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tracing: Fix double free of event_trigger_data
2018-07-25 12:57 [PATCH] tracing: Fix double free of event_trigger_data Steven Rostedt
@ 2018-07-25 15:43 ` Masami Hiramatsu
2018-07-25 16:40 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2018-07-25 15:43 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Masami Hiramatsu, Tom Zanussi, Hiraku Toyooka, Ingo Molnar,
Andrew Morton
On Wed, 25 Jul 2018 08:57:40 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> Running the following:
>
> # cd /sys/kernel/debug/tracing
> # echo 500000 > buffer_size_kb
> [ Or some other number that takes up most of memory ]
> # echo snapshot > events/sched/sched_switch/trigger
>
> Triggers the following bug:
>
> ------------[ cut here ]------------
> kernel BUG at mm/slub.c:296!
> invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC PTI
> CPU: 6 PID: 6878 Comm: bash Not tainted 4.18.0-rc6-test+ #1066
> Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
> RIP: 0010:kfree+0x16c/0x180
> Code: 05 41 0f b6 72 51 5b 5d 41 5c 4c 89 d7 e9 ac b3 f8 ff 48 89 d9 48 89 da 41 b8 01 00 00 00 5b 5d 41 5c 4c 89 d6 e9 f4 f3 ff ff <0f> 0b 0f 0b 48 8b 3d d9 d8 f9 00 e9 c1 fe ff ff 0f 1f 40 00 0f 1f
> RSP: 0018:ffffb654436d3d88 EFLAGS: 00010246
> RAX: ffff91a9d50f3d80 RBX: ffff91a9d50f3d80 RCX: ffff91a9d50f3d80
> RDX: 00000000000006a4 RSI: ffff91a9de5a60e0 RDI: ffff91a9d9803500
> RBP: ffffffff8d267c80 R08: 00000000000260e0 R09: ffffffff8c1a56be
> R10: fffff0d404543cc0 R11: 0000000000000389 R12: ffffffff8c1a56be
> R13: ffff91a9d9930e18 R14: ffff91a98c0c2890 R15: ffffffff8d267d00
> FS: 00007f363ea64700(0000) GS:ffff91a9de580000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000055c1cacc8e10 CR3: 00000000d9b46003 CR4: 00000000001606e0
> Call Trace:
> event_trigger_callback+0xee/0x1d0
> event_trigger_write+0xfc/0x1a0
> __vfs_write+0x33/0x190
> ? handle_mm_fault+0x115/0x230
> ? _cond_resched+0x16/0x40
> vfs_write+0xb0/0x190
> ksys_write+0x52/0xc0
> do_syscall_64+0x5a/0x160
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x7f363e16ab50
> Code: 73 01 c3 48 8b 0d 38 83 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d 79 db 2c 00 00 75 10 b8 01 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 1e e3 01 00 48 89 04 24
> RSP: 002b:00007fff9a4c6378 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000009 RCX: 00007f363e16ab50
> RDX: 0000000000000009 RSI: 000055c1cacc8e10 RDI: 0000000000000001
> RBP: 000055c1cacc8e10 R08: 00007f363e435740 R09: 00007f363ea64700
> R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000009
> R13: 0000000000000001 R14: 00007f363e4345e0 R15: 00007f363e4303c0
> Modules linked in: ip6table_filter ip6_tables snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device i915 snd_pcm snd_timer i2c_i801 snd soundcore i2c_algo_bit drm_kms_helper
> 86_pkg_temp_thermal video kvm_intel kvm irqbypass wmi e1000e
> ---[ end trace d301afa879ddfa25 ]---
>
> The cause is because the register_snapshot_trigger() call failed to
> allocate the snapshot buffer, and then called unregister_trigger()
> which freed the data that was passed to it. Then on return to the
> function that called register_snapshot_trigger(), as it sees it
> failed to register, it frees the trigger_data again and causes
> a double free.
>
> By calling event_trigger_init() on the trigger_data (which only ups
> the reference counter for it), and then event_trigger_free() afterward,
> the trigger_data would not get freed by the registering trigger function
> as it would only up and lower the ref count for it. If the register
> trigger function fails, then the event_trigger_free() called after it
> will free the trigger data normally.
>
> Link: http://lkml.kernel.org/r/20180724191331.738eb819@gandalf.local.home
>
Looks good to me.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks!
> Cc: stable@vger.kerne.org
> Fixes: 93e31ffbf417 ("tracing: Add 'snapshot' event trigger command")
> Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> kernel/trace/trace_events_trigger.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index d18249683682..d18ec0e58be2 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -679,6 +679,8 @@ event_trigger_callback(struct event_command *cmd_ops,
> goto out_free;
>
> out_reg:
> + /* Up the trigger_data count to make sure reg doesn't free it on failure */
> + event_trigger_init(trigger_ops, trigger_data);
> ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
> /*
> * The above returns on success the # of functions enabled,
> @@ -686,11 +688,13 @@ event_trigger_callback(struct event_command *cmd_ops,
> * Consider no functions a failure too.
> */
> if (!ret) {
> + cmd_ops->unreg(glob, trigger_ops, trigger_data, file);
> ret = -ENOENT;
> - goto out_free;
> - } else if (ret < 0)
> - goto out_free;
> - ret = 0;
> + } else if (ret > 0)
> + ret = 0;
> +
> + /* Down the counter of trigger_data or free it if not used anymore */
> + event_trigger_free(trigger_ops, trigger_data);
> out:
> return ret;
>
> --
> 2.13.6
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracing: Fix double free of event_trigger_data
2018-07-25 15:43 ` Masami Hiramatsu
@ 2018-07-25 16:40 ` Steven Rostedt
2018-07-25 19:29 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-07-25 16:40 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: LKML, Tom Zanussi, Hiraku Toyooka, Ingo Molnar, Andrew Morton
On Thu, 26 Jul 2018 00:43:49 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> On Wed, 25 Jul 2018 08:57:40 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > Running the following:
> >
> > # cd /sys/kernel/debug/tracing
> > # echo 500000 > buffer_size_kb
> > [ Or some other number that takes up most of memory ]
> > # echo snapshot > events/sched/sched_switch/trigger
> >
> > Triggers the following bug:
> >
> > ------------[ cut here ]------------
> > kernel BUG at mm/slub.c:296!
> > invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC PTI
> > CPU: 6 PID: 6878 Comm: bash Not tainted 4.18.0-rc6-test+ #1066
> > Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
> > RIP: 0010:kfree+0x16c/0x180
> > Code: 05 41 0f b6 72 51 5b 5d 41 5c 4c 89 d7 e9 ac b3 f8 ff 48 89 d9 48 89 da 41 b8 01 00 00 00 5b 5d 41 5c 4c 89 d6 e9 f4 f3 ff ff <0f> 0b 0f 0b 48 8b 3d d9 d8 f9 00 e9 c1 fe ff ff 0f 1f 40 00 0f 1f
> > RSP: 0018:ffffb654436d3d88 EFLAGS: 00010246
> > RAX: ffff91a9d50f3d80 RBX: ffff91a9d50f3d80 RCX: ffff91a9d50f3d80
> > RDX: 00000000000006a4 RSI: ffff91a9de5a60e0 RDI: ffff91a9d9803500
> > RBP: ffffffff8d267c80 R08: 00000000000260e0 R09: ffffffff8c1a56be
> > R10: fffff0d404543cc0 R11: 0000000000000389 R12: ffffffff8c1a56be
> > R13: ffff91a9d9930e18 R14: ffff91a98c0c2890 R15: ffffffff8d267d00
> > FS: 00007f363ea64700(0000) GS:ffff91a9de580000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 000055c1cacc8e10 CR3: 00000000d9b46003 CR4: 00000000001606e0
> > Call Trace:
> > event_trigger_callback+0xee/0x1d0
> > event_trigger_write+0xfc/0x1a0
> > __vfs_write+0x33/0x190
> > ? handle_mm_fault+0x115/0x230
> > ? _cond_resched+0x16/0x40
> > vfs_write+0xb0/0x190
> > ksys_write+0x52/0xc0
> > do_syscall_64+0x5a/0x160
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x7f363e16ab50
> > Code: 73 01 c3 48 8b 0d 38 83 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d 79 db 2c 00 00 75 10 b8 01 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 1e e3 01 00 48 89 04 24
> > RSP: 002b:00007fff9a4c6378 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> > RAX: ffffffffffffffda RBX: 0000000000000009 RCX: 00007f363e16ab50
> > RDX: 0000000000000009 RSI: 000055c1cacc8e10 RDI: 0000000000000001
> > RBP: 000055c1cacc8e10 R08: 00007f363e435740 R09: 00007f363ea64700
> > R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000009
> > R13: 0000000000000001 R14: 00007f363e4345e0 R15: 00007f363e4303c0
> > Modules linked in: ip6table_filter ip6_tables snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_seq snd_seq_device i915 snd_pcm snd_timer i2c_i801 snd soundcore i2c_algo_bit drm_kms_helper
> > 86_pkg_temp_thermal video kvm_intel kvm irqbypass wmi e1000e
> > ---[ end trace d301afa879ddfa25 ]---
> >
> > The cause is because the register_snapshot_trigger() call failed to
> > allocate the snapshot buffer, and then called unregister_trigger()
> > which freed the data that was passed to it. Then on return to the
> > function that called register_snapshot_trigger(), as it sees it
> > failed to register, it frees the trigger_data again and causes
> > a double free.
> >
> > By calling event_trigger_init() on the trigger_data (which only ups
> > the reference counter for it), and then event_trigger_free() afterward,
> > the trigger_data would not get freed by the registering trigger function
> > as it would only up and lower the ref count for it. If the register
> > trigger function fails, then the event_trigger_free() called after it
> > will free the trigger data normally.
> >
> > Link: http://lkml.kernel.org/r/20180724191331.738eb819@gandalf.local.home
> >
>
> Looks good to me.
>
> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
>
Hmm, looks to me that event_enable_trigger_func() suffers the same
issue. Perhaps we should add this patch too:
-- Steve
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index d18ec0e58be2..2681d917f896 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1457,6 +1457,10 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
ret = trace_event_enable_disable(event_enable_file, 1, 1);
if (ret < 0)
goto out_put;
+
+ /* Up the trigger_data count to make sure reg doesn't free it on failure */
+ event_trigger_init(trigger_ops, trigger_data);
+
ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
/*
* The above returns on success the # of functions enabled,
@@ -1464,12 +1468,13 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
* Consider no functions a failure too.
*/
if (!ret) {
+ cmd_ops->unreg(glob, trigger_ops, trigger_data, file);
ret = -ENOENT;
- goto out_disable;
- } else if (ret < 0)
- goto out_disable;
- /* Just return zero, not the number of enabled functions */
- ret = 0;
+ } else if (ret > 0)
+ ret = 0;
+
+ /* Down the counter of trigger_data or free it if not used anymore */
+ event_trigger_free(trigger_ops, trigger_data);
out:
return ret;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tracing: Fix double free of event_trigger_data
2018-07-25 16:40 ` Steven Rostedt
@ 2018-07-25 19:29 ` Steven Rostedt
2018-07-25 19:53 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-07-25 19:29 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: LKML, Tom Zanussi, Hiraku Toyooka, Ingo Molnar, Andrew Morton
On Wed, 25 Jul 2018 12:40:08 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> Hmm, looks to me that event_enable_trigger_func() suffers the same
> issue. Perhaps we should add this patch too:
>
> -- Steve
>
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index d18ec0e58be2..2681d917f896 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -1457,6 +1457,10 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
> ret = trace_event_enable_disable(event_enable_file, 1, 1);
> if (ret < 0)
> goto out_put;
> +
> + /* Up the trigger_data count to make sure reg doesn't free it on failure */
> + event_trigger_init(trigger_ops, trigger_data);
> +
> ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
> /*
> * The above returns on success the # of functions enabled,
> @@ -1464,12 +1468,13 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
> * Consider no functions a failure too.
> */
> if (!ret) {
> + cmd_ops->unreg(glob, trigger_ops, trigger_data, file);
> ret = -ENOENT;
> - goto out_disable;
> - } else if (ret < 0)
> - goto out_disable;
> - /* Just return zero, not the number of enabled functions */
> - ret = 0;
> + } else if (ret > 0)
> + ret = 0;
> +
> + /* Down the counter of trigger_data or free it if not used anymore */
> + event_trigger_free(trigger_ops, trigger_data);
Nope, this doesn't work. It's a little more complex than the other one.
I'll just leave it, and fix the reg code for 4.19.
-- Steve
> out:
> return ret;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracing: Fix double free of event_trigger_data
2018-07-25 19:29 ` Steven Rostedt
@ 2018-07-25 19:53 ` Steven Rostedt
2018-07-25 22:51 ` Masami Hiramatsu
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-07-25 19:53 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: LKML, Tom Zanussi, Hiraku Toyooka, Ingo Molnar, Andrew Morton
On Wed, 25 Jul 2018 15:29:04 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> Nope, this doesn't work. It's a little more complex than the other one.
> I'll just leave it, and fix the reg code for 4.19.
I take this back. Looks like it just needs to be handled slightly
differently. Thoughts?
-- Steve
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index d18ec0e58be2..5dea177cef53 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1420,6 +1420,9 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
goto out;
}
+ /* Up the trigger_data count to make sure nothing frees it on failure */
+ event_trigger_init(trigger_ops, trigger_data);
+
if (trigger) {
number = strsep(&trigger, ":");
@@ -1470,6 +1473,7 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
goto out_disable;
/* Just return zero, not the number of enabled functions */
ret = 0;
+ event_trigger_free(trigger_ops, trigger_data);
out:
return ret;
@@ -1480,7 +1484,7 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
out_free:
if (cmd_ops->set_filter)
cmd_ops->set_filter(NULL, trigger_data, NULL);
- kfree(trigger_data);
+ event_trigger_free(trigger_ops, trigger_data);
kfree(enable_data);
goto out;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tracing: Fix double free of event_trigger_data
2018-07-25 19:53 ` Steven Rostedt
@ 2018-07-25 22:51 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2018-07-25 22:51 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Tom Zanussi, Hiraku Toyooka, Ingo Molnar, Andrew Morton
On Wed, 25 Jul 2018 15:53:21 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 25 Jul 2018 15:29:04 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > Nope, this doesn't work. It's a little more complex than the other one.
> > I'll just leave it, and fix the reg code for 4.19.
>
> I take this back. Looks like it just needs to be handled slightly
> differently. Thoughts?
Ah, I see, the filter should be cleared :)
OK, this should work.
Reivewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks,
>
> -- Steve
>
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index d18ec0e58be2..5dea177cef53 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -1420,6 +1420,9 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
> goto out;
> }
>
> + /* Up the trigger_data count to make sure nothing frees it on failure */
> + event_trigger_init(trigger_ops, trigger_data);
> +
> if (trigger) {
> number = strsep(&trigger, ":");
>
> @@ -1470,6 +1473,7 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
> goto out_disable;
> /* Just return zero, not the number of enabled functions */
> ret = 0;
> + event_trigger_free(trigger_ops, trigger_data);
> out:
> return ret;
>
> @@ -1480,7 +1484,7 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
> out_free:
> if (cmd_ops->set_filter)
> cmd_ops->set_filter(NULL, trigger_data, NULL);
> - kfree(trigger_data);
> + event_trigger_free(trigger_ops, trigger_data);
> kfree(enable_data);
> goto out;
> }
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-25 22:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-25 12:57 [PATCH] tracing: Fix double free of event_trigger_data Steven Rostedt
2018-07-25 15:43 ` Masami Hiramatsu
2018-07-25 16:40 ` Steven Rostedt
2018-07-25 19:29 ` Steven Rostedt
2018-07-25 19:53 ` Steven Rostedt
2018-07-25 22:51 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).