Hi Aleksei,
On Thu, 2024-07-11 at 20:35 +0000, Aleksei Vetrov wrote:
> elf_memory open mode recently changed from ELF_C_READ to
> ELF_C_READ_MMAP.
That was this commit:
commit cc44ac6740797a23cd0af0cb22bd828d569224b8
Author: Mark Wielaard <mark@klomp.org>
Date: Thu Feb 1 14:56:18 2024 +0100
libelf: Treat elf_memory as if using ELF_C_READ_MMAP
An Elf handle created through elf_memory was treated as if opened with
ELF_C_READ. Which means libelf believed it had read the memory itself
and could simply write to it if it wanted (because it wasn't mmaped
directly on top of a file). This causes issues when that memory was
actually read-only. Work around this by pretending the memory was
actually read with ELF_C_READ_MMAP (so directly readable, but not
writable).
Add extra tests to elfgetzdata to check using elf_memory with
read-only memory works as expected.
* libelf/elf_memory.c (elf_memory): Call
__libelf_read_mmaped_file with ELF_C_READ_MMAP.
* tests/elfgetzdata.c (main): Add new "mem" option.
* tests/run-elfgetzdata.sh: Also run all tests with new
"mem" option.
https://sourceware.org/bugzilla/show_bug.cgi?id=31225
Reported-by: Derek Bruening <bruening@google.com>
Signed-off-by: Mark Wielaard <mark@klomp.org>
> This broken dwfl_report_offline_memory that changes
> mode to ELF_C_READ_MMAP_PRIVATE to be compatible with subsequent
> elf_begin on embedded ELF files.
Aha, right, originally, before that patch, the cmd had to be changed to
ELF_C_READ_MMAP or ELF_C_READ_MMAP_PRIVATE because elf_memory didn't
before, but not elf_memory picks MMAP, so using MMAP_PRIVATE conflicts.
> The proper implementation of dwfl_report_offline_memory doesn't change
> open mode and subsequent elf_begin invocations simply use cmd from the
> reference Elf*.
Right, in hindsight, if elf_memory had set cmd itself correctly from
the start this is the correct thing to do.
> Add tests to exercise Elf* to trigger the bug caused by incorrect cmd
> set to Elf*.
Thanks, checked that it fails before with an assert in getshdr and
succeeds after the patch.
> * libdwfl/offline.c (process_archive): Use archive->cmd
> instead of hardcoded ELF_C_READ_MMAP_PRIVATE.
> * libdwfl/open.c (libdw_open_elf): Use elf->cmd instead of
> hardcoded ELF_C_READ_MMAP_PRIVATE.
> (__libdw_open_elf_memory): Don't override (*elfp)->cmd.
> * tests/Makefile.am (dwfl_report_offline_memory): Add libelf
> as dependency.
> * tests/dwfl-report-offline-memory.c: Add count_sections to
> exercise Elf* from dwfl_report_offline_memory.
> * tests/run-dwfl-report-offline-memory.sh: Add expected number
> of sections to test invocations.
Looks obviously correct (in hindsight).
Pushed.
Thanks,
Mark
@@ -32,6 +32,7 @@
# include <config.h>
#endif
+#include "libelfP.h"
#include "libdwflP.h"
#include <fcntl.h>
@@ -254,7 +255,7 @@ process_archive (Dwfl *dwfl, const char *name, const char *file_name, int fd,
{
Dwfl_Module *mod = NULL;
/* elf_begin supports opening archives even with fd == -1 passed. */
- Elf *member = elf_begin (fd, ELF_C_READ_MMAP_PRIVATE, archive);
+ Elf *member = elf_begin (fd, archive->cmd, archive);
if (unlikely (member == NULL)) /* Empty archive. */
{
__libdwfl_seterrno (DWFL_E_BADELF);
@@ -263,7 +264,7 @@ process_archive (Dwfl *dwfl, const char *name, const char *file_name, int fd,
while (process_archive_member (dwfl, name, file_name, predicate,
fd, member, &mod) != ELF_C_NULL)
- member = elf_begin (fd, ELF_C_READ_MMAP_PRIVATE, archive);
+ member = elf_begin (fd, archive->cmd, archive);
/* We can drop the archive Elf handle even if we're still using members
in live modules. When the last module's elf_end on a member returns
@@ -151,7 +151,7 @@ libdw_open_elf (int *fdp, Elf **elfp, bool close_on_fail, bool archive_ok,
elf->state.ar.elf_ar_hdr.ar_name = "libdwfl is faking you out";
elf->state.ar.elf_ar_hdr.ar_size = elf->maximum_size - offset;
elf->state.ar.offset = offset - sizeof (struct ar_hdr);
- Elf *subelf = elf_begin (-1, ELF_C_READ_MMAP_PRIVATE, elf);
+ Elf *subelf = elf_begin (-1, elf->cmd, elf);
elf->kind = ELF_K_NONE;
if (unlikely (subelf == NULL))
error = DWFL_E_LIBELF;
@@ -215,8 +215,6 @@ __libdw_open_elf_memory (char *data, size_t size, Elf **elfp, bool archive_ok)
{
return DWFL_E_LIBELF;
}
- /* Allow using this ELF as reference for subsequent elf_begin calls. */
- (*elfp)->cmd = ELF_C_READ_MMAP_PRIVATE;
return libdw_open_elf (&fd, elfp, false, archive_ok, true, false, true);
}
@@ -792,7 +792,7 @@ test_elf_cntl_gelf_getshdr_LDADD = $(libelf)
dwflsyms_LDADD = $(libdw) $(libelf) $(argp_LDADD)
dwfllines_LDADD = $(libeu) $(libdw) $(libelf) $(argp_LDADD)
dwfl_report_elf_align_LDADD = $(libeu) $(libdw)
-dwfl_report_offline_memory_LDADD = $(libeu) $(libdw)
+dwfl_report_offline_memory_LDADD = $(libeu) $(libdw) $(libelf)
dwfl_report_segment_contiguous_LDADD = $(libdw) $(libebl) $(libelf)
varlocs_LDADD = $(libeu) $(libdw) $(libelf) $(argp_LDADD)
backtrace_LDADD = $(libeu) $(libdw) $(libelf) $(argp_LDADD)
@@ -18,14 +18,19 @@
#include <config.h>
#include <assert.h>
+#include <errno.h>
+#include <error.h>
#include <fcntl.h>
#include <locale.h>
#include <stdio.h>
#include <stdio_ext.h>
#include <stdlib.h>
+#include <string.h>
#include <unistd.h>
+
#include ELFUTILS_HEADER(dwfl)
-#include "system.h"
+#include ELFUTILS_HEADER(elf)
+#include <gelf.h>
static const Dwfl_Callbacks offline_callbacks =
@@ -45,6 +50,20 @@ count_modules (Dwfl_Module *mod __attribute__ ((unused)),
return DWARF_CB_OK;
}
+static int
+count_sections (Elf *elf)
+{
+ int result = 0;
+ Elf_Scn *section = NULL;
+ GElf_Shdr header;
+ while ((section = elf_nextscn (elf, section)) != NULL)
+ {
+ assert (gelf_getshdr (section, &header) != NULL);
+ result += 1;
+ }
+ return result;
+}
+
int
main (int argc, char **argv)
{
@@ -54,10 +73,11 @@ main (int argc, char **argv)
/* Set locale. */
(void) setlocale (LC_ALL, "");
- if (argc != 3)
+ if (argc != 4)
error (-1, 0,
"usage: dwfl_report_offline_memory [filename] "
- "[expected number of modules]");
+ "[expected number of modules] "
+ "[expected number of sections]");
const char *fname = argv[1];
int fd = open (fname, O_RDONLY);
@@ -100,6 +120,12 @@ main (int argc, char **argv)
assert (endptr && !*endptr);
assert (number_of_modules == expected_number_of_modules);
+ GElf_Addr loadbase = 0;
+ Elf *elf = dwfl_module_getelf (mod, &loadbase);
+ int number_of_sections = count_sections (elf);
+ int expected_number_of_sections = atoi (argv[3]);
+ assert (number_of_sections == expected_number_of_sections);
+
dwfl_end (dwfl);
free (data);
@@ -26,8 +26,8 @@ testfiles testarchive64.a
# bzip2 -zf test-ar-duplicates.a
testfiles test-ar-duplicates.a
-testrun ${abs_builddir}/dwfl-report-offline-memory ./testfile-dwfl-report-elf-align-shlib.so 1
-testrun ${abs_builddir}/dwfl-report-offline-memory ./testarchive64.a 3
-testrun ${abs_builddir}/dwfl-report-offline-memory ./test-ar-duplicates.a 1
+testrun ${abs_builddir}/dwfl-report-offline-memory ./testfile-dwfl-report-elf-align-shlib.so 1 24
+testrun ${abs_builddir}/dwfl-report-offline-memory ./testarchive64.a 3 10
+testrun ${abs_builddir}/dwfl-report-offline-memory ./test-ar-duplicates.a 1 7
exit 0