[PATCH] tracing: hist: keep the entry count when the stats allocation fails

[PATCH] tracing: hist: keep the entry count when the stats allocation fails

[Donggeun Yoo]

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.

Fixes: abaa5258ce5e ("tracing: Add .percent suffix option to histogram values")
Cc: stable@vger.kernel.org
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
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/20260907052113.430818-1-donggeunyoo.kernel@gmail.com/

 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

Re: [PATCH] tracing: hist: keep the entry count when the stats allocation fails

[Masami Hiramatsu]

On Mon,  7 Sep 2026 15:03:23 +0900
Donggeun Yoo <donggeunyoo.kernel@gmail.com> 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) <mhiramat@kernel.org>

Thanks,

> Fixes: abaa5258ce5e ("tracing: Add .percent suffix option to histogram values")
> Cc: stable@vger.kernel.org
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
> 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/20260907052113.430818-1-donggeunyoo.kernel@gmail.com/
> 
>  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) <mhiramat@kernel.org>

Re: [PATCH] tracing: hist: keep the entry count when the stats allocation fails

[Steven Rostedt]

On Mon,  7 Sep 2026 15:03:23 +0900
Donggeun Yoo <donggeunyoo.kernel@gmail.com> wrote:

First I want to say thank you for all you fixes you have been sending. The
histogram code needs a lot more love that it has been given ;-)

Note, I've been changing the subject lines of your patches to:

This one:

 [PATCH] tracing: Keep the entrty count when the histogram stats allocation fails

And for you other patches:

 [PATCH] tracing: Free histogram ...

As histograms are not a separate subsystem and just part of the tracing
subsystem. And all subjects should start with a capital letter.

> 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.
> 
> Fixes: abaa5258ce5e ("tracing: Add .percent suffix option to histogram values")
> Cc: stable@vger.kernel.org

Please place the Cc stable above the fixes.

I'm not sure who is suggesting that but I'm seeing a lot of patches that do
that. To me, the Cc's (even to stable) should be the first thing in the
footer portion of the change log.

> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
> 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/20260907052113.430818-1-donggeunyoo.kernel@gmail.com/

Informative links should go above the '---' and be included in the git
commit. In fact, you should always add a reported-by if something informed
you of a fix. I'll be adding to this patch:

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260907053113.1CED91F00A3A@smtp.kernel.org/

-- Steve