#919921 qemu-user Linux ELF loader fails to handle pure BSS segments

Package:
qemu-user
Source:
qemu
Description:
QEMU user mode emulation binaries
Submitter:
Ben Hutchings
Date:
2022-04-24 08:45:03 UTC
Severity:
normal
Tags:
#919921#5
Date:
2019-01-20 17:55:00 UTC
From:
To:
I've been building and testing klibc across many architectures using
qemu-user, and I found that qemu-user fails to load a few programs on
a few architectures, reporting an EINVAL error code.  Here's the
"readelf -l" output for one such program:

    Elf file type is EXEC (Executable file)
    Entry point 0x10000100
    There are 5 program headers, starting at offset 52

    Program Headers:
      Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
      PHDR           0x000034 0x10000034 0x10000034 0x000a0 0x000a0 R   0x4
      INTERP         0x0000d4 0x100000d4 0x100000d4 0x0002a 0x0002a R   0x1
          [Requesting program interpreter: /lib/klibc-R7FVdnsTBUFpWPgCV6FR07b-mf8.so]
      LOAD           0x000000 0x10000000 0x10000000 0x002f8 0x002f8 R E 0x10000
      LOAD           0x010000 0x10020000 0x10020000 0x00000 0x08000 RW  0x10000
      GNU_STACK      0x000000 0x00000000 0x00000000 0x00000 0x00000 RWE 0x10

     Section to Segment mapping:
      Segment Sections...
       00
       01     .interp
       02     .interp .text .rodata .eh_frame
       03     .bss
       04

The unusual feature of this program, and all the others that failed,
is that there is a LOAD segment with a file-size of 0 (i.e.  only BSS,
no initialised data).  load_elf_image() will try to mmap() initialised
data for this section even though there is none and a length of 0 is
invalid.

The change that seems to fix this is to skip the mmap() in this case:
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2316,11 +2316,13 @@ static void load_elf_image(const char *i
             vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
             vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + vaddr_po);

-            error = target_mmap(vaddr_ps, vaddr_len,
-                                elf_prot, MAP_PRIVATE | MAP_FIXED,
-                                image_fd, eppnt->p_offset - vaddr_po);
-            if (error == -1) {
-                goto exit_perror;
+            if (vaddr_len != 0) {
+                error = target_mmap(vaddr_ps, vaddr_len,
+                                    elf_prot, MAP_PRIVATE | MAP_FIXED,
+                                    image_fd, eppnt->p_offset - vaddr_po);
+                if (error == -1) {
+                    goto exit_perror;
+                }
             }

             vaddr_ef = vaddr + eppnt->p_filesz;
--- END ---

#919921#10
Date:
2019-01-22 05:43:21 UTC
From:
To:
Forwarding to qemu-devel@
http://bugs.debian.org/919921

Thanks!

20.01.2019 20:55, Ben Hutchings wrote:

#919921#15
Date:
2019-01-22 09:39:25 UTC
From:
To:
Hi Ben,

This is probably not the good fix, since now your process doesn't have
anything mapped to use his BSS :)

What about this fix instead, using the segment memory size rather than
the file size:

#919921#20
Date:
2019-01-23 05:31:35 UTC
From:
To:
Not true.  The mapping happens in zero_bss.

No, there's only filesz bytes in the file.  I'd expect zero_bss to map over the
extra that you just mapped, but it doesn't help.


r~

#919921#25
Date:
2019-10-07 17:29:19 UTC
From:
To:
Is there a reason why my fix can't be applied?  Do I need to open a bug
report in Launchpad for this?

Ben.
-- 
Ben Hutchings - Debian developer, member of kernel, installer and LTS teams

#919921#30
Date:
2019-10-07 17:34:28 UTC
From:
To:
Hi Ben,

Simply send your patch formally to qemu-devel@nongnu.org.

#919921#35
Date:
2020-07-22 19:30:44 UTC
From:
To:
I see that qemu 4.1 have this commit:
https://salsa.debian.org/qemu-team/qemu/-/commit/d87146bce08d3d2ea6c00025d7ee0bfa77991692

-            error = target_mmap(vaddr_ps, vaddr_len,
-                                elf_prot, MAP_PRIVATE | MAP_FIXED,
-                                image_fd, eppnt->p_offset - vaddr_po);
-            if (error == -1) {
-                goto exit_perror;
+            /*
+             * Some segments may be completely empty without any backing file
+             * segment, in that case just let zero_bss allocate an empty buffer
+             * for it.
+             */
+            if (eppnt->p_filesz != 0) {
+                error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
+                                    MAP_PRIVATE | MAP_FIXED,
+                                    image_fd, eppnt->p_offset - vaddr_po);
+
+                if (error == -1) {
+                    goto exit_perror;
+                }
             }


I'm not sure I understand what's going on here. This looks like
a different case to me, maybe we still need to add the suggested
condition (vaddr_len != 0) to the mix?

Thanks,

/mjt

#919921#40
Date:
2022-04-24 08:41:24 UTC
From:
To:
Control: tag -1 - patch

Hi Ben!

I just come across an old qemu-user bugreport. Can you tell whenever this
is still relevant for current qemu (6.x or 7.0)?  There were some changes
in that area, and I don't know which program you used for testing.

We had quite some discussion in that bug report, and the last question is
still unanswered.

Removing the tag "patch" for now, because the patch provided does not apply
to qemu anymore.

Thank you!

/mjt