[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240826131341.GA4167@debian-dev>
Date: Mon, 26 Aug 2024 21:13:41 +0800
From: Leo Yan <leo.yan@...ux.dev>
To: Steve Clevenger <scclevenger@...amperecomputing.com>
Cc: james.clark@...aro.org, mike.leach@...aro.org, suzuki.poulose@....com,
leo.yan@....com, ilkka@...ampercomputing.com,
coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] Add dso__is_pie call to identify ELF PIE
Hi Steve,
On Tue, Aug 20, 2024 at 04:11:35PM -0600, Steve Clevenger wrote:
> From: "steve.c.clevenger.ampere" <scclevenger@...amperecomputing.com>
>
> Add dso__is_pie global to read the .dynamic section DT_FLAGS_1 entry for
> the DF_1_PIE flag. This identifies position executable code.
>
> Signed-off-by: steve.c.clevenger.ampere <scclevenger@...amperecomputing.com>
> ---
> tools/perf/util/symbol-elf.c | 55 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index e398abfd13a0..1d4bd222b314 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -662,6 +662,61 @@ static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,
> return err;
> }
>
> +/*
> + * Check dynamic section DT_FLAGS_1 for a Position Independent
> + * Executable (PIE).
> + */
> +bool dso__is_pie(struct dso *dso)
> +{
> + Elf *elf = NULL;
> + Elf_Scn *scn = NULL;
> + GElf_Ehdr ehdr;
> + GElf_Shdr shdr;
> + bool is_pie = false;
> + char dso_path[PATH_MAX];
> + int fd = -1;
> +
> + if (!dso || (elf_version(EV_CURRENT) == EV_NONE))
> + return is_pie; // false
> +
> + dso__build_id_filename(dso, dso_path, sizeof(dso_path), false);
> +
> + fd = open(dso_path, O_RDONLY);
> +
> + if (fd < 0) {
> + pr_debug("%s: cannot read cached %s.\n", __func__, dso_path);
> + return is_pie; // false
> + }
> +
> + elf = elf_begin(fd, ELF_C_READ, NULL);
> + gelf_getehdr(elf, &ehdr);
> +
> + if (ehdr.e_type == ET_DYN) {
The code looks good to me, just several nitpicks.
To avoid indentation, we can firstly check the failure case and directly
exit for it.
if (ehdr.e_type != ET_DYN)
goto exit_elf_end;
> + scn = elf_section_by_name(elf, &ehdr, &shdr, ".dynamic", NULL);
Ditto.
if (!scn)
goto exit_elf_end;
> + if (scn) { // check DT_FLAGS_1
> + Elf_Data *data;
> + GElf_Dyn *entry;
> + int n_entries = shdr.sh_size / sizeof(GElf_Dyn);
> +
> + data = (Elf_Data *) elf_getdata(scn, NULL);
For a safe code, it is good to check if pointers (data and
data->d_buf) are valid before dereference them.
if (!data || !data->d_buf)
goto exit_elf_end;
With above changes:
Reviewed-by: Leo Yan <leo.yan@....com>
> + for (int i = 0; i < n_entries; i++) {
> + entry = ((GElf_Dyn *) data->d_buf) + i;
> + if (entry->d_tag == DT_FLAGS_1) {
> + if ((entry->d_un.d_val & DF_1_PIE) != 0) {
> + is_pie = true;
> + break;
> + }
> + }
> + } // end for
> + }
> + }
> +
> + elf_end(elf);
> + close(fd);
> +
> + return is_pie;
> +}
> +
> /*
> * We need to check if we have a .dynsym, so that we can handle the
> * .plt, synthesizing its symbols, that aren't on the symtabs (be it
> --
> 2.25.1
>
>
Powered by blists - more mailing lists