diff mbox series

[v4] ld.so: Replace DL_RO_DYN_SECTION with dl_relocate_ld

Message ID 20210919162805.3277956-1-hjl.tools@gmail.com
State Committed
Commit b413280cfb16834450f66f554bc0d618bb513851
Headers show
Series [v4] ld.so: Replace DL_RO_DYN_SECTION with dl_relocate_ld | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

H.J. Lu Sept. 19, 2021, 4:28 p.m. UTC
Changes in v4:

1. Change l_ld_readonly to bitfield.

Changes in v3:

1. Remove DL_RO_DYN_TEMP_CNT.
2. Update comments for dl_relocate_ld.

Changes in v2:

1. Replace DL_RO_DYN_SECTION with dl_relocate_ld so that MIPS and RISC-V
won't relocate dynamic section even if it is writable.
2. Add a run-time testcase.

We can't relocate entries in dynamic section if it is readonly:

1. Add a l_ld_readonly field to struct link_map to indicate if dynamic
section is readonly and set it based on p_flags of PT_DYNAMIC segment.
2. Replace DL_RO_DYN_SECTION with dl_relocate_ld to decide if dynamic
section should be relocated.
3. Remove DL_RO_DYN_TEMP_CNT.
4. Don't use a static dynamic section to make readonly dynamic section
in vDSO writable.
5. Remove the temp argument from elf_get_dynamic_info.
---
 elf/Makefile                     | 11 +++++++++--
 elf/dl-load.c                    |  3 ++-
 elf/dl-reloc-static-pie.c        | 12 +++++++++++-
 elf/get-dynamic-info.h           | 21 +++------------------
 elf/rtld.c                       |  6 ++++--
 elf/setup-vdso.h                 |  5 ++---
 elf/tst-ro-dynamic-mod.c         | 19 +++++++++++++++++++
 elf/tst-ro-dynamic-mod.map       | 16 ++++++++++++++++
 elf/tst-ro-dynamic.c             | 31 +++++++++++++++++++++++++++++++
 include/link.h                   |  3 +++
 sysdeps/generic/dl-relocate-ld.h | 32 ++++++++++++++++++++++++++++++++
 sysdeps/generic/ldsodefs.h       |  7 ++-----
 sysdeps/mips/dl-relocate-ld.h    | 32 ++++++++++++++++++++++++++++++++
 sysdeps/mips/ldsodefs.h          |  4 ----
 sysdeps/riscv/dl-relocate-ld.h   | 32 ++++++++++++++++++++++++++++++++
 sysdeps/riscv/ldsodefs.h         |  5 -----
 16 files changed, 198 insertions(+), 41 deletions(-)
 create mode 100644 elf/tst-ro-dynamic-mod.c
 create mode 100644 elf/tst-ro-dynamic-mod.map
 create mode 100644 elf/tst-ro-dynamic.c
 create mode 100644 sysdeps/generic/dl-relocate-ld.h
 create mode 100644 sysdeps/mips/dl-relocate-ld.h
 create mode 100644 sysdeps/riscv/dl-relocate-ld.h

Comments

Siddhesh Poyarekar Sept. 20, 2021, 3:28 a.m. UTC | #1
On 9/19/21 21:58, H.J. Lu wrote:
> Changes in v4:
> 
> 1. Change l_ld_readonly to bitfield.
> 
> Changes in v3:
> 
> 1. Remove DL_RO_DYN_TEMP_CNT.
> 2. Update comments for dl_relocate_ld.
> 
> Changes in v2:
> 
> 1. Replace DL_RO_DYN_SECTION with dl_relocate_ld so that MIPS and RISC-V
> won't relocate dynamic section even if it is writable.
> 2. Add a run-time testcase.
> 
> We can't relocate entries in dynamic section if it is readonly:
> 
> 1. Add a l_ld_readonly field to struct link_map to indicate if dynamic
> section is readonly and set it based on p_flags of PT_DYNAMIC segment.
> 2. Replace DL_RO_DYN_SECTION with dl_relocate_ld to decide if dynamic
> section should be relocated.
> 3. Remove DL_RO_DYN_TEMP_CNT.
> 4. Don't use a static dynamic section to make readonly dynamic section
> in vDSO writable.
> 5. Remove the temp argument from elf_get_dynamic_info.

LGTM, thanks for doing this.  AFAICT you've addressed their concerns but 
please check with Florian and Carlos before committing.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> ---
>   elf/Makefile                     | 11 +++++++++--
>   elf/dl-load.c                    |  3 ++-
>   elf/dl-reloc-static-pie.c        | 12 +++++++++++-
>   elf/get-dynamic-info.h           | 21 +++------------------
>   elf/rtld.c                       |  6 ++++--
>   elf/setup-vdso.h                 |  5 ++---
>   elf/tst-ro-dynamic-mod.c         | 19 +++++++++++++++++++
>   elf/tst-ro-dynamic-mod.map       | 16 ++++++++++++++++
>   elf/tst-ro-dynamic.c             | 31 +++++++++++++++++++++++++++++++
>   include/link.h                   |  3 +++
>   sysdeps/generic/dl-relocate-ld.h | 32 ++++++++++++++++++++++++++++++++
>   sysdeps/generic/ldsodefs.h       |  7 ++-----
>   sysdeps/mips/dl-relocate-ld.h    | 32 ++++++++++++++++++++++++++++++++
>   sysdeps/mips/ldsodefs.h          |  4 ----
>   sysdeps/riscv/dl-relocate-ld.h   | 32 ++++++++++++++++++++++++++++++++
>   sysdeps/riscv/ldsodefs.h         |  5 -----
>   16 files changed, 198 insertions(+), 41 deletions(-)
>   create mode 100644 elf/tst-ro-dynamic-mod.c
>   create mode 100644 elf/tst-ro-dynamic-mod.map
>   create mode 100644 elf/tst-ro-dynamic.c
>   create mode 100644 sysdeps/generic/dl-relocate-ld.h
>   create mode 100644 sysdeps/mips/dl-relocate-ld.h
>   create mode 100644 sysdeps/riscv/dl-relocate-ld.h
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 9f3fadc37e..eeb47145fc 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -223,7 +223,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>   	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
>   	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
>   	 tst-tls20 tst-tls21 tst-dlmopen-dlerror tst-dlmopen-gethostbyname \
> -	 tst-dl-is_dso
> +	 tst-dl-is_dso tst-ro-dynamic
>   #	 reldep9
>   tests-internal += loadtest unload unload2 circleload1 \
>   	 neededtest neededtest2 neededtest3 neededtest4 \
> @@ -359,7 +359,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
>   		libmarkermod4-1 libmarkermod4-2 libmarkermod4-3 libmarkermod4-4 \
>   		tst-tls20mod-bad tst-tls21mod tst-dlmopen-dlerror-mod \
>   		tst-auxvalmod \
> -		tst-dlmopen-gethostbyname-mod \
> +		tst-dlmopen-gethostbyname-mod tst-ro-dynamic-mod \
>   
>   # Most modules build with _ISOMAC defined, but those filtered out
>   # depend on internal headers.
> @@ -1908,3 +1908,10 @@ $(objpfx)tst-getauxval-static.out: $(objpfx)tst-auxvalmod.so
>   tst-getauxval-static-ENV = LD_LIBRARY_PATH=$(objpfx):$(common-objpfx)
>   
>   $(objpfx)tst-dlmopen-gethostbyname.out: $(objpfx)tst-dlmopen-gethostbyname-mod.so
> +
> +$(objpfx)tst-ro-dynamic: $(objpfx)tst-ro-dynamic-mod.so
> +$(objpfx)tst-ro-dynamic-mod.so: $(objpfx)tst-ro-dynamic-mod.os \
> +  tst-ro-dynamic-mod.map
> +	$(LINK.o) -nostdlib -nostartfiles -shared -o $@ \
> +		-Wl,--script=tst-ro-dynamic-mod.map \
> +		$(objpfx)tst-ro-dynamic-mod.os

Adding new test and options.  OK.

> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 650e4edc35..4445c28ef3 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1149,6 +1149,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>   		 such a segment to avoid a crash later.  */
>   	      l->l_ld = (void *) ph->p_vaddr;
>   	      l->l_ldnum = ph->p_memsz / sizeof (ElfW(Dyn));
> +	      l->l_ld_readonly = (ph->p_flags & PF_W) == 0;

Mark read-only for DSOs.  OK.

>   	    }
>   	  break;
>   
> @@ -1292,7 +1293,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>     else
>       l->l_ld = (ElfW(Dyn) *) ((ElfW(Addr)) l->l_ld + l->l_addr);
>   
> -  elf_get_dynamic_info (l, NULL);
> +  elf_get_dynamic_info (l);
>   
>     /* Make sure we are not dlopen'ing an object that has the
>        DF_1_NOOPEN flag set, or a PIE object.  */
> diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> index d5bd2f31e9..2fb02d7276 100644
> --- a/elf/dl-reloc-static-pie.c
> +++ b/elf/dl-reloc-static-pie.c
> @@ -40,7 +40,17 @@ _dl_relocate_static_pie (void)
>   
>     /* Read our own dynamic section and fill in the info array.  */
>     main_map->l_ld = ((void *) main_map->l_addr + elf_machine_dynamic ());
> -  elf_get_dynamic_info (main_map, NULL);
> +
> +  const ElfW(Phdr) *ph, *phdr = GL(dl_phdr);
> +  size_t phnum = GL(dl_phnum);
> +  for (ph = phdr; ph < &phdr[phnum]; ++ph)
> +    if (ph->p_type == PT_DYNAMIC)
> +      {
> +	main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
> +	break;
> +      }
> +
> +  elf_get_dynamic_info (main_map);

Mark read-only for static-pie.  OK.

>   
>   # ifdef ELF_MACHINE_BEFORE_RTLD_RELOC
>     ELF_MACHINE_BEFORE_RTLD_RELOC (main_map->l_info);
> diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
> index d8ec32377d..4aa2058abf 100644
> --- a/elf/get-dynamic-info.h
> +++ b/elf/get-dynamic-info.h
> @@ -28,7 +28,7 @@ static
>   auto
>   #endif
>   inline void __attribute__ ((unused, always_inline))
> -elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
> +elf_get_dynamic_info (struct link_map *l)
>   {
>   #if __ELF_NATIVE_CLASS == 32
>     typedef Elf32_Word d_tag_utype;
> @@ -69,28 +69,15 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
>         info[i] = dyn;
>       }
>   
> -#define DL_RO_DYN_TEMP_CNT	8
> -
> -#ifndef DL_RO_DYN_SECTION
>     /* Don't adjust .dynamic unnecessarily.  */
> -  if (l->l_addr != 0)
> +  if (l->l_addr != 0 && dl_relocate_ld (l))
>       {
>         ElfW(Addr) l_addr = l->l_addr;
> -      int cnt = 0;
>   
>   # define ADJUST_DYN_INFO(tag) \
>         do								      \
>   	if (info[tag] != NULL)						      \
> -	  {								      \
> -	    if (temp)							      \
> -	      {								      \
> -		temp[cnt].d_tag = info[tag]->d_tag;			      \
> -		temp[cnt].d_un.d_ptr = info[tag]->d_un.d_ptr + l_addr;	      \
> -		info[tag] = temp + cnt++;				      \
> -	      }								      \
> -	    else							      \
> -	      info[tag]->d_un.d_ptr += l_addr;				      \
> -	  }								      \
> +         info[tag]->d_un.d_ptr += l_addr;				      \

Simplify by using the readonly flag.  OK.

>         while (0)
>   
>         ADJUST_DYN_INFO (DT_HASH);
> @@ -107,9 +94,7 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
>         ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
>         ADJUST_DYN_INFO (ADDRIDX (DT_GNU_HASH));
>   # undef ADJUST_DYN_INFO
> -      assert (cnt <= DL_RO_DYN_TEMP_CNT);
>       }
> -#endif
>     if (info[DT_PLTREL] != NULL)
>       {
>   #if ELF_MACHINE_NO_RELA
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 878e6480f4..88a78326c8 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -463,6 +463,7 @@ _dl_start_final (void *arg, struct dl_start_final_info *info)
>   #ifndef DONT_USE_BOOTSTRAP_MAP
>     GL(dl_rtld_map).l_addr = info->l.l_addr;
>     GL(dl_rtld_map).l_ld = info->l.l_ld;
> +  GL(dl_rtld_map).l_ld_readonly = info->l.l_ld_readonly;
>     memcpy (GL(dl_rtld_map).l_info, info->l.l_info,
>   	  sizeof GL(dl_rtld_map).l_info);
>     GL(dl_rtld_map).l_mach = info->l.l_mach;
> @@ -546,7 +547,7 @@ _dl_start (void *arg)
>   
>     /* Read our own dynamic section and fill in the info array.  */
>     bootstrap_map.l_ld = (void *) bootstrap_map.l_addr + elf_machine_dynamic ();
> -  elf_get_dynamic_info (&bootstrap_map, NULL);
> +  elf_get_dynamic_info (&bootstrap_map);
>   
>   #if NO_TLS_OFFSET != 0
>     bootstrap_map.l_tls_offset = NO_TLS_OFFSET;
> @@ -1468,6 +1469,7 @@ dl_main (const ElfW(Phdr) *phdr,
>   	/* This tells us where to find the dynamic section,
>   	   which tells us everything we need to do.  */
>   	main_map->l_ld = (void *) main_map->l_addr + ph->p_vaddr;
> +	main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
>   	break;
>         case PT_INTERP:
>   	/* This "interpreter segment" was used by the program loader to
> @@ -1613,7 +1615,7 @@ dl_main (const ElfW(Phdr) *phdr,
>     if (! rtld_is_main)
>       {
>         /* Extract the contents of the dynamic section for easy access.  */
> -      elf_get_dynamic_info (main_map, NULL);
> +      elf_get_dynamic_info (main_map);

Mark read-only for main executable.  OK.

>   
>         /* If the main map is libc.so, update the base namespace to
>   	 refer to this map.  If libc.so is loaded later, this happens
> diff --git a/elf/setup-vdso.h b/elf/setup-vdso.h
> index 86c491e49c..f44748bc98 100644
> --- a/elf/setup-vdso.h
> +++ b/elf/setup-vdso.h
> @@ -33,8 +33,6 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
>   				       0, LM_ID_BASE);
>     if (__glibc_likely (l != NULL))
>       {
> -      static ElfW(Dyn) dyn_temp[DL_RO_DYN_TEMP_CNT] attribute_relro;
> -
>         l->l_phdr = ((const void *) GLRO(dl_sysinfo_dso)
>   		   + GLRO(dl_sysinfo_dso)->e_phoff);
>         l->l_phnum = GLRO(dl_sysinfo_dso)->e_phnum;
> @@ -45,6 +43,7 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
>   	    {
>   	      l->l_ld = (void *) ph->p_vaddr;
>   	      l->l_ldnum = ph->p_memsz / sizeof (ElfW(Dyn));
> +	      l->l_ld_readonly = (ph->p_flags & PF_W) == 0;

Mark read-only for vdso.  OK.

>   	    }
>   	  else if (ph->p_type == PT_LOAD)
>   	    {
> @@ -65,7 +64,7 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
>         l->l_map_end += l->l_addr;
>         l->l_text_end += l->l_addr;
>         l->l_ld = (void *) ((ElfW(Addr)) l->l_ld + l->l_addr);
> -      elf_get_dynamic_info (l, dyn_temp);
> +      elf_get_dynamic_info (l);
>         _dl_setup_hash (l);
>         l->l_relocated = 1;
>   
> diff --git a/elf/tst-ro-dynamic-mod.c b/elf/tst-ro-dynamic-mod.c
> new file mode 100644
> index 0000000000..6d99925964
> --- /dev/null
> +++ b/elf/tst-ro-dynamic-mod.c
> @@ -0,0 +1,19 @@
> +/* Test case for DSO with readonly dynamic section.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +int foo = -1;
> diff --git a/elf/tst-ro-dynamic-mod.map b/elf/tst-ro-dynamic-mod.map
> new file mode 100644
> index 0000000000..2fe4a2998c
> --- /dev/null
> +++ b/elf/tst-ro-dynamic-mod.map
> @@ -0,0 +1,16 @@
> +SECTIONS
> +{
> + . = SIZEOF_HEADERS;
> + .dynamic : { *(.dynamic) } :text :dynamic
> + .rodata : { *(.data*) *(.bss*) } :text
> + /DISCARD/ : {
> +  *(.note.gnu.property)
> + }
> + .note : { *(.note.*) } :text :note
> +}
> +PHDRS
> +{
> + text PT_LOAD FLAGS(5) FILEHDR PHDRS;
> + dynamic PT_DYNAMIC FLAGS(4);
> + note PT_NOTE FLAGS(4);
> +}
> diff --git a/elf/tst-ro-dynamic.c b/elf/tst-ro-dynamic.c
> new file mode 100644
> index 0000000000..3a18f8789a
> --- /dev/null
> +++ b/elf/tst-ro-dynamic.c
> @@ -0,0 +1,31 @@
> +/* Test case for DSO with readonly dynamic section.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/check.h>
> +#include <support/test-driver.h>
> +
> +extern int foo;
> +
> +static int
> +do_test (void)
> +{
> +  TEST_COMPARE (foo, -1);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

New test.  OK.

> diff --git a/include/link.h b/include/link.h
> index 4af16cb596..c46aced9f7 100644
> --- a/include/link.h
> +++ b/include/link.h
> @@ -205,6 +205,7 @@ struct link_map
>       unsigned int l_free_initfini:1; /* Nonzero if l_initfini can be
>   				       freed, ie. not allocated with
>   				       the dummy malloc in ld.so.  */
> +    unsigned int l_ld_readonly:1; /* Nonzero if dynamic section is readonly.  */

Flag bit.  OK.

>   
>       /* NODELETE status of the map.  Only valid for maps of type
>          lt_loaded.  Lazy binding sets l_nodelete_active directly,
> @@ -342,6 +343,8 @@ struct link_map
>       unsigned long long int l_serial;
>     };
>   
> +#include <dl-relocate-ld.h>
> +
>   /* Information used by audit modules.  For most link maps, this data
>      immediate follows the link map in memory.  For the dynamic linker,
>      it is allocated separately.  See link_map_audit_state in
> diff --git a/sysdeps/generic/dl-relocate-ld.h b/sysdeps/generic/dl-relocate-ld.h
> new file mode 100644
> index 0000000000..5fae206db9
> --- /dev/null
> +++ b/sysdeps/generic/dl-relocate-ld.h
> @@ -0,0 +1,32 @@
> +/* Check if dynamic section should be relocated.  Generic version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_RELOCATE_LD_H
> +#define _DL_RELOCATE_LD_H
> +
> +/* Return true if dynamic section in the shared library L should be
> +   relocated.  */
> +
> +static inline bool
> +dl_relocate_ld (const struct link_map *l)
> +{
> +  /* Don't relocate dynamic section if it is readonly  */
> +  return !l->l_ld_readonly;
> +}

Depend on read-only flag for all architectures except those that 
override this definition.  OK.

> +
> +#endif /* _DL_RELOCATE_LD_H */
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index fd67871f4b..eafa308047 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -75,11 +75,8 @@ __BEGIN_DECLS
>     have to be accessed via the D_PTR macro.  The macro is needed since for
>     most architectures the entry is already relocated - but for some not
>     and we need to relocate at access time.  */
> -#ifdef DL_RO_DYN_SECTION
> -# define D_PTR(map, i) ((map)->i->d_un.d_ptr + (map)->l_addr)
> -#else
> -# define D_PTR(map, i) (map)->i->d_un.d_ptr
> -#endif
> +#define D_PTR(map, i) \
> +  ((map)->i->d_un.d_ptr + (dl_relocate_ld (map) ? 0 : (map)->l_addr))

Simplify D_PTR to add l_addr for read-only maps.

>   
>   /* Result of the lookup functions and how to retrieve the base address.  */
>   typedef struct link_map *lookup_t;
> diff --git a/sysdeps/mips/dl-relocate-ld.h b/sysdeps/mips/dl-relocate-ld.h
> new file mode 100644
> index 0000000000..0c18d9a567
> --- /dev/null
> +++ b/sysdeps/mips/dl-relocate-ld.h
> @@ -0,0 +1,32 @@
> +/* Check if dynamic section should be relocated.  MIPS version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_RELOCATE_LD_H
> +#define _DL_RELOCATE_LD_H
> +
> +/* Return true if dynamic section in the shared library L should be
> +   relocated.  */
> +
> +static inline bool
> +dl_relocate_ld (const struct link_map *l)
> +{
> +  /* Never relocate dynamic section.  */
> +  return false;
> +}

mips does not like relocating the dynamic section at all.  OK.

> +
> +#endif /* _DL_RELOCATE_LD_H */
> diff --git a/sysdeps/mips/ldsodefs.h b/sysdeps/mips/ldsodefs.h
> index 4db7c60e38..36fd09a8bd 100644
> --- a/sysdeps/mips/ldsodefs.h
> +++ b/sysdeps/mips/ldsodefs.h
> @@ -75,10 +75,6 @@ struct La_mips_64_retval;
>   					  struct La_mips_64_retval *,	    \
>   					  const char *);
>   
> -/* The MIPS ABI specifies that the dynamic section has to be read-only.  */
> -
> -#define DL_RO_DYN_SECTION 1
> -
>   #include_next <ldsodefs.h>
>   
>   /* The 64-bit MIPS ELF ABI uses an unusual reloc format.  Each
> diff --git a/sysdeps/riscv/dl-relocate-ld.h b/sysdeps/riscv/dl-relocate-ld.h
> new file mode 100644
> index 0000000000..10327454b1
> --- /dev/null
> +++ b/sysdeps/riscv/dl-relocate-ld.h
> @@ -0,0 +1,32 @@
> +/* Check if dynamic section should be relocated.  RISC-V version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_RELOCATE_LD_H
> +#define _DL_RELOCATE_LD_H
> +
> +/* Return true if dynamic section in the shared library L should be
> +   relocated.  */
> +
> +static inline bool
> +dl_relocate_ld (const struct link_map *l)
> +{
> +  /* Never relocate dynamic section for ABI compatibility.  */
> +  return false;
> +}

Neither does risc-v.  OK.

> +
> +#endif /* _DL_RELOCATE_LD_H */
> diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h
> index 0c696714a7..8947ffe4b5 100644
> --- a/sysdeps/riscv/ldsodefs.h
> +++ b/sysdeps/riscv/ldsodefs.h
> @@ -38,11 +38,6 @@ struct La_riscv_retval;
>   				       struct La_riscv_retval *,	\
>   				       const char *);
>   
> -/* Although the RISC-V ABI does not specify that the dynamic section has
> -   to be read-only, it needs to be kept for ABI compatibility.  */
> -
> -#define DL_RO_DYN_SECTION 1
> -
>   #include_next <ldsodefs.h>
>   
>   #endif
>
Florian Weimer Sept. 20, 2021, 8:53 a.m. UTC | #2
* H. J. Lu:

> We can't relocate entries in dynamic section if it is readonly:
>
> 1. Add a l_ld_readonly field to struct link_map to indicate if dynamic
> section is readonly and set it based on p_flags of PT_DYNAMIC segment.
> 2. Replace DL_RO_DYN_SECTION with dl_relocate_ld to decide if dynamic
> section should be relocated.
> 3. Remove DL_RO_DYN_TEMP_CNT.
> 4. Don't use a static dynamic section to make readonly dynamic section
> in vDSO writable.
> 5. Remove the temp argument from elf_get_dynamic_info.

I don't have any objections to this patch.

Thanks,
Florian
Andreas Schwab Sept. 20, 2021, 9:47 a.m. UTC | #3
This breaks aarch64.

gcc   -nostdlib -nostartfiles -shared -o /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/ld.so.new              \
          -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both -Wl,-z,force-bti,--fatal-warnings -Wl,-z,defs -Wl,-z,now  \
          /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/librtld.os -Wl,--version-script=/home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/ld.map            \
          -Wl,-soname=ld-linux-aarch64.so.1                     \
          -Wl,-defsym=_begin=0
/usr/lib64/gcc/aarch64-suse-linux/11/../../../../aarch64-suse-linux/bin/ld: /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/librtld.os: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:607: /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/ld.so] Error 1

Andreas.
Florian Weimer Sept. 20, 2021, 10:03 a.m. UTC | #4
* Andreas Schwab:

> This breaks aarch64.
>
> gcc   -nostdlib -nostartfiles -shared -o /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/ld.so.new              \
>           -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both -Wl,-z,force-bti,--fatal-warnings -Wl,-z,defs -Wl,-z,now  \
>           /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/librtld.os -Wl,--version-script=/home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/ld.map            \
>           -Wl,-soname=ld-linux-aarch64.so.1                     \
>           -Wl,-defsym=_begin=0
> /usr/lib64/gcc/aarch64-suse-linux/11/../../../../aarch64-suse-linux/bin/ld: /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/librtld.os: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
> collect2: error: ld returned 1 exit status
> make[2]: *** [Makefile:607: /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/ld.so] Error 1

This is rather odd because the patch does not change the set of linker
input files or its attributes.

Or is this about commit a93d9e03a31ec14405cb3a09aa95413b67067380
("Extend struct r_debug to support multiple namespaces [BZ #15971]")?

Thanks,
Florian
Andreas Schwab Sept. 20, 2021, 10:31 a.m. UTC | #5
On Sep 20 2021, Florian Weimer wrote:

> Or is this about commit a93d9e03a31ec14405cb3a09aa95413b67067380
> ("Extend struct r_debug to support multiple namespaces [BZ #15971]")?

Yes, that's the commit I meant indeed.

Andreas.
Florian Weimer Sept. 20, 2021, 10:32 a.m. UTC | #6
* Andreas Schwab:

> On Sep 20 2021, Florian Weimer wrote:
>
>> Or is this about commit a93d9e03a31ec14405cb3a09aa95413b67067380
>> ("Extend struct r_debug to support multiple namespaces [BZ #15971]")?
>
> Yes, that's the commit I meant indeed.

Okay, I'm testing a patch.

Thanks,
Florian
H.J. Lu Sept. 20, 2021, 2:19 p.m. UTC | #7
On Sun, Sep 19, 2021 at 8:28 PM Siddhesh Poyarekar
<siddhesh@sourceware.org> wrote:
>
> On 9/19/21 21:58, H.J. Lu wrote:
> > Changes in v4:
> >
> > 1. Change l_ld_readonly to bitfield.
> >
> > Changes in v3:
> >
> > 1. Remove DL_RO_DYN_TEMP_CNT.
> > 2. Update comments for dl_relocate_ld.
> >
> > Changes in v2:
> >
> > 1. Replace DL_RO_DYN_SECTION with dl_relocate_ld so that MIPS and RISC-V
> > won't relocate dynamic section even if it is writable.
> > 2. Add a run-time testcase.
> >
> > We can't relocate entries in dynamic section if it is readonly:
> >
> > 1. Add a l_ld_readonly field to struct link_map to indicate if dynamic
> > section is readonly and set it based on p_flags of PT_DYNAMIC segment.
> > 2. Replace DL_RO_DYN_SECTION with dl_relocate_ld to decide if dynamic
> > section should be relocated.
> > 3. Remove DL_RO_DYN_TEMP_CNT.
> > 4. Don't use a static dynamic section to make readonly dynamic section
> > in vDSO writable.
> > 5. Remove the temp argument from elf_get_dynamic_info.
>
> LGTM, thanks for doing this.  AFAICT you've addressed their concerns but
> please check with Florian and Carlos before committing.

Carlos, do you have any comments?

>
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
> > ---
> >   elf/Makefile                     | 11 +++++++++--
> >   elf/dl-load.c                    |  3 ++-
> >   elf/dl-reloc-static-pie.c        | 12 +++++++++++-
> >   elf/get-dynamic-info.h           | 21 +++------------------
> >   elf/rtld.c                       |  6 ++++--
> >   elf/setup-vdso.h                 |  5 ++---
> >   elf/tst-ro-dynamic-mod.c         | 19 +++++++++++++++++++
> >   elf/tst-ro-dynamic-mod.map       | 16 ++++++++++++++++
> >   elf/tst-ro-dynamic.c             | 31 +++++++++++++++++++++++++++++++
> >   include/link.h                   |  3 +++
> >   sysdeps/generic/dl-relocate-ld.h | 32 ++++++++++++++++++++++++++++++++
> >   sysdeps/generic/ldsodefs.h       |  7 ++-----
> >   sysdeps/mips/dl-relocate-ld.h    | 32 ++++++++++++++++++++++++++++++++
> >   sysdeps/mips/ldsodefs.h          |  4 ----
> >   sysdeps/riscv/dl-relocate-ld.h   | 32 ++++++++++++++++++++++++++++++++
> >   sysdeps/riscv/ldsodefs.h         |  5 -----
> >   16 files changed, 198 insertions(+), 41 deletions(-)
> >   create mode 100644 elf/tst-ro-dynamic-mod.c
> >   create mode 100644 elf/tst-ro-dynamic-mod.map
> >   create mode 100644 elf/tst-ro-dynamic.c
> >   create mode 100644 sysdeps/generic/dl-relocate-ld.h
> >   create mode 100644 sysdeps/mips/dl-relocate-ld.h
> >   create mode 100644 sysdeps/riscv/dl-relocate-ld.h
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index 9f3fadc37e..eeb47145fc 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -223,7 +223,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
> >        tst-tls-ie tst-tls-ie-dlmopen argv0test \
> >        tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
> >        tst-tls20 tst-tls21 tst-dlmopen-dlerror tst-dlmopen-gethostbyname \
> > -      tst-dl-is_dso
> > +      tst-dl-is_dso tst-ro-dynamic
> >   #    reldep9
> >   tests-internal += loadtest unload unload2 circleload1 \
> >        neededtest neededtest2 neededtest3 neededtest4 \
> > @@ -359,7 +359,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
> >               libmarkermod4-1 libmarkermod4-2 libmarkermod4-3 libmarkermod4-4 \
> >               tst-tls20mod-bad tst-tls21mod tst-dlmopen-dlerror-mod \
> >               tst-auxvalmod \
> > -             tst-dlmopen-gethostbyname-mod \
> > +             tst-dlmopen-gethostbyname-mod tst-ro-dynamic-mod \
> >
> >   # Most modules build with _ISOMAC defined, but those filtered out
> >   # depend on internal headers.
> > @@ -1908,3 +1908,10 @@ $(objpfx)tst-getauxval-static.out: $(objpfx)tst-auxvalmod.so
> >   tst-getauxval-static-ENV = LD_LIBRARY_PATH=$(objpfx):$(common-objpfx)
> >
> >   $(objpfx)tst-dlmopen-gethostbyname.out: $(objpfx)tst-dlmopen-gethostbyname-mod.so
> > +
> > +$(objpfx)tst-ro-dynamic: $(objpfx)tst-ro-dynamic-mod.so
> > +$(objpfx)tst-ro-dynamic-mod.so: $(objpfx)tst-ro-dynamic-mod.os \
> > +  tst-ro-dynamic-mod.map
> > +     $(LINK.o) -nostdlib -nostartfiles -shared -o $@ \
> > +             -Wl,--script=tst-ro-dynamic-mod.map \
> > +             $(objpfx)tst-ro-dynamic-mod.os
>
> Adding new test and options.  OK.
>
> > diff --git a/elf/dl-load.c b/elf/dl-load.c
> > index 650e4edc35..4445c28ef3 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -1149,6 +1149,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> >                such a segment to avoid a crash later.  */
> >             l->l_ld = (void *) ph->p_vaddr;
> >             l->l_ldnum = ph->p_memsz / sizeof (ElfW(Dyn));
> > +           l->l_ld_readonly = (ph->p_flags & PF_W) == 0;
>
> Mark read-only for DSOs.  OK.
>
> >           }
> >         break;
> >
> > @@ -1292,7 +1293,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> >     else
> >       l->l_ld = (ElfW(Dyn) *) ((ElfW(Addr)) l->l_ld + l->l_addr);
> >
> > -  elf_get_dynamic_info (l, NULL);
> > +  elf_get_dynamic_info (l);
> >
> >     /* Make sure we are not dlopen'ing an object that has the
> >        DF_1_NOOPEN flag set, or a PIE object.  */
> > diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> > index d5bd2f31e9..2fb02d7276 100644
> > --- a/elf/dl-reloc-static-pie.c
> > +++ b/elf/dl-reloc-static-pie.c
> > @@ -40,7 +40,17 @@ _dl_relocate_static_pie (void)
> >
> >     /* Read our own dynamic section and fill in the info array.  */
> >     main_map->l_ld = ((void *) main_map->l_addr + elf_machine_dynamic ());
> > -  elf_get_dynamic_info (main_map, NULL);
> > +
> > +  const ElfW(Phdr) *ph, *phdr = GL(dl_phdr);
> > +  size_t phnum = GL(dl_phnum);
> > +  for (ph = phdr; ph < &phdr[phnum]; ++ph)
> > +    if (ph->p_type == PT_DYNAMIC)
> > +      {
> > +     main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
> > +     break;
> > +      }
> > +
> > +  elf_get_dynamic_info (main_map);
>
> Mark read-only for static-pie.  OK.
>
> >
> >   # ifdef ELF_MACHINE_BEFORE_RTLD_RELOC
> >     ELF_MACHINE_BEFORE_RTLD_RELOC (main_map->l_info);
> > diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
> > index d8ec32377d..4aa2058abf 100644
> > --- a/elf/get-dynamic-info.h
> > +++ b/elf/get-dynamic-info.h
> > @@ -28,7 +28,7 @@ static
> >   auto
> >   #endif
> >   inline void __attribute__ ((unused, always_inline))
> > -elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
> > +elf_get_dynamic_info (struct link_map *l)
> >   {
> >   #if __ELF_NATIVE_CLASS == 32
> >     typedef Elf32_Word d_tag_utype;
> > @@ -69,28 +69,15 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
> >         info[i] = dyn;
> >       }
> >
> > -#define DL_RO_DYN_TEMP_CNT   8
> > -
> > -#ifndef DL_RO_DYN_SECTION
> >     /* Don't adjust .dynamic unnecessarily.  */
> > -  if (l->l_addr != 0)
> > +  if (l->l_addr != 0 && dl_relocate_ld (l))
> >       {
> >         ElfW(Addr) l_addr = l->l_addr;
> > -      int cnt = 0;
> >
> >   # define ADJUST_DYN_INFO(tag) \
> >         do                                                                  \
> >       if (info[tag] != NULL)                                                \
> > -       {                                                                   \
> > -         if (temp)                                                         \
> > -           {                                                               \
> > -             temp[cnt].d_tag = info[tag]->d_tag;                           \
> > -             temp[cnt].d_un.d_ptr = info[tag]->d_un.d_ptr + l_addr;        \
> > -             info[tag] = temp + cnt++;                                     \
> > -           }                                                               \
> > -         else                                                              \
> > -           info[tag]->d_un.d_ptr += l_addr;                                \
> > -       }                                                                   \
> > +         info[tag]->d_un.d_ptr += l_addr;                                  \
>
> Simplify by using the readonly flag.  OK.
>
> >         while (0)
> >
> >         ADJUST_DYN_INFO (DT_HASH);
> > @@ -107,9 +94,7 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
> >         ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
> >         ADJUST_DYN_INFO (ADDRIDX (DT_GNU_HASH));
> >   # undef ADJUST_DYN_INFO
> > -      assert (cnt <= DL_RO_DYN_TEMP_CNT);
> >       }
> > -#endif
> >     if (info[DT_PLTREL] != NULL)
> >       {
> >   #if ELF_MACHINE_NO_RELA
> > diff --git a/elf/rtld.c b/elf/rtld.c
> > index 878e6480f4..88a78326c8 100644
> > --- a/elf/rtld.c
> > +++ b/elf/rtld.c
> > @@ -463,6 +463,7 @@ _dl_start_final (void *arg, struct dl_start_final_info *info)
> >   #ifndef DONT_USE_BOOTSTRAP_MAP
> >     GL(dl_rtld_map).l_addr = info->l.l_addr;
> >     GL(dl_rtld_map).l_ld = info->l.l_ld;
> > +  GL(dl_rtld_map).l_ld_readonly = info->l.l_ld_readonly;
> >     memcpy (GL(dl_rtld_map).l_info, info->l.l_info,
> >         sizeof GL(dl_rtld_map).l_info);
> >     GL(dl_rtld_map).l_mach = info->l.l_mach;
> > @@ -546,7 +547,7 @@ _dl_start (void *arg)
> >
> >     /* Read our own dynamic section and fill in the info array.  */
> >     bootstrap_map.l_ld = (void *) bootstrap_map.l_addr + elf_machine_dynamic ();
> > -  elf_get_dynamic_info (&bootstrap_map, NULL);
> > +  elf_get_dynamic_info (&bootstrap_map);
> >
> >   #if NO_TLS_OFFSET != 0
> >     bootstrap_map.l_tls_offset = NO_TLS_OFFSET;
> > @@ -1468,6 +1469,7 @@ dl_main (const ElfW(Phdr) *phdr,
> >       /* This tells us where to find the dynamic section,
> >          which tells us everything we need to do.  */
> >       main_map->l_ld = (void *) main_map->l_addr + ph->p_vaddr;
> > +     main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
> >       break;
> >         case PT_INTERP:
> >       /* This "interpreter segment" was used by the program loader to
> > @@ -1613,7 +1615,7 @@ dl_main (const ElfW(Phdr) *phdr,
> >     if (! rtld_is_main)
> >       {
> >         /* Extract the contents of the dynamic section for easy access.  */
> > -      elf_get_dynamic_info (main_map, NULL);
> > +      elf_get_dynamic_info (main_map);
>
> Mark read-only for main executable.  OK.
>
> >
> >         /* If the main map is libc.so, update the base namespace to
> >        refer to this map.  If libc.so is loaded later, this happens
> > diff --git a/elf/setup-vdso.h b/elf/setup-vdso.h
> > index 86c491e49c..f44748bc98 100644
> > --- a/elf/setup-vdso.h
> > +++ b/elf/setup-vdso.h
> > @@ -33,8 +33,6 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
> >                                      0, LM_ID_BASE);
> >     if (__glibc_likely (l != NULL))
> >       {
> > -      static ElfW(Dyn) dyn_temp[DL_RO_DYN_TEMP_CNT] attribute_relro;
> > -
> >         l->l_phdr = ((const void *) GLRO(dl_sysinfo_dso)
> >                  + GLRO(dl_sysinfo_dso)->e_phoff);
> >         l->l_phnum = GLRO(dl_sysinfo_dso)->e_phnum;
> > @@ -45,6 +43,7 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
> >           {
> >             l->l_ld = (void *) ph->p_vaddr;
> >             l->l_ldnum = ph->p_memsz / sizeof (ElfW(Dyn));
> > +           l->l_ld_readonly = (ph->p_flags & PF_W) == 0;
>
> Mark read-only for vdso.  OK.
>
> >           }
> >         else if (ph->p_type == PT_LOAD)
> >           {
> > @@ -65,7 +64,7 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
> >         l->l_map_end += l->l_addr;
> >         l->l_text_end += l->l_addr;
> >         l->l_ld = (void *) ((ElfW(Addr)) l->l_ld + l->l_addr);
> > -      elf_get_dynamic_info (l, dyn_temp);
> > +      elf_get_dynamic_info (l);
> >         _dl_setup_hash (l);
> >         l->l_relocated = 1;
> >
> > diff --git a/elf/tst-ro-dynamic-mod.c b/elf/tst-ro-dynamic-mod.c
> > new file mode 100644
> > index 0000000000..6d99925964
> > --- /dev/null
> > +++ b/elf/tst-ro-dynamic-mod.c
> > @@ -0,0 +1,19 @@
> > +/* Test case for DSO with readonly dynamic section.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +int foo = -1;
> > diff --git a/elf/tst-ro-dynamic-mod.map b/elf/tst-ro-dynamic-mod.map
> > new file mode 100644
> > index 0000000000..2fe4a2998c
> > --- /dev/null
> > +++ b/elf/tst-ro-dynamic-mod.map
> > @@ -0,0 +1,16 @@
> > +SECTIONS
> > +{
> > + . = SIZEOF_HEADERS;
> > + .dynamic : { *(.dynamic) } :text :dynamic
> > + .rodata : { *(.data*) *(.bss*) } :text
> > + /DISCARD/ : {
> > +  *(.note.gnu.property)
> > + }
> > + .note : { *(.note.*) } :text :note
> > +}
> > +PHDRS
> > +{
> > + text PT_LOAD FLAGS(5) FILEHDR PHDRS;
> > + dynamic PT_DYNAMIC FLAGS(4);
> > + note PT_NOTE FLAGS(4);
> > +}
> > diff --git a/elf/tst-ro-dynamic.c b/elf/tst-ro-dynamic.c
> > new file mode 100644
> > index 0000000000..3a18f8789a
> > --- /dev/null
> > +++ b/elf/tst-ro-dynamic.c
> > @@ -0,0 +1,31 @@
> > +/* Test case for DSO with readonly dynamic section.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <support/check.h>
> > +#include <support/test-driver.h>
> > +
> > +extern int foo;
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  TEST_COMPARE (foo, -1);
> > +  return 0;
> > +}
> > +
> > +#include <support/test-driver.c>
>
> New test.  OK.
>
> > diff --git a/include/link.h b/include/link.h
> > index 4af16cb596..c46aced9f7 100644
> > --- a/include/link.h
> > +++ b/include/link.h
> > @@ -205,6 +205,7 @@ struct link_map
> >       unsigned int l_free_initfini:1; /* Nonzero if l_initfini can be
> >                                      freed, ie. not allocated with
> >                                      the dummy malloc in ld.so.  */
> > +    unsigned int l_ld_readonly:1; /* Nonzero if dynamic section is readonly.  */
>
> Flag bit.  OK.
>
> >
> >       /* NODELETE status of the map.  Only valid for maps of type
> >          lt_loaded.  Lazy binding sets l_nodelete_active directly,
> > @@ -342,6 +343,8 @@ struct link_map
> >       unsigned long long int l_serial;
> >     };
> >
> > +#include <dl-relocate-ld.h>
> > +
> >   /* Information used by audit modules.  For most link maps, this data
> >      immediate follows the link map in memory.  For the dynamic linker,
> >      it is allocated separately.  See link_map_audit_state in
> > diff --git a/sysdeps/generic/dl-relocate-ld.h b/sysdeps/generic/dl-relocate-ld.h
> > new file mode 100644
> > index 0000000000..5fae206db9
> > --- /dev/null
> > +++ b/sysdeps/generic/dl-relocate-ld.h
> > @@ -0,0 +1,32 @@
> > +/* Check if dynamic section should be relocated.  Generic version.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _DL_RELOCATE_LD_H
> > +#define _DL_RELOCATE_LD_H
> > +
> > +/* Return true if dynamic section in the shared library L should be
> > +   relocated.  */
> > +
> > +static inline bool
> > +dl_relocate_ld (const struct link_map *l)
> > +{
> > +  /* Don't relocate dynamic section if it is readonly  */
> > +  return !l->l_ld_readonly;
> > +}
>
> Depend on read-only flag for all architectures except those that
> override this definition.  OK.
>
> > +
> > +#endif /* _DL_RELOCATE_LD_H */
> > diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> > index fd67871f4b..eafa308047 100644
> > --- a/sysdeps/generic/ldsodefs.h
> > +++ b/sysdeps/generic/ldsodefs.h
> > @@ -75,11 +75,8 @@ __BEGIN_DECLS
> >     have to be accessed via the D_PTR macro.  The macro is needed since for
> >     most architectures the entry is already relocated - but for some not
> >     and we need to relocate at access time.  */
> > -#ifdef DL_RO_DYN_SECTION
> > -# define D_PTR(map, i) ((map)->i->d_un.d_ptr + (map)->l_addr)
> > -#else
> > -# define D_PTR(map, i) (map)->i->d_un.d_ptr
> > -#endif
> > +#define D_PTR(map, i) \
> > +  ((map)->i->d_un.d_ptr + (dl_relocate_ld (map) ? 0 : (map)->l_addr))
>
> Simplify D_PTR to add l_addr for read-only maps.
>
> >
> >   /* Result of the lookup functions and how to retrieve the base address.  */
> >   typedef struct link_map *lookup_t;
> > diff --git a/sysdeps/mips/dl-relocate-ld.h b/sysdeps/mips/dl-relocate-ld.h
> > new file mode 100644
> > index 0000000000..0c18d9a567
> > --- /dev/null
> > +++ b/sysdeps/mips/dl-relocate-ld.h
> > @@ -0,0 +1,32 @@
> > +/* Check if dynamic section should be relocated.  MIPS version.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _DL_RELOCATE_LD_H
> > +#define _DL_RELOCATE_LD_H
> > +
> > +/* Return true if dynamic section in the shared library L should be
> > +   relocated.  */
> > +
> > +static inline bool
> > +dl_relocate_ld (const struct link_map *l)
> > +{
> > +  /* Never relocate dynamic section.  */
> > +  return false;
> > +}
>
> mips does not like relocating the dynamic section at all.  OK.
>
> > +
> > +#endif /* _DL_RELOCATE_LD_H */
> > diff --git a/sysdeps/mips/ldsodefs.h b/sysdeps/mips/ldsodefs.h
> > index 4db7c60e38..36fd09a8bd 100644
> > --- a/sysdeps/mips/ldsodefs.h
> > +++ b/sysdeps/mips/ldsodefs.h
> > @@ -75,10 +75,6 @@ struct La_mips_64_retval;
> >                                         struct La_mips_64_retval *,       \
> >                                         const char *);
> >
> > -/* The MIPS ABI specifies that the dynamic section has to be read-only.  */
> > -
> > -#define DL_RO_DYN_SECTION 1
> > -
> >   #include_next <ldsodefs.h>
> >
> >   /* The 64-bit MIPS ELF ABI uses an unusual reloc format.  Each
> > diff --git a/sysdeps/riscv/dl-relocate-ld.h b/sysdeps/riscv/dl-relocate-ld.h
> > new file mode 100644
> > index 0000000000..10327454b1
> > --- /dev/null
> > +++ b/sysdeps/riscv/dl-relocate-ld.h
> > @@ -0,0 +1,32 @@
> > +/* Check if dynamic section should be relocated.  RISC-V version.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _DL_RELOCATE_LD_H
> > +#define _DL_RELOCATE_LD_H
> > +
> > +/* Return true if dynamic section in the shared library L should be
> > +   relocated.  */
> > +
> > +static inline bool
> > +dl_relocate_ld (const struct link_map *l)
> > +{
> > +  /* Never relocate dynamic section for ABI compatibility.  */
> > +  return false;
> > +}
>
> Neither does risc-v.  OK.
>
> > +
> > +#endif /* _DL_RELOCATE_LD_H */
> > diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h
> > index 0c696714a7..8947ffe4b5 100644
> > --- a/sysdeps/riscv/ldsodefs.h
> > +++ b/sysdeps/riscv/ldsodefs.h
> > @@ -38,11 +38,6 @@ struct La_riscv_retval;
> >                                      struct La_riscv_retval *,        \
> >                                      const char *);
> >
> > -/* Although the RISC-V ABI does not specify that the dynamic section has
> > -   to be read-only, it needs to be kept for ABI compatibility.  */
> > -
> > -#define DL_RO_DYN_SECTION 1
> > -
> >   #include_next <ldsodefs.h>
> >
> >   #endif
> >
H.J. Lu Sept. 22, 2021, 6 p.m. UTC | #8
On Mon, Sep 20, 2021 at 1:54 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > We can't relocate entries in dynamic section if it is readonly:
> >
> > 1. Add a l_ld_readonly field to struct link_map to indicate if dynamic
> > section is readonly and set it based on p_flags of PT_DYNAMIC segment.
> > 2. Replace DL_RO_DYN_SECTION with dl_relocate_ld to decide if dynamic
> > section should be relocated.
> > 3. Remove DL_RO_DYN_TEMP_CNT.
> > 4. Don't use a static dynamic section to make readonly dynamic section
> > in vDSO writable.
> > 5. Remove the temp argument from elf_get_dynamic_info.
>
> I don't have any objections to this patch.
>
> Thanks,
> Florian
>

I am checking in this patch now.
Florian Weimer Sept. 23, 2021, 10:22 a.m. UTC | #9
* H. J. Lu:

> We can't relocate entries in dynamic section if it is readonly:
>
> 1. Add a l_ld_readonly field to struct link_map to indicate if dynamic
> section is readonly and set it based on p_flags of PT_DYNAMIC segment.
> 2. Replace DL_RO_DYN_SECTION with dl_relocate_ld to decide if dynamic
> section should be relocated.
> 3. Remove DL_RO_DYN_TEMP_CNT.
> 4. Don't use a static dynamic section to make readonly dynamic section
> in vDSO writable.
> 5. Remove the temp argument from elf_get_dynamic_info.

Sorry, this does not appear to work: For !DONT_USE_BOOTSTRAP_MAP,
bootstrap_map.l_ld_readonly is not initialized during the initial
self-relocation.  This is visible as a valgrind failure on
powerpc64le-linux-gnu:

+ /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2 --library-path /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64 /usr/bin/valgrind --error-exitcode=1 /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2 --library-path /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64 /usr/bin/true
==1924587== Memcheck, a memory error detector
==1924587== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1924587== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==1924587== Command: /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2 --library-path /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64 /usr/bin/true
==1924587== 
==1924587== Conditional jump or move depends on uninitialised value(s)
==1924587==    at 0x191620: elf_get_dynamic_info (get-dynamic-info.h:73)
==1924587==    by 0x191620: _dl_start (rtld.c:550)
==1924587==    by 0x190677: (below main) (in /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2)
==1924587== 

For DONT_USE_BOOTSTRAP_MAP, bootstrap_map.l_ld_readonly is always zero,
which is incorrect for RISC-V (which is PI_STATIC_AND_HIDDEN and thus
DONT_USE_BOOTSTRAP_MAP).

I don't see how to fix this without bringing back DL_RO_DYN_SECTION.

Thanks,
Florian
H.J. Lu Sept. 23, 2021, 3:47 p.m. UTC | #10
On Thu, Sep 23, 2021 at 3:22 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > We can't relocate entries in dynamic section if it is readonly:
> >
> > 1. Add a l_ld_readonly field to struct link_map to indicate if dynamic
> > section is readonly and set it based on p_flags of PT_DYNAMIC segment.
> > 2. Replace DL_RO_DYN_SECTION with dl_relocate_ld to decide if dynamic
> > section should be relocated.
> > 3. Remove DL_RO_DYN_TEMP_CNT.
> > 4. Don't use a static dynamic section to make readonly dynamic section
> > in vDSO writable.
> > 5. Remove the temp argument from elf_get_dynamic_info.
>
> Sorry, this does not appear to work: For !DONT_USE_BOOTSTRAP_MAP,
> bootstrap_map.l_ld_readonly is not initialized during the initial
> self-relocation.  This is visible as a valgrind failure on
> powerpc64le-linux-gnu:
>
> + /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2 --library-path /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64 /usr/bin/valgrind --error-exitcode=1 /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2 --library-path /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64 /usr/bin/true
> ==1924587== Memcheck, a memory error detector
> ==1924587== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> ==1924587== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
> ==1924587== Command: /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2 --library-path /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64 /usr/bin/true
> ==1924587==
> ==1924587== Conditional jump or move depends on uninitialised value(s)
> ==1924587==    at 0x191620: elf_get_dynamic_info (get-dynamic-info.h:73)
> ==1924587==    by 0x191620: _dl_start (rtld.c:550)
> ==1924587==    by 0x190677: (below main) (in /builddir/build/BUILDROOT/glibc-2.34.9000-9.fc36.ppc64le/lib64/ld64.so.2)
> ==1924587==
>
> For DONT_USE_BOOTSTRAP_MAP, bootstrap_map.l_ld_readonly is always zero,
> which is incorrect for RISC-V (which is PI_STATIC_AND_HIDDEN and thus
> DONT_USE_BOOTSTRAP_MAP).
>
> I don't see how to fix this without bringing back DL_RO_DYN_SECTION.

We can define DL_RO_DYN_SECTION and do

diff --git a/elf/rtld.c b/elf/rtld.c
index 8d2bba3d43..83adf1c5f5 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -547,6 +547,7 @@ _dl_start (void *arg)

   /* Read our own dynamic section and fill in the info array.  */
   bootstrap_map.l_ld = (void *) bootstrap_map.l_addr + elf_machine_dynamic ();
+  bootstrap_map.l_ld_readonly = DL_RO_DYN_SECTION;
   elf_get_dynamic_info (&bootstrap_map);

 #if NO_TLS_OFFSET != 0

Will this work?
Florian Weimer Sept. 23, 2021, 4:02 p.m. UTC | #11
* H. J. Lu:

> We can define DL_RO_DYN_SECTION and do
>
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 8d2bba3d43..83adf1c5f5 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -547,6 +547,7 @@ _dl_start (void *arg)
>
>    /* Read our own dynamic section and fill in the info array.  */
>    bootstrap_map.l_ld = (void *) bootstrap_map.l_addr + elf_machine_dynamic ();
> +  bootstrap_map.l_ld_readonly = DL_RO_DYN_SECTION;
>    elf_get_dynamic_info (&bootstrap_map);
>
>  #if NO_TLS_OFFSET != 0
>
> Will this work?

I think it should.  We can use DL_RO_DYN_SECTION to define a single,
generic version of dl_relocate_ld, too.

Thanks,
Florian
H.J. Lu Sept. 23, 2021, 4:15 p.m. UTC | #12
On Thu, Sep 23, 2021 at 9:02 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > We can define DL_RO_DYN_SECTION and do
> >
> > diff --git a/elf/rtld.c b/elf/rtld.c
> > index 8d2bba3d43..83adf1c5f5 100644
> > --- a/elf/rtld.c
> > +++ b/elf/rtld.c
> > @@ -547,6 +547,7 @@ _dl_start (void *arg)
> >
> >    /* Read our own dynamic section and fill in the info array.  */
> >    bootstrap_map.l_ld = (void *) bootstrap_map.l_addr + elf_machine_dynamic ();
> > +  bootstrap_map.l_ld_readonly = DL_RO_DYN_SECTION;
> >    elf_get_dynamic_info (&bootstrap_map);
> >
> >  #if NO_TLS_OFFSET != 0
> >
> > Will this work?
>
> I think it should.  We can use DL_RO_DYN_SECTION to define a single,
> generic version of dl_relocate_ld, too.

Here is the patch:

https://sourceware.org/pipermail/libc-alpha/2021-September/131294.html
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 9f3fadc37e..eeb47145fc 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -223,7 +223,7 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
 	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
 	 tst-tls20 tst-tls21 tst-dlmopen-dlerror tst-dlmopen-gethostbyname \
-	 tst-dl-is_dso
+	 tst-dl-is_dso tst-ro-dynamic
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
@@ -359,7 +359,7 @@  modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		libmarkermod4-1 libmarkermod4-2 libmarkermod4-3 libmarkermod4-4 \
 		tst-tls20mod-bad tst-tls21mod tst-dlmopen-dlerror-mod \
 		tst-auxvalmod \
-		tst-dlmopen-gethostbyname-mod \
+		tst-dlmopen-gethostbyname-mod tst-ro-dynamic-mod \
 
 # Most modules build with _ISOMAC defined, but those filtered out
 # depend on internal headers.
@@ -1908,3 +1908,10 @@  $(objpfx)tst-getauxval-static.out: $(objpfx)tst-auxvalmod.so
 tst-getauxval-static-ENV = LD_LIBRARY_PATH=$(objpfx):$(common-objpfx)
 
 $(objpfx)tst-dlmopen-gethostbyname.out: $(objpfx)tst-dlmopen-gethostbyname-mod.so
+
+$(objpfx)tst-ro-dynamic: $(objpfx)tst-ro-dynamic-mod.so
+$(objpfx)tst-ro-dynamic-mod.so: $(objpfx)tst-ro-dynamic-mod.os \
+  tst-ro-dynamic-mod.map
+	$(LINK.o) -nostdlib -nostartfiles -shared -o $@ \
+		-Wl,--script=tst-ro-dynamic-mod.map \
+		$(objpfx)tst-ro-dynamic-mod.os
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 650e4edc35..4445c28ef3 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1149,6 +1149,7 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 		 such a segment to avoid a crash later.  */
 	      l->l_ld = (void *) ph->p_vaddr;
 	      l->l_ldnum = ph->p_memsz / sizeof (ElfW(Dyn));
+	      l->l_ld_readonly = (ph->p_flags & PF_W) == 0;
 	    }
 	  break;
 
@@ -1292,7 +1293,7 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
   else
     l->l_ld = (ElfW(Dyn) *) ((ElfW(Addr)) l->l_ld + l->l_addr);
 
-  elf_get_dynamic_info (l, NULL);
+  elf_get_dynamic_info (l);
 
   /* Make sure we are not dlopen'ing an object that has the
      DF_1_NOOPEN flag set, or a PIE object.  */
diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
index d5bd2f31e9..2fb02d7276 100644
--- a/elf/dl-reloc-static-pie.c
+++ b/elf/dl-reloc-static-pie.c
@@ -40,7 +40,17 @@  _dl_relocate_static_pie (void)
 
   /* Read our own dynamic section and fill in the info array.  */
   main_map->l_ld = ((void *) main_map->l_addr + elf_machine_dynamic ());
-  elf_get_dynamic_info (main_map, NULL);
+
+  const ElfW(Phdr) *ph, *phdr = GL(dl_phdr);
+  size_t phnum = GL(dl_phnum);
+  for (ph = phdr; ph < &phdr[phnum]; ++ph)
+    if (ph->p_type == PT_DYNAMIC)
+      {
+	main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
+	break;
+      }
+
+  elf_get_dynamic_info (main_map);
 
 # ifdef ELF_MACHINE_BEFORE_RTLD_RELOC
   ELF_MACHINE_BEFORE_RTLD_RELOC (main_map->l_info);
diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
index d8ec32377d..4aa2058abf 100644
--- a/elf/get-dynamic-info.h
+++ b/elf/get-dynamic-info.h
@@ -28,7 +28,7 @@  static
 auto
 #endif
 inline void __attribute__ ((unused, always_inline))
-elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
+elf_get_dynamic_info (struct link_map *l)
 {
 #if __ELF_NATIVE_CLASS == 32
   typedef Elf32_Word d_tag_utype;
@@ -69,28 +69,15 @@  elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
       info[i] = dyn;
     }
 
-#define DL_RO_DYN_TEMP_CNT	8
-
-#ifndef DL_RO_DYN_SECTION
   /* Don't adjust .dynamic unnecessarily.  */
-  if (l->l_addr != 0)
+  if (l->l_addr != 0 && dl_relocate_ld (l))
     {
       ElfW(Addr) l_addr = l->l_addr;
-      int cnt = 0;
 
 # define ADJUST_DYN_INFO(tag) \
       do								      \
 	if (info[tag] != NULL)						      \
-	  {								      \
-	    if (temp)							      \
-	      {								      \
-		temp[cnt].d_tag = info[tag]->d_tag;			      \
-		temp[cnt].d_un.d_ptr = info[tag]->d_un.d_ptr + l_addr;	      \
-		info[tag] = temp + cnt++;				      \
-	      }								      \
-	    else							      \
-	      info[tag]->d_un.d_ptr += l_addr;				      \
-	  }								      \
+         info[tag]->d_un.d_ptr += l_addr;				      \
       while (0)
 
       ADJUST_DYN_INFO (DT_HASH);
@@ -107,9 +94,7 @@  elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
       ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
       ADJUST_DYN_INFO (ADDRIDX (DT_GNU_HASH));
 # undef ADJUST_DYN_INFO
-      assert (cnt <= DL_RO_DYN_TEMP_CNT);
     }
-#endif
   if (info[DT_PLTREL] != NULL)
     {
 #if ELF_MACHINE_NO_RELA
diff --git a/elf/rtld.c b/elf/rtld.c
index 878e6480f4..88a78326c8 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -463,6 +463,7 @@  _dl_start_final (void *arg, struct dl_start_final_info *info)
 #ifndef DONT_USE_BOOTSTRAP_MAP
   GL(dl_rtld_map).l_addr = info->l.l_addr;
   GL(dl_rtld_map).l_ld = info->l.l_ld;
+  GL(dl_rtld_map).l_ld_readonly = info->l.l_ld_readonly;
   memcpy (GL(dl_rtld_map).l_info, info->l.l_info,
 	  sizeof GL(dl_rtld_map).l_info);
   GL(dl_rtld_map).l_mach = info->l.l_mach;
@@ -546,7 +547,7 @@  _dl_start (void *arg)
 
   /* Read our own dynamic section and fill in the info array.  */
   bootstrap_map.l_ld = (void *) bootstrap_map.l_addr + elf_machine_dynamic ();
-  elf_get_dynamic_info (&bootstrap_map, NULL);
+  elf_get_dynamic_info (&bootstrap_map);
 
 #if NO_TLS_OFFSET != 0
   bootstrap_map.l_tls_offset = NO_TLS_OFFSET;
@@ -1468,6 +1469,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	/* This tells us where to find the dynamic section,
 	   which tells us everything we need to do.  */
 	main_map->l_ld = (void *) main_map->l_addr + ph->p_vaddr;
+	main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
 	break;
       case PT_INTERP:
 	/* This "interpreter segment" was used by the program loader to
@@ -1613,7 +1615,7 @@  dl_main (const ElfW(Phdr) *phdr,
   if (! rtld_is_main)
     {
       /* Extract the contents of the dynamic section for easy access.  */
-      elf_get_dynamic_info (main_map, NULL);
+      elf_get_dynamic_info (main_map);
 
       /* If the main map is libc.so, update the base namespace to
 	 refer to this map.  If libc.so is loaded later, this happens
diff --git a/elf/setup-vdso.h b/elf/setup-vdso.h
index 86c491e49c..f44748bc98 100644
--- a/elf/setup-vdso.h
+++ b/elf/setup-vdso.h
@@ -33,8 +33,6 @@  setup_vdso (struct link_map *main_map __attribute__ ((unused)),
 				       0, LM_ID_BASE);
   if (__glibc_likely (l != NULL))
     {
-      static ElfW(Dyn) dyn_temp[DL_RO_DYN_TEMP_CNT] attribute_relro;
-
       l->l_phdr = ((const void *) GLRO(dl_sysinfo_dso)
 		   + GLRO(dl_sysinfo_dso)->e_phoff);
       l->l_phnum = GLRO(dl_sysinfo_dso)->e_phnum;
@@ -45,6 +43,7 @@  setup_vdso (struct link_map *main_map __attribute__ ((unused)),
 	    {
 	      l->l_ld = (void *) ph->p_vaddr;
 	      l->l_ldnum = ph->p_memsz / sizeof (ElfW(Dyn));
+	      l->l_ld_readonly = (ph->p_flags & PF_W) == 0;
 	    }
 	  else if (ph->p_type == PT_LOAD)
 	    {
@@ -65,7 +64,7 @@  setup_vdso (struct link_map *main_map __attribute__ ((unused)),
       l->l_map_end += l->l_addr;
       l->l_text_end += l->l_addr;
       l->l_ld = (void *) ((ElfW(Addr)) l->l_ld + l->l_addr);
-      elf_get_dynamic_info (l, dyn_temp);
+      elf_get_dynamic_info (l);
       _dl_setup_hash (l);
       l->l_relocated = 1;
 
diff --git a/elf/tst-ro-dynamic-mod.c b/elf/tst-ro-dynamic-mod.c
new file mode 100644
index 0000000000..6d99925964
--- /dev/null
+++ b/elf/tst-ro-dynamic-mod.c
@@ -0,0 +1,19 @@ 
+/* Test case for DSO with readonly dynamic section.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+int foo = -1;
diff --git a/elf/tst-ro-dynamic-mod.map b/elf/tst-ro-dynamic-mod.map
new file mode 100644
index 0000000000..2fe4a2998c
--- /dev/null
+++ b/elf/tst-ro-dynamic-mod.map
@@ -0,0 +1,16 @@ 
+SECTIONS
+{
+ . = SIZEOF_HEADERS;
+ .dynamic : { *(.dynamic) } :text :dynamic
+ .rodata : { *(.data*) *(.bss*) } :text
+ /DISCARD/ : {
+  *(.note.gnu.property)
+ }
+ .note : { *(.note.*) } :text :note
+}
+PHDRS
+{
+ text PT_LOAD FLAGS(5) FILEHDR PHDRS;
+ dynamic PT_DYNAMIC FLAGS(4);
+ note PT_NOTE FLAGS(4);
+}
diff --git a/elf/tst-ro-dynamic.c b/elf/tst-ro-dynamic.c
new file mode 100644
index 0000000000..3a18f8789a
--- /dev/null
+++ b/elf/tst-ro-dynamic.c
@@ -0,0 +1,31 @@ 
+/* Test case for DSO with readonly dynamic section.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/check.h>
+#include <support/test-driver.h>
+
+extern int foo;
+
+static int
+do_test (void)
+{
+  TEST_COMPARE (foo, -1);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/include/link.h b/include/link.h
index 4af16cb596..c46aced9f7 100644
--- a/include/link.h
+++ b/include/link.h
@@ -205,6 +205,7 @@  struct link_map
     unsigned int l_free_initfini:1; /* Nonzero if l_initfini can be
 				       freed, ie. not allocated with
 				       the dummy malloc in ld.so.  */
+    unsigned int l_ld_readonly:1; /* Nonzero if dynamic section is readonly.  */
 
     /* NODELETE status of the map.  Only valid for maps of type
        lt_loaded.  Lazy binding sets l_nodelete_active directly,
@@ -342,6 +343,8 @@  struct link_map
     unsigned long long int l_serial;
   };
 
+#include <dl-relocate-ld.h>
+
 /* Information used by audit modules.  For most link maps, this data
    immediate follows the link map in memory.  For the dynamic linker,
    it is allocated separately.  See link_map_audit_state in
diff --git a/sysdeps/generic/dl-relocate-ld.h b/sysdeps/generic/dl-relocate-ld.h
new file mode 100644
index 0000000000..5fae206db9
--- /dev/null
+++ b/sysdeps/generic/dl-relocate-ld.h
@@ -0,0 +1,32 @@ 
+/* Check if dynamic section should be relocated.  Generic version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_RELOCATE_LD_H
+#define _DL_RELOCATE_LD_H
+
+/* Return true if dynamic section in the shared library L should be
+   relocated.  */
+
+static inline bool
+dl_relocate_ld (const struct link_map *l)
+{
+  /* Don't relocate dynamic section if it is readonly  */
+  return !l->l_ld_readonly;
+}
+
+#endif /* _DL_RELOCATE_LD_H */
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index fd67871f4b..eafa308047 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -75,11 +75,8 @@  __BEGIN_DECLS
   have to be accessed via the D_PTR macro.  The macro is needed since for
   most architectures the entry is already relocated - but for some not
   and we need to relocate at access time.  */
-#ifdef DL_RO_DYN_SECTION
-# define D_PTR(map, i) ((map)->i->d_un.d_ptr + (map)->l_addr)
-#else
-# define D_PTR(map, i) (map)->i->d_un.d_ptr
-#endif
+#define D_PTR(map, i) \
+  ((map)->i->d_un.d_ptr + (dl_relocate_ld (map) ? 0 : (map)->l_addr))
 
 /* Result of the lookup functions and how to retrieve the base address.  */
 typedef struct link_map *lookup_t;
diff --git a/sysdeps/mips/dl-relocate-ld.h b/sysdeps/mips/dl-relocate-ld.h
new file mode 100644
index 0000000000..0c18d9a567
--- /dev/null
+++ b/sysdeps/mips/dl-relocate-ld.h
@@ -0,0 +1,32 @@ 
+/* Check if dynamic section should be relocated.  MIPS version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_RELOCATE_LD_H
+#define _DL_RELOCATE_LD_H
+
+/* Return true if dynamic section in the shared library L should be
+   relocated.  */
+
+static inline bool
+dl_relocate_ld (const struct link_map *l)
+{
+  /* Never relocate dynamic section.  */
+  return false;
+}
+
+#endif /* _DL_RELOCATE_LD_H */
diff --git a/sysdeps/mips/ldsodefs.h b/sysdeps/mips/ldsodefs.h
index 4db7c60e38..36fd09a8bd 100644
--- a/sysdeps/mips/ldsodefs.h
+++ b/sysdeps/mips/ldsodefs.h
@@ -75,10 +75,6 @@  struct La_mips_64_retval;
 					  struct La_mips_64_retval *,	    \
 					  const char *);
 
-/* The MIPS ABI specifies that the dynamic section has to be read-only.  */
-
-#define DL_RO_DYN_SECTION 1
-
 #include_next <ldsodefs.h>
 
 /* The 64-bit MIPS ELF ABI uses an unusual reloc format.  Each
diff --git a/sysdeps/riscv/dl-relocate-ld.h b/sysdeps/riscv/dl-relocate-ld.h
new file mode 100644
index 0000000000..10327454b1
--- /dev/null
+++ b/sysdeps/riscv/dl-relocate-ld.h
@@ -0,0 +1,32 @@ 
+/* Check if dynamic section should be relocated.  RISC-V version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_RELOCATE_LD_H
+#define _DL_RELOCATE_LD_H
+
+/* Return true if dynamic section in the shared library L should be
+   relocated.  */
+
+static inline bool
+dl_relocate_ld (const struct link_map *l)
+{
+  /* Never relocate dynamic section for ABI compatibility.  */
+  return false;
+}
+
+#endif /* _DL_RELOCATE_LD_H */
diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h
index 0c696714a7..8947ffe4b5 100644
--- a/sysdeps/riscv/ldsodefs.h
+++ b/sysdeps/riscv/ldsodefs.h
@@ -38,11 +38,6 @@  struct La_riscv_retval;
 				       struct La_riscv_retval *,	\
 				       const char *);
 
-/* Although the RISC-V ABI does not specify that the dynamic section has
-   to be read-only, it needs to be kept for ABI compatibility.  */
-
-#define DL_RO_DYN_SECTION 1
-
 #include_next <ldsodefs.h>
 
 #endif