Re: [PATCH] linux-user/elfload: Fix handling of pure BSS segments

[Peter Maydell]

On Wed, 18 Nov 2020 at 16:55, Stephen Long <steplong@quicinc.com> wrote:
>
> qemu-user fails to load ELFs with only BSS and no data section
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Stephen Long <steplong@quicinc.com>
> ---
>
> Submitting this on behalf of Ben Hutchings. Feel free to edit the commit
> msg.
>
>  linux-user/elfload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 0b02a92602..af16d94c61 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2783,7 +2783,7 @@ static void load_elf_image(const char *image_name, int 
> image_fd,
>               * segment, in that case just let zero_bss allocate an empty 
> buffer
>               * for it.
>               */
> -            if (eppnt->p_filesz != 0) {
> +            if (vaddr_len != 0) {
>                  error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
>                                      MAP_PRIVATE | MAP_FIXED,
>                                      image_fd, eppnt->p_offset - vaddr_po);

So (having run into a different instance of this elsewhere), a
couple of questions:

(a) what does "fails to load" mean here? In the sample binary
I had, we got a SIGSEGV in zero_bss() when it tried to memset()
memory that hadn't been mmap()ed. Is that the only failure mode,
or can this manifest in other ways too?

(b) The comment immediately before this change says:
     * Some segments may be completely empty without any backing file
     * segment, in that case just let zero_bss allocate an empty buffer
     * for it.
which is justifying why it was looking at p_filesz and not vaddr_len.
With this change to the code, the comment becomes stale and needs
updating.

(c) After this change, are there still cases where zero_bss()
needs to do its mmap()/page_set_flags(), or does that become
unnecessary ?

thanks
-- PMM