elf: Apply attribute_relro to pointers in elf/dl-minimal.c

Message ID 87tv3d2x91.fsf@oldenburg2.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer Feb. 26, 2020, 3:18 p.m. UTC
  The present code leaves the function pointers unprotected, but moves
some of the static functions into .data.rel.ro instead.  This causes
the linker to produce an allocatable, executable, writable section
and eventually an RWX load segment.  Not only do we really do not
want that, it also breaks valgrind because valgrind does not load
debuginfo from the mmap interceptor if all it sees are RX and RWX
mappings.

-----
 elf/dl-minimal.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
  

Comments

H.J. Lu Feb. 26, 2020, 3:51 p.m. UTC | #1
On Wed, Feb 26, 2020 at 7:18 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> The present code leaves the function pointers unprotected, but moves
> some of the static functions into .data.rel.ro instead.  This causes
> the linker to produce an allocatable, executable, writable section
> and eventually an RWX load segment.  Not only do we really do not
> want that, it also breaks valgrind because valgrind does not load
> debuginfo from the mmap interceptor if all it sees are RX and RWX
> mappings.
>
> -----
>  elf/dl-minimal.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/elf/dl-minimal.c b/elf/dl-minimal.c
> index c79ce23be4..7c64e24c87 100644
> --- a/elf/dl-minimal.c
> +++ b/elf/dl-minimal.c
> @@ -39,16 +39,16 @@
>    implementation below.  Before the final relocation,
>    __rtld_malloc_init_real is called to replace the pointers with the
>    real implementation.  */
> -__typeof (calloc) *__rtld_calloc;
> -__typeof (free) *__rtld_free;
> -__typeof (malloc) *__rtld_malloc;
> -__typeof (realloc) *__rtld_realloc;
> +__typeof (calloc) *__rtld_calloc attribute_relro;
> +__typeof (free) *__rtld_free attribute_relro;
> +__typeof (malloc) *__rtld_malloc attribute_relro;
> +__typeof (realloc) *__rtld_realloc attribute_relro;
>
>  /* Defined below.  */
> -static __typeof (calloc) rtld_calloc attribute_relro;
> -static __typeof (free) rtld_free attribute_relro;
> -static __typeof (malloc) rtld_malloc attribute_relro;
> -static __typeof (realloc) rtld_realloc attribute_relro;
> +static __typeof (calloc) rtld_calloc;
> +static __typeof (free) rtld_free;
> +static __typeof (malloc) rtld_malloc;
> +static __typeof (realloc) rtld_realloc;
>
>  void
>  __rtld_malloc_init_stubs (void)
>

LGTM.

Thanks.
  
Joseph Myers Feb. 26, 2020, 9:23 p.m. UTC | #2
This change (commit 758599bc9dcc5764e862bd9e1613c5d1e6efc5d3) breaks the 
build for alpha-linux-gnu with GCC and binutils mainline.  I get a series 
of errors of the form (this is in the "compilers" glibc build):

/scratch/jmyers/glibc/many10/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/librtld.os: 
in function `calloc':
/scratch/jmyers/glibc/many10/src/glibc/elf/../include/rtld-malloc.h:44:(.text+0xd98): 
relocation truncated to fit: GPREL16 against symbol `__rtld_calloc' 
defined in .data.rel.ro section in 
/scratch/jmyers/glibc/many10/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/librtld.os
/scratch/jmyers/glibc/many10/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/librtld.os: 
in function `malloc':
/scratch/jmyers/glibc/many10/src/glibc/elf/../include/rtld-malloc.h:56:(.text+0x2978): 
relocation truncated to fit: GPREL16 against symbol `__rtld_malloc' 
defined in .data.rel.ro section in 
/scratch/jmyers/glibc/many10/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/elf/librtld.os

https://sourceware.org/ml/libc-testresults/2020-q1/msg00270.html

Reverting to the previous glibc commit, while keeping other components the 
same, eliminates that build failure.

I don't know whether the problem lies in GCC, binutils or glibc (but 
https://sourceware.org/ml/libc-testresults/2020-q1/msg00271.html shows it 
doesn't appear with GCC 9 branch and binutils 2.34 branch).
  

Patch

diff --git a/elf/dl-minimal.c b/elf/dl-minimal.c
index c79ce23be4..7c64e24c87 100644
--- a/elf/dl-minimal.c
+++ b/elf/dl-minimal.c
@@ -39,16 +39,16 @@ 
   implementation below.  Before the final relocation,
   __rtld_malloc_init_real is called to replace the pointers with the
   real implementation.  */
-__typeof (calloc) *__rtld_calloc;
-__typeof (free) *__rtld_free;
-__typeof (malloc) *__rtld_malloc;
-__typeof (realloc) *__rtld_realloc;
+__typeof (calloc) *__rtld_calloc attribute_relro;
+__typeof (free) *__rtld_free attribute_relro;
+__typeof (malloc) *__rtld_malloc attribute_relro;
+__typeof (realloc) *__rtld_realloc attribute_relro;
 
 /* Defined below.  */
-static __typeof (calloc) rtld_calloc attribute_relro;
-static __typeof (free) rtld_free attribute_relro;
-static __typeof (malloc) rtld_malloc attribute_relro;
-static __typeof (realloc) rtld_realloc attribute_relro;
+static __typeof (calloc) rtld_calloc;
+static __typeof (free) rtld_free;
+static __typeof (malloc) rtld_malloc;
+static __typeof (realloc) rtld_realloc;
 
 void
 __rtld_malloc_init_stubs (void)