[PATCH 2/2] libdwfl: elf_from_remote_memory only trust shdrs of last file-only segment.

Jan Kratochvil jan.kratochvil at redhat.com
Sun Mar 16 16:46:38 UTC 2014


On Mon, 03 Mar 2014 15:31:28 +0100, Mark Wielaard wrote:
[...]
> --- a/libdwfl/elf-from-memory.c
> +++ b/libdwfl/elf-from-memory.c
> @@ -196,12 +196,14 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>    /* Scan for PT_LOAD segments to find the total size of the file image.  */
>    size_t contents_size = 0;
>    GElf_Off segments_end = 0;
> +  GElf_Off segments_end_mem = 0;
>    GElf_Addr loadbase = ehdr_vma;
>    bool found_base = false;
>    switch (ehdr.e32.e_ident[EI_CLASS])
>      {
>        inline bool handle_segment (GElf_Addr vaddr, GElf_Off offset,
> -				  GElf_Xword filesz, GElf_Xword palign)
> +				  GElf_Xword filesz, GElf_Xword memsz,
> +				  GElf_Xword palign)
>  	{
>  	  /* Sanity check the alignment requirements.  */
>  	  if ((palign & (pagesize - 1)) != 0
> @@ -221,6 +223,7 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>  	    }
>  
>  	  segments_end = offset + filesz;
> +	  segments_end_mem = offset + memsz;
>  	  return false;
>  	}
>  
> @@ -231,7 +234,8 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>        for (uint_fast16_t i = 0; i < phnum; ++i)
>  	if (phdrs.p32[i].p_type == PT_LOAD)
>  	  if (handle_segment (phdrs.p32[i].p_vaddr, phdrs.p32[i].p_offset,
> -			      phdrs.p32[i].p_filesz, phdrs.p32[i].p_align))
> +			      phdrs.p32[i].p_filesz, phdrs.p32[i].p_memsz,
> +			      phdrs.p32[i].p_align))
>  	    goto bad_elf;
>        break;
>  
> @@ -242,7 +246,8 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>        for (uint_fast16_t i = 0; i < phnum; ++i)
>  	if (phdrs.p64[i].p_type == PT_LOAD)
>  	  if (handle_segment (phdrs.p64[i].p_vaddr, phdrs.p64[i].p_offset,
> -			      phdrs.p64[i].p_filesz, phdrs.p64[i].p_align))
> +			      phdrs.p64[i].p_filesz, phdrs.p64[i].p_memsz,
> +			      phdrs.p64[i].p_align))
>  	    goto bad_elf;
>        break;
>  
> @@ -253,9 +258,11 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>  
>    /* Trim the last segment so we don't bother with zeros in the last page
>       that are off the end of the file.  However, if the extra bit in that
> -     page includes the section headers, keep them.  */
> +     page includes the section headers and the memory isn't extended (which
> +     might indicate it will have been reused otherwise), keep them.  */
>    if ((GElf_Off) contents_size > segments_end
> -      && (GElf_Off) contents_size >= shdrs_end)
> +      && (GElf_Off) contents_size >= shdrs_end
> +      && segments_end == segments_end_mem)
>      {
>        contents_size = segments_end;
>        if ((GElf_Off) contents_size < shdrs_end)
> @@ -321,8 +328,9 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>  	    goto read_error;
>  
>        /* If the segments visible in memory didn't include the section
> -	 headers, then clear them from the file header.  */
> -      if (contents_size < shdrs_end)
> +	 headers or the memory might be used for something else, then
> +	 clear them from the file header.  */
> +      if (contents_size < shdrs_end || segments_end != segments_end_mem)
>  	{
>  	  ehdr.e64.e_shoff = 0;
>  	  ehdr.e64.e_shnum = 0;

I do not see why to make this change.  The chunk @@ -253,9 +258,11 @@ above
already trimmed contents_size to segment's FileSiz so the original conditional
'contents_size < shdrs_end' has to be true.

I have verified that this patch will fix the loading of DWARF from
/usr/lib/debug/ for the Firefox reproducer.  This patch still works after
removing this last chunk.  I have verified that both these conditionals are
true for the Firefox reproducer:
	contents_size < shdrs_end
	segments_end != segments_end_mem

If any such conditional matters here
then just 'segments_end != segments_end_mem' is not complete as for example
I can have section headers at the very beginning of the ELF file and they
would get cleared just because there is some .bss at the end of the ELF file.

BTW there is an equivalent code variant to patch like this chunk also for
32-bit ELFs, if there is a patch needed.


I am OK with the patch if the last chunk gets removed.



Thanks,
Jan


More information about the elfutils-devel mailing list