[PATCH] libdwfl: Make dwfl_report_offline_memory work with ELF_C_READ_MMAP

[Mark Wielaard]

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