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

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

On Thu, Jan 9, 2014 at 8:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jan 9, 2014 at 6:50 AM, Alan Modra <amodra@gmail.com> wrote:
>> On Thu, Jan 09, 2014 at 04:08:17AM -0800, H.J. Lu wrote:
>>> On Wed, Jan 8, 2014 at 8:13 PM, Alan Modra <amodra@gmail.com> wrote:
>>> > On Wed, Jan 08, 2014 at 06:01:49AM -0800, H.J. Lu wrote:
>>> >> >> +         /* If PT_LOAD segment doesn't fit PT_GNU_RELRO segment,
>>> >> >> +            adjust its p_filesz and p_memsz.  */
>>> >
>>> > This is broken.  What if there is another LOAD segment following
>>> > the one you're adjusting?
>>>
>>> It will only happen with a costumer linker script using
>>> DATA_SEGMENT_ALIGN,  DATA_SEGMENT_RELRO_END,
>>> DATA_SEGMENT_END.
>>
>> Right.  Such a script is possible.
>>
>>>  Do you have a testcase?
>>
>> No.
>>
>>> We can
>>> either ignore relro or issue an error.
>>
>> Please fix it one way or the other.
>>
>
> This patch issues an error if there is another PT_LOAD
> segment:
>
> diff --git a/bfd/elf.c b/bfd/elf.c
> index 870e281..452cae0 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -4816,6 +4816,22 @@ assign_file_positions_for_load_sections (bfd *abfd,
>    if (p->p_vaddr + p->p_filesz < relro_end)
>      {
>        bfd_vma adjust = relro_end - (p->p_vaddr + p->p_filesz);
> +      /* This won't work if there is another PT_LOAD segment
> + after this PT_LOAD segment.  */
> +      struct elf_segment_map *mn;
> +      for (mn = m->next; mn != NULL; mn = mn->next)
> + if (mn->p_type == PT_LOAD)
> +  break;
> +      if (mn != NULL)
> + {
> +  _bfd_error_handler
> +    (_("%B: 2 PT_LOAD segments overlap due to PT_GNU_RELRO segment"),
> +     abfd);
> +  print_segment_map (m);
> +  print_segment_map (mn);
> +  bfd_set_error (bfd_error_bad_value);
> +  return FALSE;
> + }
>        p->p_filesz += adjust;
>        off += adjust;
>      }
>

This triggered tests in  ld/testsuite/ld-elf/binutils.exp for x86_64-nacl.
Before my change, ld -shared -z relro generates

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x010000 0x00000000 0x00000000 0x10000 0x10000 R E 0x10000
  LOAD           0x000000 0x10000000 0x10000000 0x001d8 0x001d8 R   0x10000
  LOAD           0x00f800 0x1001f800 0x1001f800 0x00458 0x00458 RW  0x10000
  DYNAMIC        0x00fc00 0x1001fc00 0x1001fc00 0x00058 0x00058 RW  0x4
  TLS            0x00f800 0x1001f800 0x1001f800 0x00001 0x00800 R   0x400

 Section to Segment mapping:
  Segment Sections...
   00     .text
   01     .hash .dynsym .dynstr
   02     .tdata .dynamic
   03     .dynamic
   04     .tdata .tbss

After my change, it generates:

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x010000 0x00000000 0x00000000 0x10000 0x10000 R E 0x10000
  LOAD           0x000000 0x10000000 0x10000000 0x001f8 0x001f8 R   0x10000
  LOAD           0x00f800 0x1001f800 0x1001f800 0x00800 0x00800 RW  0x10000
  DYNAMIC        0x00fc00 0x1001fc00 0x1001fc00 0x00058 0x00058 RW  0x4
  TLS            0x00f800 0x1001f800 0x1001f800 0x00001 0x00800 R   0x400
  GNU_RELRO      0x00f800 0x1001f800 0x1001f800 0x00800 0x00800 R   0x1

 Section to Segment mapping:
  Segment Sections...
   00     .text
   01     .hash .dynsym .dynstr
   02     .tdata .dynamic
   03     .dynamic
   04     .tdata .tbss
   05     .tdata .dynamic

It is OK to have another PT_LOAD segment.

H.J.