This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR ld/16322: ld fails to generate GNU_RELRO segment
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>, Nick Clifton <nickc at redhat dot com>
- Date: Fri, 20 Dec 2013 06:00:12 -0800
- Subject: Re: [PATCH] PR ld/16322: ld fails to generate GNU_RELRO segment
- Authentication-results: sourceware.org; auth=none
- References: <20131212185803 dot GA2434 at intel dot com>
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?
--
H.J.