* [PATCH] tracing: Verify event formats that have "%*p.."
@ 2025-03-27 15:49 Steven Rostedt
2025-03-27 18:32 ` Libo Chen
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2025-03-27 15:49 UTC (permalink / raw)
To: LKML, Linux Trace Kernel; +Cc: Masami Hiramatsu, Mathieu Desnoyers, Libo Chen
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://lore.kernel.org/all/bcba4d76-2c3f-4d11-baf0-02905db953dd@oracle.com/
Reported-by: Libo Chen <libo.chen@oracle.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
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 7b3ef1d26167..34e7b4af9f88 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..0622c9e3f2be 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))
);
/*
--
2.47.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tracing: Verify event formats that have "%*p.."
2025-03-27 15:49 [PATCH] tracing: Verify event formats that have "%*p.." Steven Rostedt
@ 2025-03-27 18:32 ` Libo Chen
2025-03-27 18:38 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Libo Chen @ 2025-03-27 18:32 UTC (permalink / raw)
To: Steven Rostedt, LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers
On 3/27/25 08:49, 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!LTIVO1_O9wY2hBAnpNF5zcB1EFlC77zOnu4oVcM4DoD77p5ZO_m1LFZdPVJPj4spzye4JntXHOqOVxirPQ$
>
Thanks Steve, a minor style issue below, otherwise LGTM. I have tested it, it works.
Reviewed-by: Libo Chen <libo.chen@oracle.com>
Tested-by: Libo Chen <libo.chen@oracle.com>
-- Libo
> Reported-by: Libo Chen <libo.chen@oracle.com>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> 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 7b3ef1d26167..34e7b4af9f88 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..0622c9e3f2be 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))
__get_dynamic_array_len() calls don't seem to be aligned with the ones above
> );
>
> /*
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracing: Verify event formats that have "%*p.."
2025-03-27 18:32 ` Libo Chen
@ 2025-03-27 18:38 ` Steven Rostedt
2025-03-27 21:00 ` Libo Chen
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2025-03-27 18:38 UTC (permalink / raw)
To: Libo Chen; +Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers
On Thu, 27 Mar 2025 11:32:23 -0700
Libo Chen <libo.chen@oracle.com> wrote:
> On 3/27/25 08:49, 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!LTIVO1_O9wY2hBAnpNF5zcB1EFlC77zOnu4oVcM4DoD77p5ZO_m1LFZdPVJPj4spzye4JntXHOqOVxirPQ$
> >
> Thanks Steve, a minor style issue below, otherwise LGTM. I have tested it, it works.
>
> Reviewed-by: Libo Chen <libo.chen@oracle.com>
> Tested-by: Libo Chen <libo.chen@oracle.com>
Thanks for testing.
> > /*
> > * 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))
>
> __get_dynamic_array_len() calls don't seem to be aligned with the ones above
>
Ah yeah. I'll update. Note, this isn't going to go into the kernel until at
least 6.16.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Verify event formats that have "%*p.."
2025-03-27 18:38 ` Steven Rostedt
@ 2025-03-27 21:00 ` Libo Chen
2025-03-27 21:20 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Libo Chen @ 2025-03-27 21:00 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers
On 3/27/25 11:38, Steven Rostedt wrote:
> On Thu, 27 Mar 2025 11:32:23 -0700
> Libo Chen <libo.chen@oracle.com> wrote:
>
>> On 3/27/25 08:49, 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!LTIVO1_O9wY2hBAnpNF5zcB1EFlC77zOnu4oVcM4DoD77p5ZO_m1LFZdPVJPj4spzye4JntXHOqOVxirPQ$
>>>
>> Thanks Steve, a minor style issue below, otherwise LGTM. I have tested it, it works.
>>
>> Reviewed-by: Libo Chen <libo.chen@oracle.com>
>> Tested-by: Libo Chen <libo.chen@oracle.com>
>
> Thanks for testing.
>
>>> /*
>>> * 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))
>>
>> __get_dynamic_array_len() calls don't seem to be aligned with the ones above
>>
>
> Ah yeah. I'll update. Note, this isn't going to go into the kernel until at
> least 6.16.
>
Okay thanks for the note. I will stick to %lx for now and change it to %*pbl
once this gets in.
Libo
> -- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Verify event formats that have "%*p.."
2025-03-27 21:00 ` Libo Chen
@ 2025-03-27 21:20 ` Steven Rostedt
2025-03-27 21:35 ` Libo Chen
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2025-03-27 21:20 UTC (permalink / raw)
To: Libo Chen; +Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers
On Thu, 27 Mar 2025 14:00:44 -0700
Libo Chen <libo.chen@oracle.com> wrote:
> > Ah yeah. I'll update. Note, this isn't going to go into the kernel until at
> > least 6.16.
> >
>
> Okay thanks for the note. I will stick to %lx for now and change it to %*pbl
> once this gets in.
This shouldn't be a blocker for you. Do you get any errors?
Hmm, if this causes bugs (perhaps you have arguments after the %*pbl) then
I can add this as a fix and not an enhancement.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Verify event formats that have "%*p.."
2025-03-27 21:20 ` Steven Rostedt
@ 2025-03-27 21:35 ` Libo Chen
0 siblings, 0 replies; 6+ messages in thread
From: Libo Chen @ 2025-03-27 21:35 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers
On 3/27/25 14:20, Steven Rostedt wrote:
> On Thu, 27 Mar 2025 14:00:44 -0700
> Libo Chen <libo.chen@oracle.com> wrote:
>
>>> Ah yeah. I'll update. Note, this isn't going to go into the kernel until at
>>> least 6.16.
>>>
>>
>> Okay thanks for the note. I will stick to %lx for now and change it to %*pbl
>> once this gets in.
>
> This shouldn't be a blocker for you. Do you get any errors?
>
> Hmm, if this causes bugs (perhaps you have arguments after the %*pbl) then
> I can add this as a fix and not an enhancement.
Not sure if I will call it an error. But without this patch, I will just get
"nil" when using %*p so this information is lost here. If I go ahead with
%*pbl, nodemask value will not show up in the trace until this one is merged.
Libo
>
> -- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-27 21:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-27 15:49 [PATCH] tracing: Verify event formats that have "%*p.." Steven Rostedt
2025-03-27 18:32 ` Libo Chen
2025-03-27 18:38 ` Steven Rostedt
2025-03-27 21:00 ` Libo Chen
2025-03-27 21:20 ` Steven Rostedt
2025-03-27 21:35 ` Libo Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox