Re: [PATCH] tracing: Check return value of __create_val_fields() before using its result
Hi Steve, On Tue, 2020-10-13 at 15:48 -0400, Steven Rostedt wrote: > From: "Steven Rostedt (VMware)" > > 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: 0008 > #PF: supervisor read access in kernel mode > #PF: error_code(0x) - not-present page > PGD 0 P4D 0 > Oops: [#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 > 42 08 04 74 17 41 8b 8f c0 00 00 00 8d 71 01 41 89 b7 c0 00 00 > RSP: 0018:959213d53db0 EFLAGS: 00010202 > RAX: ffea RBX: RCX: 00084c04 > RDX: RSI: df7326aefebd174c RDI: 00031080 > RBP: 0002 R08: 0001 R09: 0001 > R10: 0001 R11: 0046 R12: 959211dcf690 > R13: 0001 R14: 95925a36e370 R15: 959251c89800 > FS: 7fb9ea934740() GS:95925ab0() > knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0008 CR3: c976c005 CR4: 001706e0 > 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 Thanks, Tom > Signed-off-by: Steven Rostedt (VMware) > --- > 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;
Re: [PATCH] tracing: Check return value of __create_val_fields() before using its result
On Tue, 13 Oct 2020 15:48:52 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (VMware)" > > 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: 0008 > #PF: supervisor read access in kernel mode > #PF: error_code(0x) - not-present page > PGD 0 P4D 0 > Oops: [#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 42 08 04 74 17 > 41 8b 8f c0 00 00 00 8d 71 01 41 89 b7 c0 00 00 > RSP: 0018:959213d53db0 EFLAGS: 00010202 > RAX: ffea RBX: RCX: 00084c04 > RDX: RSI: df7326aefebd174c RDI: 00031080 > RBP: 0002 R08: 0001 R09: 0001 > R10: 0001 R11: 0046 R12: 959211dcf690 > R13: 0001 R14: 95925a36e370 R15: 959251c89800 > FS: 7fb9ea934740() GS:95925ab0() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0008 CR3: c976c005 CR4: 001706e0 > 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 Thank you, > Fixes: 63a1e5de3006 ("tracing: Save normal string variables") > Signed-off-by: Steven Rostedt (VMware) > --- > 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
[PATCH] tracing: Check return value of __create_val_fields() before using its result
From: "Steven Rostedt (VMware)" 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: 0008 #PF: supervisor read access in kernel mode #PF: error_code(0x) - not-present page PGD 0 P4D 0 Oops: [#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 42 08 04 74 17 41 8b 8f c0 00 00 00 8d 71 01 41 89 b7 c0 00 00 RSP: 0018:959213d53db0 EFLAGS: 00010202 RAX: ffea RBX: RCX: 00084c04 RDX: RSI: df7326aefebd174c RDI: 00031080 RBP: 0002 R08: 0001 R09: 0001 R10: 0001 R11: 0046 R12: 959211dcf690 R13: 0001 R14: 95925a36e370 R15: 959251c89800 FS: 7fb9ea934740() GS:95925ab0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 0008 CR3: c976c005 CR4: 001706e0 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) --- 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