On Mon, 7 Sep 2026 15:03:23 +0900
Donggeun Yoo <[email protected]> wrote:
> print_entries() uses n_entries both as the number of sort entries and as
> its own return value, so the -ENOMEM it stores when the stats allocation
> fails overwrites the count that the cleanup still needs:
>
> n_entries = tracing_map_sort_entries(map, ...);
> if (n_entries < 0)
> return n_entries;
> ...
> if (!stats) {
> n_entries = -ENOMEM;
> goto out;
> }
> ...
> out:
> tracing_map_destroy_sort_entries(sort_entries, n_entries);
>
> tracing_map_destroy_sort_entries() takes an unsigned int and loops up to
> it, so -ENOMEM arrives as 4294967284. It walks an array of at most
> map->max_elts pointers and calls destroy_sort_entry(), which dereferences
> and frees, on whatever lies past the end.
>
> Reading the hist file of a trigger with a .percent value, with that
> allocation forced to fail:
>
> 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
> The buggy address belongs to a 4-page vmalloc region starting at
> ffffc90000041000 allocated at tracing_map_sort_entries+0x5c/0xd50
>
> A few pages further the fault is fatal. The registers at the oops confirm
> the bound: the loop's end pointer less the array start, over the pointer
> size, is 4294967284.
>
> Return the error in a separate variable and leave n_entries holding the
> count, the way tracing_map_sort_entries() does on its own error path.
>
> The stats block is only entered for a value carrying .percent or .graph,
> which __create_val_field() has rejected since v6.3, so this cannot be
> reached in mainline as it stands. It becomes reachable again with
> "tracing: hist: let values keep the percent and graph modifiers", so it
> should be applied first.
>
Good catch!
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Thanks,
> Fixes: abaa5258ce5e ("tracing: Add .percent suffix option to histogram
> values")
> Cc: [email protected]
> Signed-off-by: Donggeun Yoo <[email protected]>
> ---
> Reproduced under QEMU x86_64 on 1fc5a74b108f, with the percent/graph patch
> applied so the block is reachable and with the stats allocation forced to
> fail. Before this change the read of the hist file never returns: KASAN
> reports the vmalloc-out-of-bounds quoted above and the kernel then takes a
> fatal page fault. After it the read returns, the histogram prints its
> header with no rows, and KASAN is silent.
>
> Link:
> https://lore.kernel.org/linux-trace-kernel/[email protected]/
>
> kernel/trace/trace_events_hist.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace_events_hist.c
> b/kernel/trace/trace_events_hist.c
> index 963e0d6b61fd..67f324d7dc78 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -5690,7 +5690,7 @@ static int print_entries(struct seq_file *m,
> {
> struct tracing_map_sort_entry **sort_entries = NULL;
> struct tracing_map *map = hist_data->map;
> - int i, j, n_entries;
> + int i, j, n_entries, ret;
> struct hist_val_stat *stats = NULL;
> u64 val;
>
> @@ -5700,6 +5700,8 @@ static int print_entries(struct seq_file *m,
> if (n_entries < 0)
> return n_entries;
>
> + ret = n_entries;
> +
> /* Calculate the max and the total for each field if needed. */
> for (j = 0; j < hist_data->n_vals; j++) {
> if (!(hist_data->fields[j]->flags &
> @@ -5708,7 +5710,7 @@ static int print_entries(struct seq_file *m,
> if (!stats) {
> stats = kzalloc_objs(*stats, hist_data->n_vals);
> if (!stats) {
> - n_entries = -ENOMEM;
> + ret = -ENOMEM;
> goto out;
> }
> }
> @@ -5729,7 +5731,7 @@ static int print_entries(struct seq_file *m,
> out:
> tracing_map_destroy_sort_entries(sort_entries, n_entries);
>
> - return n_entries;
> + return ret;
> }
>
> static void hist_trigger_show(struct seq_file *m,
> --
> 2.53.0
>
--
Masami Hiramatsu (Google) <[email protected]>
