[PATCH v2] elf: Support multiple PT_GNU_RELRO segments

[Florian Weimer]

* Adhemerval Zanella:

> When binaries become extremely large, PC-relative references to a
> single GOT can exceed the +/- 2GB limit.

The limit is architecture-specific.  I assume this is talking about
x86-64.  Maybe add this to the commit message.

> To resolve this, we'd like to generate multiple GOTs, which would
> require multiple PT_GNU_RELRO segments.

> This change modifies RELRO protection by removing cached fields
> (l_relro_addr and l_relro_size) and instead iterating over all program
> headers to protect every PT_GNU_RELRO segment discovered. The same
> applies to _dl_readonly_area, where a queried range may now span
> multiple adjacent segments.
>
> Since mprotect operates on whole pages while the static linker only
> aligns the segment end (and possibly only to a link-time page size
> smaller than the run-time one), the protected range is computed by the
> new _dl_relro_range helper. The end is rounded down so following
> writable data never loses write access, and the start is rounded down
> only for a segment that starts a PT_LOAD segment, where the leading
> page slack is unused (BFD ld does not page-align the start, so
> rounding it up could drop the protection entirely).  A RELRO segment
> in the middle of a PT_LOAD segment is preceded by live writable data,
> so its start is rounded up instead.

Should the final case (rounding up) be an error instead?  It could
result in unexpected loss of RELRO protection.

> diff --git a/elf/dl-readonly-area.c b/elf/dl-readonly-area.c
> index 833f4559049..0b7a5c0d319 100644
> --- a/elf/dl-readonly-area.c
> +++ b/elf/dl-readonly-area.c

> +static enum dl_readonly_area_error_type
>  check_relro (const struct link_map *l, uintptr_t start, uintptr_t end)
>  {
> +  /* The range may span multiple PT_GNU_RELRO segments whose ranges are
> +     adjacent, so accumulate the covered bytes.  */
> +  size_t size = end - start;
> +  for (const ElfW(Phdr) *ph = l->l_phdr; ph < &l->l_phdr[l->l_phnum]; ++ph)
> +    if (ph->p_type == PT_GNU_RELRO)
> +      {
> +	bool at_load_start = false;
> +	for (const ElfW(Phdr) *lph = l->l_phdr;
> +	     lph < &l->l_phdr[l->l_phnum]; ++lph)
> +	  if (lph->p_type == PT_LOAD && lph->p_vaddr == ph->p_vaddr)
> +	    {
> +	      at_load_start = true;
> +	      break;
> +	    }

That's quadratic behavior, which isn't great for huge binaries.

Do we need to do any rounding here?  I think we want to make sure that
the address comes from .data.rel.ro or equivalent.  I would expect it up
to the toolchain to make sure that PT_GNU_RELRO covers the
language-defined objects.  I think we can restrict the rounding
operations the actual mapping steps only.

> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
> index 191d39cbbd9..5154937897e 100644
> --- a/elf/dl-reloc.c
> +++ b/elf/dl-reloc.c
> @@ -406,23 +406,28 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>  void
>  _dl_protect_relro (struct link_map *l)
>  {
> +  for (const ElfW(Phdr) *ph = l->l_phdr; ph < &l->l_phdr[l->l_phnum]; ++ph)
> +    if (ph->p_type == PT_GNU_RELRO)
> +      {
> +	bool at_load_start = false;
> +	for (const ElfW(Phdr) *lph = l->l_phdr;
> +	     lph < &l->l_phdr[l->l_phnum]; ++lph)
> +	  if (lph->p_type == PT_LOAD && lph->p_vaddr == ph->p_vaddr)
> +	    {
> +	      at_load_start = true;
> +	      break;
> +	    }

This happens just once, so the quadratic behavior is more acceptable.

> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
> index ffbe9958fed..28d2bbf80e6 100644
> --- a/elf/tst-relro-symbols.py
> +++ b/elf/tst-relro-symbols.py
> @@ -32,25 +32,30 @@ sys.path.append(os.path.join(

> +def find_relro(path: str, img: glibcelf.Image) -> list:
> +    """Discover the address ranges of the PT_GNU_RELRO segments."""
> +    # The computation is not entirely accurate because
> +    # _dl_protect_relro in elf/dl-reloc.c rounds both the
> +    # start end and downwards using the run-time page size.

If _dl_protect_relro is simplified as suggested, then the comment needs
to be updated.

Rest looks okay.

Thanks,
Florian