Re: [PATCH] perf: arm-spe: Fix null-ptr-deref in arm_spe__alloc_metadata()

From: James Clark
Date: Fri Dec 20 2024 - 12:49:35 EST




On 20/12/2024 2:57 pm, Nihar Chaithanya wrote:
When metadata is allocated using arm_spe__alloc_metadata(), if the
metadata version is 1, metadata is returned as NULL. This value
is dereferenced later in arm_spe__free_metadata() and it can cause
null-ptr-deref.

Modify the NULL check for metadata to return -EINVAL even when
metadata_ver == 1.


Hi Nihar,

I don't think this is the right fix. Doesn't that mean we can't open files with V1 anymore? Did you test opening an old SPE file on a new version of Perf? I actually thought I tested this before but maybe not if there's a NULL deref.

Seems like the correct fix is to not dereference anything if metadata is NULL, or make a fake placeholder one.


This issue was reported by Coverity scan [1].
[1] https://scan5.scan.coverity.com/#/project-view/63616/10063?selectedIssue=1636359


Is this supposed to be publicly accessible? It goes to a login page for me.

Closes: https://scan5.scan.coverity.com/#/project-view/63616/10063?selectedIssue=1636359
Signed-off-by: Nihar Chaithanya <niharchaithanya@xxxxxxxxx>

Probably needs a fixes: tag too for the commit that introduced the issue.

Thanks
James

---
tools/perf/util/arm-spe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index dbf13f47879c..55827d8ce133 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -1497,7 +1497,7 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
metadata = arm_spe__alloc_metadata(auxtrace_info, &metadata_ver,
&nr_cpu);
- if (!metadata && metadata_ver != 1) {
+ if (!metadata) {
pr_err("Failed to parse Arm SPE metadata.\n");
return -EINVAL;
}