2023-12-07 14:09:49

by Ben Gainey

Subject: [PATCH] tools/perf: Fix missing reference count get in call_path_from_sample

The addr_location map and maps fields in the inner loop were missing
calls to map__get/maps__get. The subsequent addr_location__exit call in
each loop puts the map/maps fields causing use-after-free aborts.

This issue reproduces on at least arm64 and x86_64 with something
simple like `perf record -g ls` followed by `perf script -s script.py`
with the following script:

perf_db_export_mode = True
perf_db_export_calls = False
perf_db_export_callchains = True

def sample_table(*args):
print(f'sample_table({args})')

def call_path_table(*args):
print(f'call_path_table({args}')

Fixes: 0dd5041c9a0ea ("perf addr_location: Add init/exit/copy functions")
Signed-off-by: Ben Gainey <[email protected]>
---
tools/perf/util/db-export.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
index b9fb71ab7a73..106429155c2e 100644
--- a/tools/perf/util/db-export.c
+++ b/tools/perf/util/db-export.c
@@ -253,8 +253,8 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
*/
addr_location__init(&al);
al.sym = node->ms.sym;
- al.map = node->ms.map;
- al.maps = thread__maps(thread);
+ al.map = map__get(node->ms.map);
+ al.maps = maps__get(thread__maps(thread));
al.addr = node->ip;

if (al.map && !al.sym)
--
2.43.0

2023-12-07 17:33:35

by Ian Rogers

Subject: Re: [PATCH] tools/perf: Fix missing reference count get in call_path_from_sample

On Thu, Dec 7, 2023 at 6:09 AM Ben Gainey <[email protected]> wrote:
>
> The addr_location map and maps fields in the inner loop were missing
> calls to map__get/maps__get. The subsequent addr_location__exit call in
> each loop puts the map/maps fields causing use-after-free aborts.
>
> This issue reproduces on at least arm64 and x86_64 with something
> simple like `perf record -g ls` followed by `perf script -s script.py`
> with the following script:
>
> perf_db_export_mode = True
> perf_db_export_calls = False
> perf_db_export_callchains = True
>
> def sample_table(*args):
> print(f'sample_table({args})')
>
> def call_path_table(*args):
> print(f'call_path_table({args}')

Thanks for this, I've had a go at turning this into a perf test and
will send it out shortly.

> Fixes: 0dd5041c9a0ea ("perf addr_location: Add init/exit/copy functions")
> Signed-off-by: Ben Gainey <[email protected]>

Tested-by: Ian Rogers <[email protected]>

Thanks,
Ian

> ---
> tools/perf/util/db-export.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index b9fb71ab7a73..106429155c2e 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -253,8 +253,8 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
> */
> addr_location__init(&al);
> al.sym = node->ms.sym;
> - al.map = node->ms.map;
> - al.maps = thread__maps(thread);
> + al.map = map__get(node->ms.map);
> + al.maps = maps__get(thread__maps(thread));
> al.addr = node->ip;
>
> if (al.map && !al.sym)
> --
> 2.43.0
>