[PATCH] tracing: hist: let values keep the percent and graph modifiers

[PATCH] tracing: hist: let values keep the percent and graph modifiers

[Donggeun Yoo]

The .percent and .graph modifiers exist only for histogram values, but a
value carrying either of them has been rejected since v6.3. The example
in Documentation/trace/histogram.rst,

  # echo 'hist:keys=prev_comm:vals=hitcount.percent:nohitcount' > \
	 events/sched/sched_switch/trigger

returns -EINVAL.

parse_field() sets the two flags only when the field is neither a key nor
a variable, that is, only on a value:

	} else if (strncmp(modifier, "percent", 7) == 0) {
		if (*flags & (HIST_FIELD_FL_VAR | HIST_FIELD_FL_KEY))
			goto error;
		*flags |= HIST_FIELD_FL_PERCENT;

__create_val_field() then rejects a value for carrying them, so no field
can reach hist_trigger_print_val(), where both are implemented.

commit e0213434fe3e ("tracing: Do not let histogram values have some
modifiers") added the check after a value with .buckets oopsed in
hist_field_name(). That happens because .buckets and .log2 make
create_hist_field() build a nested field in operands[0] which
hist_field_name() then walks into. The percent and graph flags do not
create an operand and are not read by hist_field_name(); they are only
used when printing a value.

Stop rejecting the two flags on a value. The check for variables is left
alone, where they are unreachable anyway because parse_field() rejects a
variable carrying them first.

With the two flags removed, the trigger above installs and prints as
documented:

  { prev_comm: rcu_preempt  }  hitcount (%):   0.00
  { prev_comm: init         }  hitcount (%):  99.98
  Totals:
      Hits: 237896

Fixes: e0213434fe3e ("tracing: Do not let histogram values have some modifiers")
Cc: stable@vger.kernel.org
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
Tested under QEMU x86_64 on 1fc5a74b108f. Before the change all three of

  vals=hitcount.percent
  vals=hitcount.graph
  vals=hitcount.percent:nohitcount

are rejected with -EINVAL; after it they install and print. A plain
'keys=prev_comm' trigger works on both, as a check that the test itself is
sound. tools/testing/selftests/ftrace trigger tests are unchanged, all 45
items identical before and after: 32 pass, 3 fail, 2 unresolved, 8
unsupported, with the failures also present on an unpatched kernel.

Nothing covers these two modifiers in selftests today, which is why the
regression stood since v6.3. I can follow up with a test case.

 kernel/trace/trace_events_hist.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 963e0d6b61fd..faaff7b315f6 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4313,8 +4313,7 @@ static int __create_val_field(struct hist_trigger_data *hist_data,
 			goto err;
 	} else {
 		/* Value */
-		if (hist_field->flags & (HIST_FIELD_FL_GRAPH | HIST_FIELD_FL_PERCENT |
-					 HIST_FIELD_FL_BUCKET | HIST_FIELD_FL_LOG2 |
+		if (hist_field->flags & (HIST_FIELD_FL_BUCKET | HIST_FIELD_FL_LOG2 |
 					 HIST_FIELD_FL_SYM | HIST_FIELD_FL_SYM_OFFSET |
 					 HIST_FIELD_FL_SYSCALL | HIST_FIELD_FL_STACKTRACE))
 			goto err;
-- 
2.53.0

Re: [PATCH] tracing: hist: let values keep the percent and graph modifiers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Out-of-bounds loop and invalid pointer free in tracing_map_destroy_sort_entries when stats allocation fails.
--

commit 0ec7f4c23d8ecafd15abb3d5f3ff02ac2abded8a
Author: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
tracing: hist: let values keep the percent and graph modifiers

This commit stops rejecting the .percent and .graph modifiers on histogram
values, fixing a regression introduced in v6.3. It allows these modifiers
to be parsed and printed correctly when triggered.

> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 893bd8b0e48a8..6cb595cbcdafe 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -4313,8 +4313,7 @@ static int __create_val_field(struct hist_trigger_data *hist_data,
>  			goto err;
>  	} else {
>  		/* Value */
> -		if (hist_field->flags & (HIST_FIELD_FL_GRAPH | HIST_FIELD_FL_PERCENT |
> -					 HIST_FIELD_FL_BUCKET | HIST_FIELD_FL_LOG2 |
> +		if (hist_field->flags & (HIST_FIELD_FL_BUCKET | HIST_FIELD_FL_LOG2 |
>  					 HIST_FIELD_FL_SYM | HIST_FIELD_FL_SYM_OFFSET |
>  					 HIST_FIELD_FL_SYSCALL | HIST_FIELD_FL_STACKTRACE))
>  			goto err;

[Severity: Critical]
This is a pre-existing issue, but by allowing these modifiers to pass
through, does this expose an out-of-bounds loop in print_entries()?

If kzalloc_objs() fails when allocating stats for the percent or graph
modifiers, it sets n_entries to -ENOMEM:

kernel/trace/trace_events_hist.c:print_entries() {
    ...
		if (!stats) {
			stats = kzalloc_objs(*stats, hist_data->n_vals);
			if (!stats) {
				n_entries = -ENOMEM;
				goto out;
			}
		}
    ...
}

It then jumps to the out label where it passes this negative value to
tracing_map_destroy_sort_entries():

out:
	tracing_map_destroy_sort_entries(sort_entries, n_entries);

However, tracing_map_destroy_sort_entries() takes an unsigned int for
n_entries:

kernel/trace/tracing_map.c:tracing_map_destroy_sort_entries() {
void tracing_map_destroy_sort_entries(struct tracing_map_sort_entry **entries,
                                      unsigned int n_entries)
{
	unsigned int i;

	for (i = 0; i < n_entries; i++)
		destroy_sort_entry(entries[i]);
    ...
}

Will passing -ENOMEM as n_entries result in a ~4 billion iteration loop
and an out-of-bounds memory access?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907052113.430818-1-donggeunyoo.kernel@gmail.com?part=1

Re: [PATCH] tracing: hist: let values keep the percent and graph modifiers

[Donggeun Yoo]

On Mon, Sep 07, 2026 at 05:31:12AM +0000, sashiko-bot@kernel.org wrote:
> [Critical] Out-of-bounds loop and invalid pointer free in
> tracing_map_destroy_sort_entries when stats allocation fails.

Confirmed, including the reachability the report points at: this patch is
what makes that path live again.

Reproduced with the stats allocation forced to fail, reading the hist file
of a trigger with a .percent value:

  BUG: KASAN: vmalloc-out-of-bounds in tracing_map_destroy_sort_entries+0xa0/0xb0
  Read of size 8 at addr ffffc90000045000 by task init/1
   tracing_map_destroy_sort_entries+0xa0/0xb0
   hist_show+0x6f7/0x1df0
   seq_read_iter+0x2b8/0x1190
   vfs_read+0x176/0xa40

followed by a fatal page fault a few pages further. The registers at the
oops give the bound: the loop's end pointer less the array start, over the
pointer size, is 4294967284, which is -ENOMEM as an unsigned int.

Fix sent, with Cc: stable and a note that it should be applied before this
one:

  https://lore.kernel.org/linux-trace-kernel/20260907060323.480728-1-donggeunyoo.kernel@gmail.com/