Re: [Qemu-devel] Bug#919921: qemu-user Linux ELF loader fails to handle pure BSS segments

[Philippe Mathieu-Daudé]

Hi Ben,

On 10/7/19 7:29 PM, Ben Hutchings wrote:
> On Tue, 2019-01-22 at 21:31 -0800, Richard Henderson wrote:
> > On 1/22/19 1:39 AM, Philippe Mathieu-Daudé wrote:
> > > Hi Ben,
> > >
> > > On 1/22/19 6:43 AM, Michael Tokarev wrote:
> > > > Forwarding to qemu-devel@
> > > > http://bugs.debian.org/919921
> > > >
> > > > Thanks!
> > > >
> > > > 20.01.2019 20:55, Ben Hutchings wrote:
> > > > > Package: qemu-user
> > > > > Version: 1:3.1+dfsg-2
> > > > > Severity: normal
> > > > > Tags: patch
> > > > >
> > > > > 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:
>
> Is there a reason why my fix can't be applied?  Do I need to open a bug
> report in Launchpad for this?

Simply send your patch formally to address@hidden.

> > > > > --- 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) {
> > >
> > > This is probably not the good fix, since now your process doesn't have
> > > anything mapped to use his BSS :)
> >
> > Not true.  The mapping happens in zero_bss.
> >
> > > What about this fix instead, using the segment memory size rather than
> > > the file size:
> > >
> > > -- >8 --
> > > @@ -2314,7 +2314,7 @@ static void load_elf_image(const char *image_name,
> > > int image_fd,
> > >               vaddr = load_bias + eppnt->p_vaddr;
> > >               vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
> > >               vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> > > -            vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + vaddr_po);
> > > +            vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_memsz + vaddr_po);
> > >
> > >               error = target_mmap(vaddr_ps, vaddr_len,
> > >                                   elf_prot, MAP_PRIVATE | MAP_FIXED,
> >
> > 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~