| Message ID | 20250327195311.2d89ec66@gandalf.local.home (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] tracing: Verify event formats that have "%*p.." | expand |
On 3/27/25 16:53, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The trace event verifier checks the formats of trace events to make sure > that they do not point at memory that is not in the trace event itself or > in data that will never be freed. If an event references data that was > allocated when the event triggered and that same data is freed before the > event is read, then the kernel can crash by reading freed memory. > > The verifier runs at boot up (or module load) and scans the print formats > of the events and checks their arguments to make sure that dereferenced > pointers are safe. If the format uses "%*p.." the verifier will ignore it, > and that could be dangerous. Cover this case as well. > > Also add to the sample code a use case of "%*pbl". > > Link: https://urldefense.com/v3/__https://lore.kernel.org/all/bcba4d76-2c3f-4d11-baf0-02905db953dd@oracle.com/__;!!ACWV5N9M2RV99hQ!L9dryF2Ft0p8H1X2Q6EPsm0j03lJOixhB2V-_zkgCOBUoIS5V7B8hmOZxCpHjzNnCodkqNmxUulCC5wPbg$ > this is no longer a blocker for me, as I mentioned in a different thread: https://lore.kernel.org/all/4f5453e8-61b6-45d1-8c6c-8b4fb99a6448@oracle.com/ Thanks, Libo > Reported-by: Libo Chen <libo.chen@oracle.com> > Reviewed-by: Libo Chen <libo.chen@oracle.com> > Tested-by: Libo Chen <libo.chen@oracle.com> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > Changes since v1: https://urldefense.com/v3/__https://lore.kernel.org/20250327114911.2c713511@gandalf.local.home__;!!ACWV5N9M2RV99hQ!L9dryF2Ft0p8H1X2Q6EPsm0j03lJOixhB2V-_zkgCOBUoIS5V7B8hmOZxCpHjzNnCodkqNmxUumNh4GyCw$ > > - Fixed indentation in sample header. > > kernel/trace/trace_events.c | 7 +++++++ > samples/trace_events/trace-events-sample.h | 8 ++++++-- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c > index 8e7603acca21..ceeedcb5940b 100644 > --- a/kernel/trace/trace_events.c > +++ b/kernel/trace/trace_events.c > @@ -470,6 +470,7 @@ static void test_event_printk(struct trace_event_call *call) > case '%': > continue; > case 'p': > + do_pointer: > /* Find dereferencing fields */ > switch (fmt[i + 1]) { > case 'B': case 'R': case 'r': > @@ -498,6 +499,12 @@ static void test_event_printk(struct trace_event_call *call) > continue; > if (fmt[i + j] == '*') { > star = true; > + /* Handle %*pbl case */ > + if (!j && fmt[i + 1] == 'p') { > + arg++; > + i++; > + goto do_pointer; > + } > continue; > } > if ((fmt[i + j] == 's')) { > diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h > index 999f78d380ae..1a05fc153353 100644 > --- a/samples/trace_events/trace-events-sample.h > +++ b/samples/trace_events/trace-events-sample.h > @@ -319,7 +319,8 @@ TRACE_EVENT(foo_bar, > __assign_cpumask(cpum, cpumask_bits(mask)); > ), > > - TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s", __entry->foo, __entry->bar, > + TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s [%d] %*pbl", > + __entry->foo, __entry->bar, > > /* > * Notice here the use of some helper functions. This includes: > @@ -370,7 +371,10 @@ TRACE_EVENT(foo_bar, > > __get_str(str), __get_str(lstr), > __get_bitmask(cpus), __get_cpumask(cpum), > - __get_str(vstr)) > + __get_str(vstr), > + __get_dynamic_array_len(cpus), > + __get_dynamic_array_len(cpus), > + __get_dynamic_array(cpus)) > ); > > /*
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index 8e7603acca21..ceeedcb5940b 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -470,6 +470,7 @@ static void test_event_printk(struct trace_event_call *call) case '%': continue; case 'p': + do_pointer: /* Find dereferencing fields */ switch (fmt[i + 1]) { case 'B': case 'R': case 'r': @@ -498,6 +499,12 @@ static void test_event_printk(struct trace_event_call *call) continue; if (fmt[i + j] == '*') { star = true; + /* Handle %*pbl case */ + if (!j && fmt[i + 1] == 'p') { + arg++; + i++; + goto do_pointer; + } continue; } if ((fmt[i + j] == 's')) { diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h index 999f78d380ae..1a05fc153353 100644 --- a/samples/trace_events/trace-events-sample.h +++ b/samples/trace_events/trace-events-sample.h @@ -319,7 +319,8 @@ TRACE_EVENT(foo_bar, __assign_cpumask(cpum, cpumask_bits(mask)); ), - TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s", __entry->foo, __entry->bar, + TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s [%d] %*pbl", + __entry->foo, __entry->bar, /* * Notice here the use of some helper functions. This includes: @@ -370,7 +371,10 @@ TRACE_EVENT(foo_bar, __get_str(str), __get_str(lstr), __get_bitmask(cpus), __get_cpumask(cpum), - __get_str(vstr)) + __get_str(vstr), + __get_dynamic_array_len(cpus), + __get_dynamic_array_len(cpus), + __get_dynamic_array(cpus)) ); /*