[PATCH v1] perf test cpumap: Avoid use-after-free following merge

[PATCH v1] perf test cpumap: Avoid use-after-free following merge

[Ian Rogers]

Previously cpu maps in the test weren't modified by calls to the cpu
map API, however, perf_cpu_map__merge was modified so the left hand
argument was updated. In the test this meant the maps copy of the
"two" map was put/deleted in the merge meaning when accessed via maps,
the pointer was stale and to the put/deleted memory. To fix this add
an extra layer of indirection to the maps array, so the updated value
of two is accessed.

Fixes: a9d2217556f7 ("libperf cpumap: Refactor perf_cpu_map__merge()")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/cpumap.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/cpumap.c b/tools/perf/tests/cpumap.c
index 5ed7ff072ea3..2354246afc5a 100644
--- a/tools/perf/tests/cpumap.c
+++ b/tools/perf/tests/cpumap.c
@@ -252,16 +252,16 @@ static int test__cpu_map_equal(struct test_suite *test __maybe_unused, int subte
 	struct perf_cpu_map *empty = perf_cpu_map__intersect(one, two);
 	struct perf_cpu_map *pair = perf_cpu_map__new("1-2");
 	struct perf_cpu_map *tmp;
-	struct perf_cpu_map *maps[] = {empty, any, one, two, pair};
+	struct perf_cpu_map **maps[] = {&empty, &any, &one, &two, &pair};
 
 	for (size_t i = 0; i < ARRAY_SIZE(maps); i++) {
 		/* Maps equal themself. */
-		TEST_ASSERT_VAL("equal", perf_cpu_map__equal(maps[i], maps[i]));
+		TEST_ASSERT_VAL("equal", perf_cpu_map__equal(*maps[i], *maps[i]));
 		for (size_t j = 0; j < ARRAY_SIZE(maps); j++) {
 			/* Maps dont't equal each other. */
 			if (i == j)
 				continue;
-			TEST_ASSERT_VAL("not equal", !perf_cpu_map__equal(maps[i], maps[j]));
+			TEST_ASSERT_VAL("not equal", !perf_cpu_map__equal(*maps[i], *maps[j]));
 		}
 	}
 
@@ -274,7 +274,7 @@ static int test__cpu_map_equal(struct test_suite *test __maybe_unused, int subte
 	perf_cpu_map__put(tmp);
 
 	for (size_t i = 0; i < ARRAY_SIZE(maps); i++)
-		perf_cpu_map__put(maps[i]);
+		perf_cpu_map__put(*maps[i]);
 
 	return TEST_OK;
 }
-- 
2.47.1.613.gc27f4b7a9f-goog

Re: [PATCH v1] perf test cpumap: Avoid use-after-free following merge

[James Clark]

On 08/01/2025 5:15 am, Ian Rogers wrote:
> Previously cpu maps in the test weren't modified by calls to the cpu
> map API, however, perf_cpu_map__merge was modified so the left hand
> argument was updated. In the test this meant the maps copy of the
> "two" map was put/deleted in the merge meaning when accessed via maps,
> the pointer was stale and to the put/deleted memory. To fix this add
> an extra layer of indirection to the maps array, so the updated value
> of two is accessed.
> 
> Fixes: a9d2217556f7 ("libperf cpumap: Refactor perf_cpu_map__merge()")
> Signed-off-by: Ian Rogers <irogers@google.com>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
>   tools/perf/tests/cpumap.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/tests/cpumap.c b/tools/perf/tests/cpumap.c
> index 5ed7ff072ea3..2354246afc5a 100644
> --- a/tools/perf/tests/cpumap.c
> +++ b/tools/perf/tests/cpumap.c
> @@ -252,16 +252,16 @@ static int test__cpu_map_equal(struct test_suite *test __maybe_unused, int subte
>   	struct perf_cpu_map *empty = perf_cpu_map__intersect(one, two);
>   	struct perf_cpu_map *pair = perf_cpu_map__new("1-2");
>   	struct perf_cpu_map *tmp;
> -	struct perf_cpu_map *maps[] = {empty, any, one, two, pair};
> +	struct perf_cpu_map **maps[] = {&empty, &any, &one, &two, &pair};
>   
>   	for (size_t i = 0; i < ARRAY_SIZE(maps); i++) {
>   		/* Maps equal themself. */
> -		TEST_ASSERT_VAL("equal", perf_cpu_map__equal(maps[i], maps[i]));
> +		TEST_ASSERT_VAL("equal", perf_cpu_map__equal(*maps[i], *maps[i]));
>   		for (size_t j = 0; j < ARRAY_SIZE(maps); j++) {
>   			/* Maps dont't equal each other. */
>   			if (i == j)
>   				continue;
> -			TEST_ASSERT_VAL("not equal", !perf_cpu_map__equal(maps[i], maps[j]));
> +			TEST_ASSERT_VAL("not equal", !perf_cpu_map__equal(*maps[i], *maps[j]));
>   		}
>   	}
>   
> @@ -274,7 +274,7 @@ static int test__cpu_map_equal(struct test_suite *test __maybe_unused, int subte
>   	perf_cpu_map__put(tmp);
>   
>   	for (size_t i = 0; i < ARRAY_SIZE(maps); i++)
> -		perf_cpu_map__put(maps[i]);
> +		perf_cpu_map__put(*maps[i]);
>   
>   	return TEST_OK;
>   }

Re: [PATCH v1] perf test cpumap: Avoid use-after-free following merge

[Arnaldo Carvalho de Melo]

On Wed, Jan 08, 2025 at 03:07:53PM +0000, James Clark wrote:
> 
> 
> On 08/01/2025 5:15 am, Ian Rogers wrote:
> > Previously cpu maps in the test weren't modified by calls to the cpu
> > map API, however, perf_cpu_map__merge was modified so the left hand
> > argument was updated. In the test this meant the maps copy of the
> > "two" map was put/deleted in the merge meaning when accessed via maps,
> > the pointer was stale and to the put/deleted memory. To fix this add
> > an extra layer of indirection to the maps array, so the updated value
> > of two is accessed.
> > 
> > Fixes: a9d2217556f7 ("libperf cpumap: Refactor perf_cpu_map__merge()")
> > Signed-off-by: Ian Rogers <irogers@google.com>
> 
> Reviewed-by: James Clark <james.clark@linaro.org>

Thanks, applied to perf-tools-next,

- Arnaldo