diff mbox series

[v2] ld.so: Initialize bootstrap_map.l_ld_readonly [BZ #28340]

Message ID 20210928154754.1485726-1-hjl.tools@gmail.com
State Superseded
Headers
Series [v2] ld.so: Initialize bootstrap_map.l_ld_readonly [BZ #28340] |

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. 28, 2021, 3:47 p.m. UTC 
  1. Define DL_RO_DYN_SECTION to initalize bootstrap_map.l_ld_readonly
before calling elf_get_dynamic_info to get dynamic info in bootstrap_map,
2. Define a single

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 || DL_RO_DYN_SECTION);
}

This updates BZ #28340 fix.
---
 elf/rtld.c                       |  1 +
 sysdeps/generic/dl-relocate-ld.h | 11 ++---------
 sysdeps/generic/ldsodefs.h       | 10 ++++++++++
 sysdeps/mips/dl-relocate-ld.h    | 11 ++---------
 sysdeps/riscv/dl-relocate-ld.h   | 11 ++---------
 5 files changed, 17 insertions(+), 27 deletions(-)

Comments

Carlos O'Donell Oct. 18, 2021, 4:22 p.m. UTC | #1 
On 9/28/21 11:47, H.J. Lu via Libc-alpha wrote:
> 1. Define DL_RO_DYN_SECTION to initalize bootstrap_map.l_ld_readonly
> before calling elf_get_dynamic_info to get dynamic info in bootstrap_map,
> 2. Define a single
> 
> 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 || DL_RO_DYN_SECTION);
> }
> 
> This updates BZ #28340 fix.

Please post v3 with update for Adhemveral's change.

I'm reviewing this right now with the obvious merge conflict fix.

> ---
>  elf/rtld.c                       |  1 +
>  sysdeps/generic/dl-relocate-ld.h | 11 ++---------
>  sysdeps/generic/ldsodefs.h       | 10 ++++++++++
>  sysdeps/mips/dl-relocate-ld.h    | 11 ++---------
>  sysdeps/riscv/dl-relocate-ld.h   | 11 ++---------
>  5 files changed, 17 insertions(+), 27 deletions(-)
> 
> 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;

This conflicts with Adhemerval's recent change to elf_get_dynamic_info.

Could you please update this patch?

>    elf_get_dynamic_info (&bootstrap_map);

I'm testing the obvious merge conflict fix.

>  
>  #if NO_TLS_OFFSET != 0
> diff --git a/sysdeps/generic/dl-relocate-ld.h b/sysdeps/generic/dl-relocate-ld.h
> index 5fae206db9..cfb86c2d6a 100644
> --- a/sysdeps/generic/dl-relocate-ld.h
> +++ b/sysdeps/generic/dl-relocate-ld.h
> @@ -19,14 +19,7 @@
>  #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;
> -}
> +/* The dynamic section is writable.  */
> +#define DL_RO_DYN_SECTION 0
>  
>  #endif /* _DL_RELOCATE_LD_H */
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index d49529da0d..668383ab80 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -69,6 +69,16 @@ __BEGIN_DECLS
>     `ElfW(TYPE)' is used in place of `Elf32_TYPE' or `Elf64_TYPE'.  */
>  #define ELFW(type)	_ElfW (ELF, __ELF_NATIVE_CLASS, type)
>  
> +/* 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 || DL_RO_DYN_SECTION);
> +}
> +
>  /* All references to the value of l_info[DT_PLTGOT],
>    l_info[DT_STRTAB], l_info[DT_SYMTAB], l_info[DT_RELA],
>    l_info[DT_REL], l_info[DT_JMPREL], and l_info[VERSYMIDX (DT_VERSYM)]
> diff --git a/sysdeps/mips/dl-relocate-ld.h b/sysdeps/mips/dl-relocate-ld.h
> index 0c18d9a567..376ad75dd1 100644
> --- a/sysdeps/mips/dl-relocate-ld.h
> +++ b/sysdeps/mips/dl-relocate-ld.h
> @@ -19,14 +19,7 @@
>  #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;
> -}
> +/* The dynamic section is readonly.  */
> +#define DL_RO_DYN_SECTION 1
>  
>  #endif /* _DL_RELOCATE_LD_H */
> diff --git a/sysdeps/riscv/dl-relocate-ld.h b/sysdeps/riscv/dl-relocate-ld.h
> index 10327454b1..2ab2b8ac6c 100644
> --- a/sysdeps/riscv/dl-relocate-ld.h
> +++ b/sysdeps/riscv/dl-relocate-ld.h
> @@ -19,14 +19,7 @@
>  #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;
> -}
> +/* The dynamic section is readonly for ABI compatibility.  */
> +#define DL_RO_DYN_SECTION 1
>  
>  #endif /* _DL_RELOCATE_LD_H */
>
H.J. Lu Oct. 18, 2021, 6:01 p.m. UTC | #2 
On Mon, Oct 18, 2021 at 9:22 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 9/28/21 11:47, H.J. Lu via Libc-alpha wrote:
> > 1. Define DL_RO_DYN_SECTION to initalize bootstrap_map.l_ld_readonly
> > before calling elf_get_dynamic_info to get dynamic info in bootstrap_map,
> > 2. Define a single
> >
> > 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 || DL_RO_DYN_SECTION);
> > }
> >
> > This updates BZ #28340 fix.
>
> Please post v3 with update for Adhemveral's change.
>
> I'm reviewing this right now with the obvious merge conflict fix.
>

I have sent the v3 patch.
H.J. Lu Oct. 18, 2021, 6:37 p.m. UTC | #3 
On Mon, Oct 18, 2021 at 11:01 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Oct 18, 2021 at 9:22 AM Carlos O'Donell <carlos@redhat.com> wrote:
> >
> > On 9/28/21 11:47, H.J. Lu via Libc-alpha wrote:
> > > 1. Define DL_RO_DYN_SECTION to initalize bootstrap_map.l_ld_readonly
> > > before calling elf_get_dynamic_info to get dynamic info in bootstrap_map,
> > > 2. Define a single
> > >
> > > 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 || DL_RO_DYN_SECTION);
> > > }
> > >
> > > This updates BZ #28340 fix.
> >
> > Please post v3 with update for Adhemveral's change.
> >
> > I'm reviewing this right now with the obvious merge conflict fix.
> >
>
> I have sent the v3 patch.
>

Please use the v4 patch:

https://sourceware.org/pipermail/libc-alpha/2021-October/132063.html
diff mbox series

Patch

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
diff --git a/sysdeps/generic/dl-relocate-ld.h b/sysdeps/generic/dl-relocate-ld.h
index 5fae206db9..cfb86c2d6a 100644
--- a/sysdeps/generic/dl-relocate-ld.h
+++ b/sysdeps/generic/dl-relocate-ld.h
@@ -19,14 +19,7 @@ 
 #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;
-}
+/* The dynamic section is writable.  */
+#define DL_RO_DYN_SECTION 0
 
 #endif /* _DL_RELOCATE_LD_H */
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index d49529da0d..668383ab80 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -69,6 +69,16 @@  __BEGIN_DECLS
    `ElfW(TYPE)' is used in place of `Elf32_TYPE' or `Elf64_TYPE'.  */
 #define ELFW(type)	_ElfW (ELF, __ELF_NATIVE_CLASS, type)
 
+/* 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 || DL_RO_DYN_SECTION);
+}
+
 /* All references to the value of l_info[DT_PLTGOT],
   l_info[DT_STRTAB], l_info[DT_SYMTAB], l_info[DT_RELA],
   l_info[DT_REL], l_info[DT_JMPREL], and l_info[VERSYMIDX (DT_VERSYM)]
diff --git a/sysdeps/mips/dl-relocate-ld.h b/sysdeps/mips/dl-relocate-ld.h
index 0c18d9a567..376ad75dd1 100644
--- a/sysdeps/mips/dl-relocate-ld.h
+++ b/sysdeps/mips/dl-relocate-ld.h
@@ -19,14 +19,7 @@ 
 #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;
-}
+/* The dynamic section is readonly.  */
+#define DL_RO_DYN_SECTION 1
 
 #endif /* _DL_RELOCATE_LD_H */
diff --git a/sysdeps/riscv/dl-relocate-ld.h b/sysdeps/riscv/dl-relocate-ld.h
index 10327454b1..2ab2b8ac6c 100644
--- a/sysdeps/riscv/dl-relocate-ld.h
+++ b/sysdeps/riscv/dl-relocate-ld.h
@@ -19,14 +19,7 @@ 
 #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;
-}
+/* The dynamic section is readonly for ABI compatibility.  */
+#define DL_RO_DYN_SECTION 1
 
 #endif /* _DL_RELOCATE_LD_H */