* [PATCH] tracing: Check return value of __create_val_fields() before using its result
@ 2020-10-13 19:48 Steven Rostedt
2020-10-14 3:25 ` Masami Hiramatsu
2020-10-15 13:54 ` Tom Zanussi
0 siblings, 2 replies; 3+ messages in thread
From: Steven Rostedt @ 2020-10-13 19:48 UTC (permalink / raw)
To: LKML; +Cc: Masami Hiramatsu, Tom Zanussi, Ingo Molnar
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
After having a typo for writing a histogram trigger.
Wrote:
echo 'hist:key=pid:ts=common_timestamp.usec' > events/sched/sched_waking/trigger
Instead of:
echo 'hist:key=pid:ts=common_timestamp.usecs' > events/sched/sched_waking/trigger
and the following crash happened:
BUG: kernel NULL pointer dereference, address: 0000000000000008
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] PREEMPT SMP PTI
CPU: 4 PID: 1641 Comm: sh Not tainted 5.9.0-rc5-test+ #549
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
RIP: 0010:event_hist_trigger_func+0x70b/0x1ee0
Code: 24 08 89 d5 49 89 cc e9 8c 00 00 00 4c 89 f2 41 b9 00 10 00 00 4c 89 e1 44 89 ee 4c 89 ff e8 dc d3 ff ff 45 89 ea 4b 8b 14 d7 <f6> 42 08 04 74 17 41 8b 8f c0 00 00 00 8d 71 01 41 89 b7 c0 00 00
RSP: 0018:ffff959213d53db0 EFLAGS: 00010202
RAX: ffffffffffffffea RBX: 0000000000000000 RCX: 0000000000084c04
RDX: 0000000000000000 RSI: df7326aefebd174c RDI: 0000000000031080
RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000001 R11: 0000000000000046 R12: ffff959211dcf690
R13: 0000000000000001 R14: ffff95925a36e370 R15: ffff959251c89800
FS: 00007fb9ea934740(0000) GS:ffff95925ab00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000008 CR3: 00000000c976c005 CR4: 00000000001706e0
Call Trace:
? trigger_process_regex+0x78/0x110
trigger_process_regex+0xc5/0x110
event_trigger_write+0x71/0xd0
vfs_write+0xca/0x210
ksys_write+0x70/0xf0
do_syscall_64+0x33/0x40
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7fb9eaa29487
Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
This was caused by accessing the hlist_data fields after the call to
__create_val_fields() without checking if the creation succeed.
Fixes: 63a1e5de3006 ("tracing: Save normal string variables")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index c74a7d157306..96c3f86b81c5 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3687,7 +3687,7 @@ static int create_var_field(struct hist_trigger_data *hist_data,
ret = __create_val_field(hist_data, val_idx, file, var_name, expr_str, flags);
- if (hist_data->fields[val_idx]->flags & HIST_FIELD_FL_STRING)
+ if (!ret && hist_data->fields[val_idx]->flags & HIST_FIELD_FL_STRING)
hist_data->fields[val_idx]->var_str_idx = hist_data->n_var_str++;
return ret;
--
2.25.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] tracing: Check return value of __create_val_fields() before using its result
2020-10-13 19:48 [PATCH] tracing: Check return value of __create_val_fields() before using its result Steven Rostedt
@ 2020-10-14 3:25 ` Masami Hiramatsu
2020-10-15 13:54 ` Tom Zanussi
1 sibling, 0 replies; 3+ messages in thread
From: Masami Hiramatsu @ 2020-10-14 3:25 UTC (permalink / raw)
To: Steven Rostedt; +Cc: LKML, Masami Hiramatsu, Tom Zanussi, Ingo Molnar
On Tue, 13 Oct 2020 15:48:52 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> After having a typo for writing a histogram trigger.
>
> Wrote:
> echo 'hist:key=pid:ts=common_timestamp.usec' > events/sched/sched_waking/trigger
>
> Instead of:
> echo 'hist:key=pid:ts=common_timestamp.usecs' > events/sched/sched_waking/trigger
>
> and the following crash happened:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: 0000 [#1] PREEMPT SMP PTI
> CPU: 4 PID: 1641 Comm: sh Not tainted 5.9.0-rc5-test+ #549
> Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
> RIP: 0010:event_hist_trigger_func+0x70b/0x1ee0
> Code: 24 08 89 d5 49 89 cc e9 8c 00 00 00 4c 89 f2 41 b9 00 10 00 00 4c 89 e1 44 89 ee 4c 89 ff e8 dc d3 ff ff 45 89 ea 4b 8b 14 d7 <f6> 42 08 04 74 17 41 8b 8f c0 00 00 00 8d 71 01 41 89 b7 c0 00 00
> RSP: 0018:ffff959213d53db0 EFLAGS: 00010202
> RAX: ffffffffffffffea RBX: 0000000000000000 RCX: 0000000000084c04
> RDX: 0000000000000000 RSI: df7326aefebd174c RDI: 0000000000031080
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000046 R12: ffff959211dcf690
> R13: 0000000000000001 R14: ffff95925a36e370 R15: ffff959251c89800
> FS: 00007fb9ea934740(0000) GS:ffff95925ab00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000008 CR3: 00000000c976c005 CR4: 00000000001706e0
> Call Trace:
> ? trigger_process_regex+0x78/0x110
> trigger_process_regex+0xc5/0x110
> event_trigger_write+0x71/0xd0
> vfs_write+0xca/0x210
> ksys_write+0x70/0xf0
> do_syscall_64+0x33/0x40
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7fb9eaa29487
> Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
>
> This was caused by accessing the hlist_data fields after the call to
> __create_val_fields() without checking if the creation succeed.
>
This looks good to me.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you,
> Fixes: 63a1e5de3006 ("tracing: Save normal string variables")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> kernel/trace/trace_events_hist.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index c74a7d157306..96c3f86b81c5 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -3687,7 +3687,7 @@ static int create_var_field(struct hist_trigger_data *hist_data,
>
> ret = __create_val_field(hist_data, val_idx, file, var_name, expr_str, flags);
>
> - if (hist_data->fields[val_idx]->flags & HIST_FIELD_FL_STRING)
> + if (!ret && hist_data->fields[val_idx]->flags & HIST_FIELD_FL_STRING)
> hist_data->fields[val_idx]->var_str_idx = hist_data->n_var_str++;
>
> return ret;
> --
> 2.25.4
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tracing: Check return value of __create_val_fields() before using its result
2020-10-13 19:48 [PATCH] tracing: Check return value of __create_val_fields() before using its result Steven Rostedt
2020-10-14 3:25 ` Masami Hiramatsu
@ 2020-10-15 13:54 ` Tom Zanussi
1 sibling, 0 replies; 3+ messages in thread
From: Tom Zanussi @ 2020-10-15 13:54 UTC (permalink / raw)
To: Steven Rostedt, LKML; +Cc: Masami Hiramatsu, Ingo Molnar
Hi Steve,
On Tue, 2020-10-13 at 15:48 -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> After having a typo for writing a histogram trigger.
>
> Wrote:
> echo 'hist:key=pid:ts=common_timestamp.usec' >
> events/sched/sched_waking/trigger
>
> Instead of:
> echo 'hist:key=pid:ts=common_timestamp.usecs' >
> events/sched/sched_waking/trigger
>
> and the following crash happened:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: 0000 [#1] PREEMPT SMP PTI
> CPU: 4 PID: 1641 Comm: sh Not tainted 5.9.0-rc5-test+ #549
> Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01
> v03.03 07/14/2016
> RIP: 0010:event_hist_trigger_func+0x70b/0x1ee0
> Code: 24 08 89 d5 49 89 cc e9 8c 00 00 00 4c 89 f2 41 b9 00 10 00 00
> 4c 89 e1 44 89 ee 4c 89 ff e8 dc d3 ff ff 45 89 ea 4b 8b 14 d7 <f6>
> 42 08 04 74 17 41 8b 8f c0 00 00 00 8d 71 01 41 89 b7 c0 00 00
> RSP: 0018:ffff959213d53db0 EFLAGS: 00010202
> RAX: ffffffffffffffea RBX: 0000000000000000 RCX: 0000000000084c04
> RDX: 0000000000000000 RSI: df7326aefebd174c RDI: 0000000000031080
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000046 R12: ffff959211dcf690
> R13: 0000000000000001 R14: ffff95925a36e370 R15: ffff959251c89800
> FS: 00007fb9ea934740(0000) GS:ffff95925ab00000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000008 CR3: 00000000c976c005 CR4: 00000000001706e0
> Call Trace:
> ? trigger_process_regex+0x78/0x110
> trigger_process_regex+0xc5/0x110
> event_trigger_write+0x71/0xd0
> vfs_write+0xca/0x210
> ksys_write+0x70/0xf0
> do_syscall_64+0x33/0x40
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7fb9eaa29487
> Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f
> 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48>
> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
>
> This was caused by accessing the hlist_data fields after the call to
> __create_val_fields() without checking if the creation succeed.
>
> Fixes: 63a1e5de3006 ("tracing: Save normal string variables")
Yes, this fixes the fix, sigh.
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Thanks,
Tom
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> kernel/trace/trace_events_hist.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events_hist.c
> b/kernel/trace/trace_events_hist.c
> index c74a7d157306..96c3f86b81c5 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -3687,7 +3687,7 @@ static int create_var_field(struct
> hist_trigger_data *hist_data,
>
> ret = __create_val_field(hist_data, val_idx, file, var_name,
> expr_str, flags);
>
> - if (hist_data->fields[val_idx]->flags & HIST_FIELD_FL_STRING)
> + if (!ret && hist_data->fields[val_idx]->flags &
> HIST_FIELD_FL_STRING)
> hist_data->fields[val_idx]->var_str_idx = hist_data-
> >n_var_str++;
>
> return ret;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-15 13:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13 19:48 [PATCH] tracing: Check return value of __create_val_fields() before using its result Steven Rostedt
2020-10-14 3:25 ` Masami Hiramatsu
2020-10-15 13:54 ` Tom Zanussi
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.