| Message ID | 20220323103912.097ad3a8@gandalf.local.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tracing: Have trace event string test handle zero length strings | expand |
On Wed, Mar 23, 2022 at 10:39:12AM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > If a trace event has in its TP_printk(): > > "%*.s", len, len ? __get_str(string) : NULL > > It is perfectly valid if len is zero and passing in the NULL. > Unfortunately, the runtime string check at time of reading the trace sees > the NULL and flags it as a bad string and produces a WARN_ON(). > > Handle this case by passing into the test function if the format has an > asterisk (star) and if so, if the length is zero, then mark it as safe. > > Link: https://lore.kernel.org/all/YjsWzuw5FbWPrdqq@bfoster/ > > Cc: stable@vger.kernel.org > Reported-by: Brian Foster <bfoster@redhat.com> > Fixes: 9a6944fee68e2 ("tracing: Add a verifier to check string pointers for trace events") > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- FWIW: Tested-by: Brian Foster <bfoster@redhat.com> Thanks for the patch! > kernel/trace/trace.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index eb44418574f9..96265a717ca4 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -3663,12 +3663,17 @@ static char *trace_iter_expand_format(struct trace_iterator *iter) > } > > /* Returns true if the string is safe to dereference from an event */ > -static bool trace_safe_str(struct trace_iterator *iter, const char *str) > +static bool trace_safe_str(struct trace_iterator *iter, const char *str, > + bool star, int len) > { > unsigned long addr = (unsigned long)str; > struct trace_event *trace_event; > struct trace_event_call *event; > > + /* Ignore strings with no length */ > + if (star && !len) > + return true; > + > /* OK if part of the event data */ > if ((addr >= (unsigned long)iter->ent) && > (addr < (unsigned long)iter->ent + iter->ent_size)) > @@ -3854,7 +3859,7 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, > * instead. See samples/trace_events/trace-events-sample.h > * for reference. > */ > - if (WARN_ONCE(!trace_safe_str(iter, str), > + if (WARN_ONCE(!trace_safe_str(iter, str, star, len), > "fmt: '%s' current_buffer: '%s'", > fmt, show_buffer(&iter->seq))) { > int ret; > -- > 2.35.1 >
On Wed, 23 Mar 2022 10:46:15 -0400 Brian Foster <bfoster@redhat.com> wrote: > FWIW: > > Tested-by: Brian Foster <bfoster@redhat.com> Thanks, I'll add this tag. -- Steve
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index eb44418574f9..96265a717ca4 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -3663,12 +3663,17 @@ static char *trace_iter_expand_format(struct trace_iterator *iter) } /* Returns true if the string is safe to dereference from an event */ -static bool trace_safe_str(struct trace_iterator *iter, const char *str) +static bool trace_safe_str(struct trace_iterator *iter, const char *str, + bool star, int len) { unsigned long addr = (unsigned long)str; struct trace_event *trace_event; struct trace_event_call *event; + /* Ignore strings with no length */ + if (star && !len) + return true; + /* OK if part of the event data */ if ((addr >= (unsigned long)iter->ent) && (addr < (unsigned long)iter->ent + iter->ent_size)) @@ -3854,7 +3859,7 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, * instead. See samples/trace_events/trace-events-sample.h * for reference. */ - if (WARN_ONCE(!trace_safe_str(iter, str), + if (WARN_ONCE(!trace_safe_str(iter, str, star, len), "fmt: '%s' current_buffer: '%s'", fmt, show_buffer(&iter->seq))) { int ret;