When an ELF file is opened with ELF_C_READ_MMAP, its mapping is
created with PROT_READ.  For native-endian and suitably aligned ELF
files, the ELF header currently points directly into that mapping.

elf_update with ELF_C_NULL may update fields in the ELF header while
calculating the file layout.  This attempts to write to the read-only
mapping and can cause a segmentation fault.

Keep a writable copy of the ELF header when the file is opened with
ELF_C_READ_MMAP, as is already done for section headers.  Cover both
ELF32 and ELF64, and exercise the read-only mmap path in
test-manyfuncs.

* libelf/elf_begin.c (file_read_elf): Copy the ELF header when using
  ELF_C_READ_MMAP.
* tests/test-manyfuncs.c (check_elf): Exercise ELF_C_READ_MMAP.

https://sourceware.org/bugzilla/show_bug.cgi?id=33968
Signed-off-by: zengliwen <[email protected]>
---
 libelf/elf_begin.c     | 2 ++
 tests/test-manyfuncs.c | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index 0c9b95dd..531a0865 100644
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -346,6 +346,7 @@ file_read_elf (int fildes, void *map_address, unsigned char 
*e_ident,
 
       /* This is a 32-bit binary.  */
       if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
+         && cmd != ELF_C_READ_MMAP /* We need a copy to be able to write.  */
          && (ALLOW_UNALIGNED
              || (ehdr & (__alignof__ (Elf32_Ehdr) - 1)) == 0))
        {
@@ -436,6 +437,7 @@ file_read_elf (int fildes, void *map_address, unsigned char 
*e_ident,
 
       /* This is a 64-bit binary.  */
       if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
+         && cmd != ELF_C_READ_MMAP /* We need a copy to be able to write.  */
          && (ALLOW_UNALIGNED
              || (ehdr & (__alignof__ (Elf64_Ehdr) - 1)) == 0))
        {
diff --git a/tests/test-manyfuncs.c b/tests/test-manyfuncs.c
index 3129682d..bf96b3fc 100644
--- a/tests/test-manyfuncs.c
+++ b/tests/test-manyfuncs.c
@@ -52,7 +52,7 @@ check_elf (const char *fname, bool use_mmap)
       exit (1);
     }
 
-  Elf *elf = elf_begin (fd, use_mmap ? ELF_C_READ_MMAP_PRIVATE : ELF_C_READ,
+  Elf *elf = elf_begin (fd, use_mmap ? ELF_C_READ_MMAP : ELF_C_READ,
                        NULL);
   if (elf == NULL)
     {
@@ -63,7 +63,7 @@ check_elf (const char *fname, bool use_mmap)
   /* Check if we can update the elf layout without having read anything.
      This used to crash with 65280+ sections.
      https://sourceware.org/bugzilla/show_bug.cgi?id=33967
-     Still crashes for read-only mmap.
+     It also used to crash when opened with ELF_C_READ_MMAP.
      https://sourceware.org/bugzilla/show_bug.cgi?id=33968
   */
   if (elf_update (elf, ELF_C_NULL) < 0)
-- 
2.43.0

Reply via email to