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 <[email protected]>
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/[email protected]?part=1

Reply via email to