[PATCH] ld.so: Handle read-only dynamic section gracefully [BZ #28340]

[Siddhesh Poyarekar]

On 9/16/21 11:08 PM, H.J. Lu wrote:
> On Thu, Sep 16, 2021 at 9:45 AM Siddhesh Poyarekar
> <siddhesh@sourceware.org> wrote:
>>
>> On 9/16/21 8:48 PM, H.J. Lu wrote:
>>> On Thu, Sep 16, 2021 at 7:11 AM Carlos O'Donell <carlos@redhat.com> wrote:
>>>>
>>>> On 9/16/21 00:48, Florian Weimer via Libc-alpha wrote:
>>>>> * H. J. Lu:
>>>>>
>>>>>> There is nothing wrong with read-only dynamic segment.
>>>>>
>>>>> A relocated DYNAMIC array is part of the ABI for !DL_RO_DYN_SECTION.
>>>>> ELF requires that DT_STRTAB is present.  DT_STRTAB needs relocation.
>>>>> This means that for !DL_RO_DYN_SECTION, the dynamic segment cannot be in
>>>>> a read-ony LOAD segment for a valid ELF file.
>>>>
>>>> I agree strongly with this position.
>>>>
>>>> Even with PT_GNU_RELRO, we must only go in one direction from RW -> RO (to avoid
>>>> other security issues e.g. hardening not loosening the restrictions).
>>>>
>>>> In theory the vDSO is invalid.
>>>>
>>>> In practice it is a DL_RO_DYN_SECTION DSO but selected dynamically at runtime
>>>> rather than statically at compile time for the target.
>>
>> Actually it isn't.  The dynamic section in the vdso is already relocated
>> by the kernel when it's mapped in.  DL_RO_DYN_SECTION DSOs are not
>> relocated because of which any references to pointers written in the
>> .dynamic section need to be relocated.
> 
> No.  Relocation of vDSO dynamic section is done by elf_get_dynamic_info.
> Here is a patch to remove the hack for vDSO.
> 

Ahh, I misunderstood the comment in setup_vdso, sorry.  I verified by 
running under the debugger that the kernel doesn't adjust .dynamic 
entries before mapping the vdso.

This updated patch is definitely better IMO but it still doesn't resolve 
the two outstanding questions posed so far.  I'm on the fence about the 
first one (it's imprecise but the cost of imprecision doesn't seem high 
enough to warrant the extra check) but slanted slightly towards 
allocating memory and writing out relocated addresses in the interest of 
keeping the user experience with dl_iterate_phdr and friends consistent.

1. Should the readonly decision be based solely on DYNAMIC flags or also 
consider flags on the encompassing LOAD segment?

2. Do we want to leave .dynamic unrelocated for read-only DYNAMIC or 
should we instead allocate an array to write relocated addresses in 
there, like we did for vdso?

Siddhesh