Hi,
On Thu, 2024-08-29 at 15:47 +0200, Mark Wielaard wrote:
> So we changed elf_memory so it pretends the in-memory Elf image is
> read with ELF_C_READ_MMAP. This helps when calling elf_memory on
> read-only memory which still wants to change some things about the Elf
> like uncompress some sections (which changes the section header).
>
> With ELF_C_READ_MMAP libelf will make a copy of the section headers,
> so any changes aren't written to the memory image (because that would
> crash if the underlying memory is read-only).
>
> But that breaks another use case where elf_memory is used on rw memory
> and then the section headers are updated using libelf and it is
> expected those in-memory section headers reflect the changes.
>
> The problem of course is the elf_memory function doesn't take a "mode"
> argument. So we have to guess. And make one or the other usage
> unusable.
>
> I think we should assume the memory is read/write and at least header
> updates are written back to the memory image. But that when only just
> reading the image then nothing is changed and written back to the
> image.
>
> This does mean that when explicitly uncompressing sections you have to
> make sure the image is writable (because that updates the shdrs).
I waited too long deciding what to do here. Sorry. We are about to make
the 0.192 release today. If it isn't too late then I would like to
apply my proposed patch (as attached).
It isn't ideal, but it basically reverts to the behavior of elfutils
0.190. So you can again do direct manipulation on ELF images created
through elf_memory. So in that way it was a regression in 0.191, fixed
for 0.192.
It does mean that if the memory you gave elf_memory is read-only and
you call elf_compress on one of the sections things might crash
(because elf_compress will try to update the Elf_Shdr structure in
place to set the new size). Which indeed could be seen as a regression
from 0.191 (but was there before). If you do want to decompress (or
otherwise manipulate) the ELF image created through elf_memory you have
to do what the testcase (now) does. mmap the image using MMAP_PRIVATE
so it is (copy-on-)writable.
Longer term we probably want a new elf_memory2 function that takes an
argument describing the protection bits of the given memory.
Aaron, is it OK to commit this before you start the 0.192 release
process?
Cheers,
Mark
From 81981fdfd8f776508579da60d29e068359c61e6a Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Fri, 18 Oct 2024 16:52:49 +0200
Subject: [PATCH] libelf: Treat elf_memory image as writable
There are use cases where the Elf image created by elf_memory is
manipulated, through the libelf interfaces, in place. This doesn't
work anymore since we changed elf_memory to assume the memory is
read-only in elfutils 0.191. commit cc44ac674 ('libelf: Treat
elf_memory as if using ELF_C_READ_MMAP').
The reason for that change was that if elf_memory was given a
read-only memory image then decompressing a section with elf_compress
would crash. Since it directly writes the updated Shdr size. If you do
want to use elf_compress on an Elf created by elf_memory you have make
sure the memory is writable. You can do this for example by using mmap
PROTE_WRITE and MAP_PRIVATE.
* libelf/elf_memory.c (elf_memory): Call
__libelf_read_mmaped_file with ELF_C_READ_MMAP_PRIVATE.
* tests/elfgetzdata.c (main): Use mmap PROT_WRITE and MAP_PRIVATE.
Signed-off-by: Mark Wielaard <m...@klomp.org>
---
libelf/elf_memory.c | 3 ++-
tests/elfgetzdata.c | 6 ++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/libelf/elf_memory.c b/libelf/elf_memory.c
index 13d77cb71b39..1df49d732dd9 100644
--- a/libelf/elf_memory.c
+++ b/libelf/elf_memory.c
@@ -46,5 +46,6 @@ elf_memory (char *image, size_t size)
return NULL;
}
- return __libelf_read_mmaped_file (-1, image, 0, size, ELF_C_READ_MMAP, NULL);
+ return __libelf_read_mmaped_file (-1, image, 0, size,
+ ELF_C_READ_MMAP_PRIVATE, NULL);
}
diff --git a/tests/elfgetzdata.c b/tests/elfgetzdata.c
index 0af6c223a06b..a50275fea1a7 100644
--- a/tests/elfgetzdata.c
+++ b/tests/elfgetzdata.c
@@ -69,7 +69,8 @@ main (int argc, char *argv[])
else
{
assert (do_mem);
- // We mmap the memory ourselves, explicitly PROT_READ only
+ // We mmap the memory ourselves, explicitly PROT_READ | PROT_WRITE
+ // elf_memory needs writable memory when using elf_compress.
struct stat st;
if (fstat (fd, &st) != 0)
{
@@ -79,7 +80,8 @@ main (int argc, char *argv[])
continue;
}
map_size = st.st_size;
- map_address = mmap (NULL, map_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ map_address = mmap (NULL, map_size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE, fd, 0);
if (map_address == MAP_FAILED)
{
printf ("%s cannot mmap %s\n", argv[cnt], strerror (errno));
--
2.47.0
