[PATCH] Skip PT_DYNAMIC segment if its p_filesz == 0 [BZ #22101]

[H.J. Lu]

On 9/25/17, Carlos O'Donell <carlos@redhat.com> wrote:
> On 09/25/2017 06:33 PM, H.J. Lu wrote:
>> ELF object generated with "objcopy --only-keep-debug" has
>>
>> Type     Offset  VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>> DYNAMIC  0x0+e28 0x0+200e40 0x0+200e40 0x0+    0x0+1a0 RW  0x8
>>
>> with 0 file size. ld.so should skip such PT_DYNAMIC segments.
>>
>> Tested on x86-64.  OK for master?
>
> Are all such `objcopy --only-kee--debug` objects left with 0 file size?

Yes, that is correct.

> After your patch what happens when you run ldd on such an object?

Before:

 [hjl@gnu-efi-2 elf]$ /lib64/ld-2.25.so --list ./tst-debug1mod1.so
	statically linked

After:

[hjl@gnu-efi-2 elf]$ ./ld.so --list ./tst-debug1mod1.so
./tst-debug1mod1.so: error while loading shared libraries:
./tst-debug1mod1.so: object file has no dynamic section
[hjl@gnu-efi-2 elf]$

> The idea in bug 22101 is to add minimal code early in the dynamic
> loader to identify specially marked objects and ignore them. This
> way we put an end to the guessing game of what constitutes a valid
> ELF object.

I am not sure if this is necessary.  Any such changes won't work with
debug only objects generated by the current objcopy.

> Granted, the code you've added is quite small, so it looks like
> an interesting short term solution. It needs a more verbose
> comment for the PT_DYNAMIC case explaining why we check if
> ph->p_filesz is zero and what the consequences of that are since
> we *never* add such defensive checks in ld.so because they would

This is not entirely true.   There are plenty of sanity checks in ld.so.
For example, a few lines below, there are

       case PT_LOAD:
          /* A load command tells us to map in part of the file.
             We record the load commands and process them all later.  */
          if (__glibc_unlikely ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0))
            {
              errstring = N_("ELF load command alignment not page-aligned");
              goto call_lose;
            }
          if (__glibc_unlikely (((ph->p_vaddr - ph->p_offset)
                                 & (ph->p_align - 1)) != 0))
            {
              errstring
                = N_("ELF load command address/offset not properly aligned");
              goto call_lose;
            }

> slow down the average case of a correctly formed binary (and
> thus need a hefty comment).
>

Here is the updated patch with comments:

        case PT_DYNAMIC:
          if (ph->p_filesz)
            {
              /* Debuginfo only file from "objcopy --only-keep-debug"
                 contains PT_DYNAMIC segment with p_filesz == 0.  Skip
                 such segment to avoid crash later.  */
              l->l_ld = (void *) ph->p_vaddr;
              l->l_ldnum = ph->p_memsz / sizeof (ElfW(Dyn));
            }
          break;


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Skip-PT_DYNAMIC-segment-with-p_filesz-0-BZ-22101.patch
Type: text/x-patch
Size: 4114 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20170926/c8c48116/attachment.bin>