Re: [PATCH] PR ld/16322: ld fails to generate GNU_RELRO segment

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Dec 20, 2013 at 6:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Dec 12, 2013 at 10:58 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> Hi,
>>
>> There are 2 bugs:
>>
>> 1. lang_size_sections didn't properly align base to the maximum
>> alignment power of sections between DATA_SEGMENT_ALIGN and
>> DATA_SEGMENT_RELRO_END.
>> 2. ld failed to adjust LOAD segment to generate GNU_RELRO segment
>> when LOAD segment doesn't fit GNU_RELRO segment.  This is
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=14207
>>
>> We "fixed" ld by not generating GNU_RELRO segment.  This patch
>> adjusts LOAD segment to generate GNU_RELRO segment.  It fixes
>> PR ld/16322 and at the same time it also fixes PR binutils/16323
>> since now we can adjust LOAD segment if it is too small.  OK
>> to install?
>>
>> Thanks.
>>
>>
>> H.J.
>> ---
>> bfd/
>>
>> 2013-12-12  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>         PR ld/14207
>>         PR ld/16322
>>         PR binutils/16323
>>         * elf.c (_bfd_elf_map_sections_to_segments): Don't check section
>>         size for PT_GNU_RELRO segment.
>>         (assign_file_positions_for_load_sections): If PT_LOAD segment
>>         doesn't fit PT_GNU_RELRO segment, adjust its p_filesz and p_memsz.
>>
>> ld/
>>
>> 2013-12-12  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>         PR ld/14207
>>         PR ld/16322
>>         PR binutils/16323
>>         * ldlang.c (lang_size_sections): Properly align RELRO base.
>>
>> ld/testsuite/
>>
>> 2013-12-12  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>         PR ld/14207
>>         PR ld/16322
>>         PR binutils/16323
>>         * ld-elf/pr16322.d: New file.
>>         * ld-elf/pr16322.s: Likewise.
>>
>>         * ld-x86-64/pr14207.d: Expect PT_GNU_RELRO segment.
>>
>> diff --git a/bfd/elf.c b/bfd/elf.c
>> index 66d9644..65d579b 100644
>> --- a/bfd/elf.c
>> +++ b/bfd/elf.c
>> @@ -4176,11 +4176,7 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info)
>>                         == (SEC_LOAD | SEC_HAS_CONTENTS))
>>                       break;
>>
>> -                 if (i == (unsigned) -1)
>> -                   continue;
>> -
>> -                 if (m->sections[i]->vma + m->sections[i]->size
>> -                     >= info->relro_end)
>> +                 if (i != (unsigned) -1)
>>                     break;
>>                 }
>>             }
>> @@ -4369,6 +4365,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
>>    unsigned int alloc;
>>    unsigned int i, j;
>>    bfd_vma header_pad = 0;
>> +  bfd_vma relro_start = 0, relro_end = 0;
>>
>>    if (link_info == NULL
>>        && !_bfd_elf_map_sections_to_segments (abfd, link_info))
>> @@ -4439,6 +4436,23 @@ assign_file_positions_for_load_sections (bfd *abfd,
>>      header_pad -= off;
>>    off += header_pad;
>>
>> +  /* Get start and end of PT_GNU_RELRO segment.  */
>> +  if (link_info != NULL)
>> +    {
>> +      relro_start = link_info->relro_start;
>> +      relro_end = link_info->relro_end;
>> +    }
>> +  else
>> +    {
>> +      for (m = elf_seg_map (abfd); m != NULL; m = m->next)
>> +       if (m->p_type == PT_GNU_RELRO)
>> +         {
>> +           relro_start = m->p_paddr;
>> +           relro_end = relro_start + m->p_size;
>> +           break;
>> +         }
>> +    }
>> +
>>    for (m = elf_seg_map (abfd), p = phdrs, j = 0;
>>         m != NULL;
>>         m = m->next, p++, j++)
>> @@ -4781,6 +4795,23 @@ assign_file_positions_for_load_sections (bfd *abfd,
>>                 p->p_flags |= PF_W;
>>             }
>>         }
>> +
>> +      if (relro_start != 0
>> +         && p->p_type == PT_LOAD
>> +         && p->p_vaddr >= relro_start)
>> +       {
>> +         /* If PT_LOAD segment doesn't fit PT_GNU_RELRO segment,
>> +            adjust its p_filesz and p_memsz.  */
>> +         if (p->p_vaddr + p->p_filesz < relro_end)
>> +           {
>> +             bfd_vma adjust = relro_end - (p->p_vaddr + p->p_filesz);
>> +             p->p_filesz += adjust;
>> +             off += adjust;
>> +           }
>> +         if (p->p_vaddr + p->p_memsz < relro_end)
>> +           p->p_memsz += relro_end - (p->p_vaddr + p->p_memsz);
>> +       }
>> +
>>        off -= off_adjust;
>>
>>        /* Check that all sections are in a PT_LOAD segment.
>> diff --git a/ld/ldlang.c b/ld/ldlang.c
>> index ba7f493..7851615 100644
>> --- a/ld/ldlang.c
>> +++ b/ld/ldlang.c
>> @@ -5407,7 +5407,8 @@ lang_size_sections (bfd_boolean *relax, bfd_boolean check_regions)
>>             {
>>               if (expld.dataseg.base - (1 << max_alignment_power) < old_base)
>>                 expld.dataseg.base += expld.dataseg.pagesize;
>> -             expld.dataseg.base -= (1 << max_alignment_power);
>> +             /* Properly align base to max_alignment_power.  */
>> +             expld.dataseg.base &= ~((1 << max_alignment_power) - 1);
>>               lang_reset_memory_regions ();
>>               one_lang_size_sections_pass (relax, check_regions);
>>             }
>
> Are there any objections to this patch?
>

I checked it in.  Please let me know if there are any problems.

Thanks.


-- 
H.J.