On 17/12/20 11:48, Laurent Vivier wrote: > Le 17/12/2020 à 11:17, Giuseppe Musacchio a écrit : >> Some ELF binaries encode the .bss section as an extension of the data >> ones by setting the segment p_memsz > p_filesz. Some other binaries take >> a different route and encode it as a stand-alone PT_LOAD segment with >> p_filesz = 0 and p_memsz > 0. >> >> Both the encodings are actually correct per ELF specification but the >> ELF loader had some troubles in handling the former: with the old logic >> it was very likely to get Qemu to crash in zero_bss when trying to >> access unmapped memory. >> >> zero_bss isn't meant to allocate whole zero-filled segments but to >> "complete" a previously mapped segment with the needed zero bits. >> >> The fix is pretty simple, if the segment is completely zero-filled we >> simply allocate one or more pages (according to p_memsz) and avoid >> calling zero_bss altogether. > > > Is this also fixing what "linux-user/elfload: Fix handling of pure BSS > segments" [1] patch fixes? >
Yes, I guess I'm not the first one to hit this problem. > Thanks, > Laurent > [1] https://patchew.org/QEMU/[email protected]/ > >> Signed-off-by: Giuseppe Musacchio <[email protected]> >> --- >> linux-user/elfload.c | 30 ++++++++++++++++++++---------- >> 1 file changed, 20 insertions(+), 10 deletions(-) >> >> diff --git a/linux-user/elfload.c b/linux-user/elfload.c >> index 0b02a92602..a16c240e0f 100644 >> --- a/linux-user/elfload.c >> +++ b/linux-user/elfload.c >> @@ -2776,14 +2776,16 @@ 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_ef = vaddr + eppnt->p_filesz; >> + vaddr_em = vaddr + eppnt->p_memsz; >> >> /* >> - * Some segments may be completely empty without any backing >> file >> - * segment, in that case just let zero_bss allocate an empty >> buffer >> - * for it. >> + * Some segments may be completely empty, with a non-zero >> p_memsz >> + * but no backing file segment. >> */ >> if (eppnt->p_filesz != 0) { >> + 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); >> @@ -2791,14 +2793,22 @@ static void load_elf_image(const char *image_name, >> int image_fd, >> if (error == -1) { >> goto exit_mmap; >> } >> - } >> >> - vaddr_ef = vaddr + eppnt->p_filesz; >> - vaddr_em = vaddr + eppnt->p_memsz; >> + /* >> + * If the load segment requests extra zeros (e.g. bss), map >> it. >> + */ >> + if (eppnt->p_filesz < eppnt->p_memsz) { >> + zero_bss(vaddr_ef, vaddr_em, elf_prot); >> + } >> + } else if (eppnt->p_memsz != 0) { >> + vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_memsz + >> vaddr_po); >> + error = target_mmap(vaddr_ps, vaddr_len, elf_prot, >> + MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS, >> + -1, 0); >> >> - /* If the load segment requests extra zeros (e.g. bss), map it. >> */ >> - if (vaddr_ef < vaddr_em) { >> - zero_bss(vaddr_ef, vaddr_em, elf_prot); >> + if (error == -1) { >> + goto exit_mmap; >> + } >> } >> >> /* Find the full program boundaries. */ >> >
