[PATCH] tracing: Fix NULL dereference when copying keys for a field variable

[PATCH] tracing: Fix NULL dereference when copying keys for a field variable

[Donggeun Yoo]

create_field_var_hist() builds a hist trigger on the onmatch() event by
copying the key list of the compatible histogram found there, reading
each name from key_field->field->name. That pointer is NULL when the key
is not an event field: parse_field() leaves it NULL for common_cpu,
common_comm, common_timestamp, common_stacktrace and hitcount.
compatible_keys() compares only the type, size and signedness of each
key, so two common_cpu keys match and the loop faults.

The generated trigger is a real histogram whose element the variable is
later read out of, so it has to bucket the same way as the one it
mirrors. Render the keys with expr_field_str(), which carries .log2 and
.usecs, modifiers that change a key's value. Two things it did not carry
are added here. get_hist_field_flags() reports .stacktrace for the
common_stacktrace pseudo-field too, which parse_field() takes only on a
real field, so gate it on having one. And it reports a bare "buckets"
with the size held separately in hist_field->buckets, so append that the
way hist_field_print() does. expr_field_str() also renders expression
operands, so an expression over a bucketed field now prints the size
too, matching what hist_field_print() already shows for the key.

  # echo 'hist:keys=common_cpu:ts0=common_timestamp.usecs' > \
      events/sched/sched_waking/trigger
  # echo 'my_synth u64 lat; int prio' > synthetic_events
  # echo 'hist:keys=common_cpu:wakeup_lat=common_timestamp.usecs-$ts0:\
      onmatch(sched.sched_waking).my_synth($wakeup_lat,prio)' > \
      events/sched/sched_switch/trigger

  Oops: general protection fault, probably for non-canonical address
  0xdffffc0000000002
  KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
  RIP: 0010:action_create+0x1cb4/0x2e50
  Call Trace:
   event_hist_trigger_parse+0x3e47/0x69e0
   trigger_process_regex+0x1a6/0x250
   event_trigger_write+0xce/0x160
   vfs_write+0x1cf/0xde0
   ksys_write+0xfd/0x200
   do_syscall_64+0xda/0x4b0

Cc: stable@vger.kernel.org
Fixes: 02205a6752f2 ("tracing: Add support for 'field variables'")
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Assisted-by: Claude:claude-fable-5 [checkpatch]
---
QEMU, x86_64, CONFIG_KASAN=y, base 7.3.0-rc2-00020-g815e07c8fe88.

Eleven key kinds, one harness, both arms. Unpatched, common_cpu, common_comm,
common_timestamp, common_timestamp.usecs, common_stacktrace, hitcount and a
constant key each oops; pid, pid.log2, pid.buckets=10 and pid.buckets=64
survive and mirror as keys=pid. Patched, every one of the eleven generates a
histogram keyed exactly as the one it mirrors.

ftracetest test.d/trigger/: 37 passed, 5 failed, 0 unresolved, per-test
verdicts identical between the arms, "test field variable support" passing.
All five failures reproduce on the unpatched base.
 kernel/trace/trace_events_hist.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 8af97fd4ee2d..257f5012a4fa 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1725,7 +1725,7 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
 		flags_str = "percent";
 	else if (hist_field->flags & HIST_FIELD_FL_GRAPH)
 		flags_str = "graph";
-	else if (hist_field->flags & HIST_FIELD_FL_STACKTRACE)
+	else if (hist_field->flags & HIST_FIELD_FL_STACKTRACE && hist_field->field)
 		flags_str = "stacktrace";
 
 	return flags_str;
@@ -1754,6 +1754,9 @@ static bool expr_field_str(struct hist_field *field, struct seq_buf *s)
 			seq_buf_printf(s, ".%s", flags_str);
 	}
 
+	if (field->buckets)
+		seq_buf_printf(s, "=%ld", field->buckets);
+
 	return !seq_buf_has_overflowed(s);
 }
 
@@ -3090,7 +3093,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
 		key_field = hist_data->fields[i];
 		if (!first)
 			seq_buf_putc(&s, ',');
-		seq_buf_puts(&s, key_field->field->name);
+		expr_field_str(key_field, &s);
 		first = false;
 	}
 
-- 
2.53.0

Re: [PATCH] tracing: Fix NULL dereference when copying keys for a field variable

[Steven Rostedt]

On Sat, 12 Sep 2026 18:47:22 +0900
Donggeun Yoo <donggeunyoo.kernel@gmail.com> wrote:

> create_field_var_hist() builds a hist trigger on the onmatch() event by
> copying the key list of the compatible histogram found there, reading
> each name from key_field->field->name. That pointer is NULL when the key
> is not an event field: parse_field() leaves it NULL for common_cpu,
> common_comm, common_timestamp, common_stacktrace and hitcount.
> compatible_keys() compares only the type, size and signedness of each
> key, so two common_cpu keys match and the loop faults.
> 
> The generated trigger is a real histogram whose element the variable is
> later read out of, so it has to bucket the same way as the one it
> mirrors. Render the keys with expr_field_str(), which carries .log2 and
> .usecs, modifiers that change a key's value. Two things it did not carry
> are added here. get_hist_field_flags() reports .stacktrace for the
> common_stacktrace pseudo-field too, which parse_field() takes only on a
> real field, so gate it on having one. And it reports a bare "buckets"
> with the size held separately in hist_field->buckets, so append that the
> way hist_field_print() does. expr_field_str() also renders expression
> operands, so an expression over a bucketed field now prints the size
> too, matching what hist_field_print() already shows for the key.

The above change log is extremely hard to read. Did you get it directly
from AI? That tends to be overly verbose and adds way more data than
needed making it harder to find the important parts. Please make the
change logs more precise to what the issue is and remove the unneeded
details.

For this patch, I'm currently testing it, but let me try to decipher it
(but please clean up future patches)...

OK, I gave up on the change log as it's just garbage. I ended up
triggering the bug mentioned and looking at the code and figured it out
myself.

OK, this patch does three things so it really needs to be three
different patches. One, it fixes the reported bug by the change to call
expr_field_str(). The change log for that bug should be:

   In event_hist_trigger_parse() where it needs to create actions like
   "onmatch", it calls:

     event_hist_trigger_parse() {
       create_actions() {
         action_create() {
           trace_action_create() {
             trace_action_create_field_var() {
               create_field_var_hist()

    Where create_field_var_hist() does a loop on the hist_data
    representing the keys. The issue is, if the keys uses one of the
    pseudo field types (like common_cpu), the hist_data field element
    will have NULL for its field member causing a NULL pointer
    dereference when accessing the key_field->field->name.

    Instead of accessing it directly, use the proper handler
    expr_field_str() to get the name.

> @@ -3090,7 +3093,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
>  		key_field = hist_data->fields[i];
>  		if (!first)
>  			seq_buf_putc(&s, ',');
> -		seq_buf_puts(&s, key_field->field->name);
> +		expr_field_str(key_field, &s);
>  		first = false;
>  	}

The changes and the change log for the other two is confusing and it
doesn't show examples of the errors. They can be dropped from the NULL
pointer dereference fix and sent separately with examples of what is
actually wrong.

Oh, and there's another bug here. With your example, where you used
"prio" for sched_switch (which isn't a field), there's an error message
created, but the command still errors out.

[  415.300003] hist:sched:sched_switch: error: Couldn't parse field variable
  Command: hist:keys=stacktrace:wakeup_lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).my_synth($wakeup_lat,prio)

-- Steve

Re: [PATCH] tracing: Fix NULL dereference when copying keys for a field variable

[Steven Rostedt]

On Sun, 13 Sep 2026 12:25:23 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Oh, and there's another bug here. With your example, where you used
> "prio" for sched_switch (which isn't a field), there's an error message
> created, but the command still errors out.

I meant the command still succeeds, it doesn't error out.

-- Steve

> 
> [  415.300003] hist:sched:sched_switch: error: Couldn't parse field variable
>   Command: hist:keys=stacktrace:wakeup_lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).my_synth($wakeup_lat,prio)