* [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions
@ 2018-11-08 14:41 Tom Zanussi
2018-11-08 14:41 ` [PATCH 1/2] tracing: Prevent hist_field_var_ref() from accessing NULL tracing_map_elts Tom Zanussi
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Tom Zanussi @ 2018-11-08 14:41 UTC (permalink / raw)
To: rostedt; +Cc: vincent, linux-kernel
From: Tom Zanussi <tom.zanussi@linux.intel.com>
Hi Steve,
This is a fix for a user-reported bug in the hist triggers, where if a
variable reference is used in an expression in a histogram key, it
results in a NULL pointer dereference and subsequent Oops.
I separated the fix into two small patches, the first preventing the
immediate problem bu disallowing a var_ref from ever accessing a NULL
element, and the second disallowing a user from ever constructing such
a key.
Thanks,
Tom
The following changes since commit ee474b81fe5aa5dc0faae920bf66240fbf55f891:
tracing/kprobes: Fix strpbrk() argument order (2018-11-05 09:47:14 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git ftrace/key-ref-fix
Tom Zanussi (2):
tracing: Prevent hist_field_var_ref() from accessing NULL
tracing_map_elts
tracing: Check keys for variable references in expressions too
kernel/trace/trace_events_hist.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
--
2.14.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] tracing: Prevent hist_field_var_ref() from accessing NULL tracing_map_elts
2018-11-08 14:41 [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Tom Zanussi
@ 2018-11-08 14:41 ` Tom Zanussi
2018-11-08 14:41 ` [PATCH 2/2] tracing: Check keys for variable references in expressions too Tom Zanussi
2018-11-08 18:45 ` [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Steven Rostedt
2 siblings, 0 replies; 5+ messages in thread
From: Tom Zanussi @ 2018-11-08 14:41 UTC (permalink / raw)
To: rostedt; +Cc: vincent, linux-kernel
From: Tom Zanussi <tom.zanussi@linux.intel.com>
hist_field_var_ref() is an implementation of hist_field_fn_t(), which
can be called with a null tracing_map_elt elt param when assembling a
key in event_hist_trigger().
In the case of hist_field_var_ref() this doesn't make sense, because a
variable can only be resolved by looking it up using an already
assembled key i.e. a variable can't be used to assemble a key since
the key is required in order to access the variable.
Upper layers should prevent the user from constructing a key using a
variable in the first place, but in case one slips through, it
shouldn't cause a NULL pointer dereference. Also if one does slip
through, we want to know about it, so emit a one-time warning in that
case.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Reported-by: Vincent Bernat <vincent@bernat.ch>
---
kernel/trace/trace_events_hist.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index eb908ef2ecec..3795207a48a0 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1632,6 +1632,9 @@ static u64 hist_field_var_ref(struct hist_field *hist_field,
struct hist_elt_data *elt_data;
u64 var_val = 0;
+ if (WARN_ON_ONCE(!elt))
+ return var_val;
+
elt_data = elt->private_data;
var_val = elt_data->var_ref_vals[hist_field->var_ref_idx];
--
2.14.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] tracing: Check keys for variable references in expressions too
2018-11-08 14:41 [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Tom Zanussi
2018-11-08 14:41 ` [PATCH 1/2] tracing: Prevent hist_field_var_ref() from accessing NULL tracing_map_elts Tom Zanussi
@ 2018-11-08 14:41 ` Tom Zanussi
2018-11-08 18:45 ` [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Steven Rostedt
2 siblings, 0 replies; 5+ messages in thread
From: Tom Zanussi @ 2018-11-08 14:41 UTC (permalink / raw)
To: rostedt; +Cc: vincent, linux-kernel
From: Tom Zanussi <tom.zanussi@linux.intel.com>
There's an existing check for variable references in keys, but it
doesn't go far enough. It checks whether a key field is a variable
reference but doesn't check whether it's an expression containing
variable references, which can cause the same problems for callers.
Use the existing field_has_hist_vars() function rather than a direct
top-level flag check to catch all possible variable references.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Reported-by: Vincent Bernat <vincent@bernat.ch>
---
kernel/trace/trace_events_hist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 3795207a48a0..bf993a9a65ec 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3970,8 +3970,8 @@ static int create_key_field(struct hist_trigger_data *hist_data,
goto out;
}
- if (hist_field->flags & HIST_FIELD_FL_VAR_REF) {
- hist_err("Using variable references as keys not supported: ", field_str);
+ if (field_has_hist_vars(hist_field, 0)) {
+ hist_err("Using variable references in keys not supported: ", field_str);
destroy_hist_field(hist_field, 0);
ret = -EINVAL;
goto out;
--
2.14.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions
2018-11-08 14:41 [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Tom Zanussi
2018-11-08 14:41 ` [PATCH 1/2] tracing: Prevent hist_field_var_ref() from accessing NULL tracing_map_elts Tom Zanussi
2018-11-08 14:41 ` [PATCH 2/2] tracing: Check keys for variable references in expressions too Tom Zanussi
@ 2018-11-08 18:45 ` Steven Rostedt
2018-11-08 19:12 ` Tom Zanussi
2 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2018-11-08 18:45 UTC (permalink / raw)
To: Tom Zanussi; +Cc: vincent, linux-kernel
On Thu, 8 Nov 2018 08:41:47 -0600
Tom Zanussi <zanussi@kernel.org> wrote:
> From: Tom Zanussi <tom.zanussi@linux.intel.com>
>
> Hi Steve,
>
> This is a fix for a user-reported bug in the hist triggers, where if a
> variable reference is used in an expression in a histogram key, it
> results in a NULL pointer dereference and subsequent Oops.
How does one add a variable reference as an expression in a
histogram key?
Can you show an example?
-- Steve
>
> I separated the fix into two small patches, the first preventing the
> immediate problem bu disallowing a var_ref from ever accessing a NULL
> element, and the second disallowing a user from ever constructing such
> a key.
>
> Thanks,
>
> Tom
>
> The following changes since commit ee474b81fe5aa5dc0faae920bf66240fbf55f891:
>
> tracing/kprobes: Fix strpbrk() argument order (2018-11-05 09:47:14 -0500)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git ftrace/key-ref-fix
>
> Tom Zanussi (2):
> tracing: Prevent hist_field_var_ref() from accessing NULL
> tracing_map_elts
> tracing: Check keys for variable references in expressions too
>
> kernel/trace/trace_events_hist.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions
2018-11-08 18:45 ` [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Steven Rostedt
@ 2018-11-08 19:12 ` Tom Zanussi
0 siblings, 0 replies; 5+ messages in thread
From: Tom Zanussi @ 2018-11-08 19:12 UTC (permalink / raw)
To: Steven Rostedt; +Cc: vincent, linux-kernel
Hi Steve,
On Thu, 2018-11-08 at 13:45 -0500, Steven Rostedt wrote:
> On Thu, 8 Nov 2018 08:41:47 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
> > From: Tom Zanussi <tom.zanussi@linux.intel.com>
> >
> > Hi Steve,
> >
> > This is a fix for a user-reported bug in the hist triggers, where
> > if a
> > variable reference is used in an expression in a histogram key, it
> > results in a NULL pointer dereference and subsequent Oops.
>
> How does one add a variable reference as an expression in a
> histogram key?
>
> Can you show an example?
>
Sure, here's the example that showed the problem, and which the patch
fixes:
echo 'r:fib_table_lookup_ret fib_table_lookup $retval' > /sys/kernel/debug/tracing/kprobe_events
echo 'hist:keys=cpu:ts0=common_timestamp' > /sys/kernel/debug/tracing/events/fib/fib_table_lookup/trigger
echo 'hist:keys=cpu,common_timestamp-$ts0' > /sys/kernel/debug/tracing/events/kprobes/fib_table_lookup_ret/trigger
The following is also a variable reference in an expression, but in
this case the expression is the variable reference alone, which the
existing code handles properly and returns -EINVAL.
echo 'hist:keys=cpu,$ts0' > /sys/kernel/debug/tracing/events/kprobes/fib_table_lookup_ret/trigger
Tom
> -- Steve
>
> >
> > I separated the fix into two small patches, the first preventing
> > the
> > immediate problem bu disallowing a var_ref from ever accessing a
> > NULL
> > element, and the second disallowing a user from ever constructing
> > such
> > a key.
> >
> > Thanks,
> >
> > Tom
> >
> > The following changes since commit
> > ee474b81fe5aa5dc0faae920bf66240fbf55f891:
> >
> > tracing/kprobes: Fix strpbrk() argument order (2018-11-05
> > 09:47:14 -0500)
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-
> > trace.git ftrace/key-ref-fix
> >
> > Tom Zanussi (2):
> > tracing: Prevent hist_field_var_ref() from accessing NULL
> > tracing_map_elts
> > tracing: Check keys for variable references in expressions too
> >
> > kernel/trace/trace_events_hist.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-08 19:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-08 14:41 [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Tom Zanussi
2018-11-08 14:41 ` [PATCH 1/2] tracing: Prevent hist_field_var_ref() from accessing NULL tracing_map_elts Tom Zanussi
2018-11-08 14:41 ` [PATCH 2/2] tracing: Check keys for variable references in expressions too Tom Zanussi
2018-11-08 18:45 ` [PATCH 0/2] tracing: Fix NULL pointer bug in hist key expressions Steven Rostedt
2018-11-08 19:12 ` Tom Zanussi
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).