Re: [PATCH v3 1/6] perf arm_spe: Make a function to get the MIDR

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Missing NULL pointer check on the result of `perf_env__cpuid()` leads to a potential segmentation fault in `strtol()`.
--

commit 6dbdbd9b51f99a0d48fd13fa50acc79faa59bbb8
Author: James Clark <james.clark@linaro.org>

perf arm_spe: Make a function to get the MIDR

This commit extracts a function to get the MIDR, which will be needed to
dump IMPDEF events in later commits. No functional changes are intended.

> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
[ ... ]
> @@ -987,15 +982,28 @@ static void arm_spe__synth_ds(struct arm_spe_queue *speq,
>  
>  		pr_warning_once("Old SPE metadata, re-record to improve decode accuracy\n");
>  		cpuid = perf_env__cpuid(perf_session__env(spe->session));
> -		midr = strtol(cpuid, NULL, 16);
> -	} else {
> -		metadata = arm_spe__get_metadata_by_cpu(spe, speq->cpu);
> -		if (!metadata)
> -			return;
> -
> -		midr = metadata[ARM_SPE_CPU_MIDR];
> +		*midr = strtol(cpuid, NULL, 16);

Could this cause a segmentation fault if perf_env__cpuid() returns NULL?

If reading the CPU ID fails (for instance, when processing an SPE trace with
old metadata on a machine lacking the expected sysfs CPU ID files), passing
NULL to strtol() will crash the tool.

I note this behavior was present before this patch, and I see it is addressed
later in the patch series in commit be72d9e89c04e ("perf arm_spe: Handle
missing CPU IDs").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260414-james-spe-impdef-decode-v3-0-63baf9c893b1@linaro.org?part=1