Re: [PATCH 03/11] perf tools: Use perf_env__get_cpu_topology() in machine__resolve()

[Ian Rogers]

On Wed, Jun 10, 2026 at 9:52 AM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>
> machine__resolve() accesses env->cpu[al->cpu].socket_id after checking
> al->cpu >= 0 and env->cpu != NULL, but without validating al->cpu
> against env->nr_cpus_avail.  Since al->cpu comes from the untrusted
> perf.data sample, a crafted file with a large CPU index causes an
> out-of-bounds heap read.
>
> Use perf_env__get_cpu_topology() which validates both NULL and bounds.
> Also bounds-check al->cpu before the cast to struct perf_cpu (int16_t):
> without this, values like 65536 silently truncate to 0, bypassing the
> accessor's internal check and returning CPU 0's topology.

Thanks for clarifying this. Perhaps we should have a general
int-to-CPU helper, but that's out of scope here.

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> Fixes: 0c4c4debb0adda4c ("perf tools: Add processor socket info to hist_entry and addr_location")
> Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
> Cc: Kan Liang <kan.liang@xxxxxxxxx>
> Cc: Ian Rogers <irogers@xxxxxxxxxx>
> Assisted-by: Claude Opus 4.6 <noreply@xxxxxxxxxxxxx>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
>  tools/perf/util/event.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 66f4843bb235df53..ea75816d126a14be 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -14,6 +14,7 @@
>  #include <linux/perf_event.h>
>  #include "cpumap.h"
>  #include "dso.h"
> +#include "env.h"
>  #include "event.h"
>  #include "debug.h"
>  #include "hist.h"
> @@ -836,8 +837,18 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
>         if (al->cpu >= 0) {
>                 struct perf_env *env = machine->env;
>
> -               if (env && env->cpu)
> -                       al->socket = env->cpu[al->cpu].socket_id;
> +               /*
> +                * Bounds-check al->cpu (s32) before casting to struct perf_cpu
> +                * (int16_t): without this, e.g. 65536 truncates to 0 and silently
> +                * returns CPU 0's topology.  Can go once perf_cpu.cpu is widened.
> +                */
> +               if (env && al->cpu < env->nr_cpus_avail) {
> +                       struct cpu_topology_map *topo;
> +
> +                       topo = perf_env__get_cpu_topology(env, (struct perf_cpu){ al->cpu });
> +                       if (topo)
> +                               al->socket = topo->socket_id;
> +               }
>         }
>
>         /* Account for possible out-of-order switch events. */
> --
> 2.54.0
>