Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
MIME-Version: 1.0
References: <20231207140911.3240408-1-ben.gainey@arm.com>
In-Reply-To: <20231207140911.3240408-1-ben.gainey@arm.com>
From:   Ian Rogers <irogers@google.com>
Date:   Thu, 7 Dec 2023 09:33:10 -0800
Message-ID: <CAP-5=fVM7cdrcOKL1nRcusBZpg_6Sg5NZA6PKYWcMgeOwHjf3w@mail.gmail.com>
Subject: Re: [PATCH] tools/perf: Fix missing reference count get in call_path_from_sample
To:     Ben Gainey <ben.gainey@arm.com>
Cc:     linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
        mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
        jolsa@kernel.org, namhyung@kernel.org, adrian.hunter@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Dec 7, 2023 at 6:09=E2=80=AFAM Ben Gainey <ben.gainey@arm.com> wrot=
e:
>
> 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 =3D True
>     perf_db_export_calls =3D False
>     perf_db_export_callchains =3D 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 <ben.gainey@arm.com>

Tested-by: Ian Rogers <irogers@google.com>

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 =3D node->ms.sym;
> -               al.map =3D node->ms.map;
> -               al.maps =3D thread__maps(thread);
> +               al.map =3D map__get(node->ms.map);
> +               al.maps =3D maps__get(thread__maps(thread));
>                 al.addr =3D node->ip;
>
>                 if (al.map && !al.sym)
> --
> 2.43.0
>