diff mbox series

[3/3] elf: Relocate libc.so early during startup and dlmopen (bug 31083)

Message ID 7d8d7ed3614d4779f1b20e3acb8a7a5642d260e6.1700829130.git.fweimer@redhat.com
State New
Headers show
Series Compatibility improvement for underlinked objects | expand

Commit Message

Florian Weimer Nov. 24, 2023, 12:56 p.m. UTC
This makes it more likely that objects without dependencies can
use IFUNC resolvers in libc.so.
---
 elf/Makefile          | 21 +++++++++++++++++++++
 elf/dl-open.c         | 11 +++++++++++
 elf/rtld.c            | 10 ++++++++--
 elf/tst-nodeps1-mod.c | 25 +++++++++++++++++++++++++
 elf/tst-nodeps1.c     | 23 +++++++++++++++++++++++
 elf/tst-nodeps2-mod.c |  1 +
 elf/tst-nodeps2.c     | 29 +++++++++++++++++++++++++++++
 7 files changed, 118 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-nodeps1-mod.c
 create mode 100644 elf/tst-nodeps1.c
 create mode 100644 elf/tst-nodeps2-mod.c
 create mode 100644 elf/tst-nodeps2.c

Comments

Carlos O'Donell Nov. 24, 2023, 5:04 p.m. UTC | #1
On 11/24/23 07:56, Florian Weimer wrote:
> This makes it more likely that objects without dependencies can
> use IFUNC resolvers in libc.so.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  elf/Makefile          | 21 +++++++++++++++++++++
>  elf/dl-open.c         | 11 +++++++++++
>  elf/rtld.c            | 10 ++++++++--
>  elf/tst-nodeps1-mod.c | 25 +++++++++++++++++++++++++
>  elf/tst-nodeps1.c     | 23 +++++++++++++++++++++++
>  elf/tst-nodeps2-mod.c |  1 +
>  elf/tst-nodeps2.c     | 29 +++++++++++++++++++++++++++++
>  7 files changed, 118 insertions(+), 2 deletions(-)
>  create mode 100644 elf/tst-nodeps1-mod.c
>  create mode 100644 elf/tst-nodeps1.c
>  create mode 100644 elf/tst-nodeps2-mod.c
>  create mode 100644 elf/tst-nodeps2.c
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 3f7f89508e..afec7be084 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -433,6 +433,8 @@ tests += \
>    tst-nodelete-dlclose \
>    tst-nodelete-opened \
>    tst-nodelete2 \
> +  tst-nodeps1 \
> +  tst-nodeps2 \

OK. Two new test.

>    tst-noload \
>    tst-non-directory-path \
>    tst-null-argv \
> @@ -863,6 +865,8 @@ modules-names += \
>    tst-nodelete-dlclose-plugin \
>    tst-nodelete-opened-lib \
>    tst-nodelete2mod \
> +  tst-nodeps1-mod \
> +  tst-nodeps2-mod \

OK. Two new modules.

>    tst-non-directory-mod \
>    tst-null-argv-lib \
>    tst-p_alignmod-base \
> @@ -1030,6 +1034,8 @@ modules-names-nobuild += \
>    tst-audit24bmod1 \
>    tst-audit24bmod2 \
>    tst-big-note-lib \
> +  tst-nodeps1-mod \
> +  tst-nodeps2-mod \

OK. Add modules.

>    tst-ro-dynamic-mod \
>    # modules-names-nobuild
>  
> @@ -3009,3 +3015,18 @@ tst-env-setuid-ARGS = -- $(host-test-program-cmd)
>  
>  # Reuse a module with a SONAME, to specific as the LD_PROFILE.
>  $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
> +
> +# The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
> +$(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os
> +	$(LINK.o) -nostartfiles -nostdlib -shared -o $@ $^

OK.

> +tst-nodeps1.so-no-z-defs = yes
> +# Link libc.so before the test module with the IFUNC resolver reference.
> +LDFLAGS-tst-nodeps1 = $(common-objpfx)libc.so $(objpfx)tst-nodeps1-mod.so
> +$(objpfx)tst-nodeps1: $(objpfx)tst-nodeps1-mod.so

OK.

> +# Reuse the tst-nodeps1 module.  Link libc.so before the test module
> +# with the IFUNC resolver reference.
> +$(objpfx)tst-nodeps2-mod.so: $(common-objpfx)libc.so \
> +  $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.os
> +	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^
> +$(objpfx)tst-nodeps2.out: \
> +  $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.so

OK.

> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index 417d2fb948..b748c278ac 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -708,6 +708,17 @@ dl_open_worker_begin (void *a)
>       them.  However, such relocation dependencies in IFUNC resolvers
>       are undefined anyway, so this is not a problem.  */
>  
> +  /* Ensure that libc is relocated first.  This helps with the
> +     execution of IFUNC resolvers in libc, and matters only to newly
> +     created dlmopen namespaces.  Do not do this for static dlopen
> +     because libc has relocations against ld.so, which may not have
> +     been relocated at this point.  */
> +#ifdef SHARED
> +  if (GL(dl_ns)[args->nsid].libc_map != NULL)
> +    _dl_open_relocate_one_object (args, r, GL(dl_ns)[args->nsid].libc_map,
> +				  reloc_mode, &relocation_in_progress);

OK. Relocate libc first... in the right namespace.

> +#endif
> +
>    for (unsigned int i = last; i-- > first; )
>      _dl_open_relocate_one_object (args, r, new->l_initfini[i], reloc_mode,
>  				  &relocation_in_progress);
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 0553c05edb..19bedcd4a6 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2272,11 +2272,17 @@ dl_main (const ElfW(Phdr) *phdr,
>       objects.  We do not re-relocate the dynamic linker itself in this
>       loop because that could result in the GOT entries for functions we
>       call being changed, and that would break us.  It is safe to relocate
> -     the dynamic linker out of order because it has no copy relocs (we
> -     know that because it is self-contained).  */
> +     the dynamic linker out of order because it has no copy relocations.
> +     Likewise for libc, which is relocated early to ensure that IFUNC
> +     resolvers in libc work.  */
>  
>    int consider_profiling = GLRO(dl_profile) != NULL;
>  
> +  if (GL(dl_ns)[LM_ID_BASE].libc_map != NULL)
> +    _dl_relocate_object (GL(dl_ns)[LM_ID_BASE].libc_map,
> +			 GL(dl_ns)[LM_ID_BASE].libc_map->l_scope,
> +			 GLRO(dl_lazy) ? RTLD_LAZY : 0, consider_profiling);

OK. Relocate libc early... always in the base namespace.

> +
>    /* If we are profiling we also must do lazy reloaction.  */
>    GLRO(dl_lazy) |= consider_profiling;
>  
> diff --git a/elf/tst-nodeps1-mod.c b/elf/tst-nodeps1-mod.c
> new file mode 100644
> index 0000000000..45c8e3c631
> --- /dev/null
> +++ b/elf/tst-nodeps1-mod.c
> @@ -0,0 +1,25 @@
> +/* Test module with no libc.so dependency and string function references.
> +   Copyright (C) 2023 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 <string.h>
> +
> +/* Some references to libc symbols which are likely to have IFUNC
> +   resolvers.  If they do not, this module does not exercise bug 31083.  */
> +void *memcpy_pointer = memcpy;
> +void *memmove_pointer = memmove;
> +void *memset_pointer = memset;

OK. I don't have a better answer here either, other than some internal access to the
IFUNC impl list and see what's in that list as a cross check here. This is good for now.

> diff --git a/elf/tst-nodeps1.c b/elf/tst-nodeps1.c
> new file mode 100644
> index 0000000000..1a8bde36cd
> --- /dev/null
> +++ b/elf/tst-nodeps1.c
> @@ -0,0 +1,23 @@
> +/* Test initially loaded module with implicit libc.so dependency (bug 31083).
> +   Copyright (C) 2023 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/>.  */
> +
> +/* Testing happens before main.  */
> +int
> +main (void)
> +{
> +}
> diff --git a/elf/tst-nodeps2-mod.c b/elf/tst-nodeps2-mod.c
> new file mode 100644
> index 0000000000..4913feee9b
> --- /dev/null
> +++ b/elf/tst-nodeps2-mod.c
> @@ -0,0 +1 @@
> +/* Empty test module which depends on tst-nodeps1-mod.so.  */
> diff --git a/elf/tst-nodeps2.c b/elf/tst-nodeps2.c
> new file mode 100644
> index 0000000000..0bdc8eeb8c
> --- /dev/null
> +++ b/elf/tst-nodeps2.c
> @@ -0,0 +1,29 @@
> +/* Test dlmopen with implicit libc.so dependency (bug 31083).
> +   Copyright (C) 2023 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/xdlfcn.h>
> +
> +static int
> +do_test (void)
> +{
> +  void *handle = xdlmopen (LM_ID_NEWLM, "tst-nodeps2-mod.so", RTLD_NOW);

OK. tst-nodeps2-mod.so -> {libc.so.6, tst-nodeps1-mod.so}

> +  xdlclose (handle);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
Stefan Liebler Dec. 13, 2023, 11:58 a.m. UTC | #2
Hi Florian,

can you please have a look at elf/tst-nodeps2. Does it also fail on your
side?

If I configure with --enable-hardcoded-path-in-tests,
it fails:
- on x86_64: Didn't expect signal from child: got `Aborted'
- on s390x: Didn't expect signal from child: got `Floating point exception'


On 24.11.23 18:04, Carlos O'Donell wrote:
> On 11/24/23 07:56, Florian Weimer wrote:
>> This makes it more likely that objects without dependencies can
>> use IFUNC resolvers in libc.so.
> 
> LGTM.
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 
>> ---
>>  elf/Makefile          | 21 +++++++++++++++++++++
>>  elf/dl-open.c         | 11 +++++++++++
>>  elf/rtld.c            | 10 ++++++++--
>>  elf/tst-nodeps1-mod.c | 25 +++++++++++++++++++++++++
>>  elf/tst-nodeps1.c     | 23 +++++++++++++++++++++++
>>  elf/tst-nodeps2-mod.c |  1 +
>>  elf/tst-nodeps2.c     | 29 +++++++++++++++++++++++++++++
>>  7 files changed, 118 insertions(+), 2 deletions(-)
>>  create mode 100644 elf/tst-nodeps1-mod.c
>>  create mode 100644 elf/tst-nodeps1.c
>>  create mode 100644 elf/tst-nodeps2-mod.c
>>  create mode 100644 elf/tst-nodeps2.c
>>
>> diff --git a/elf/Makefile b/elf/Makefile
>> index 3f7f89508e..afec7be084 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -433,6 +433,8 @@ tests += \
>>    tst-nodelete-dlclose \
>>    tst-nodelete-opened \
>>    tst-nodelete2 \
>> +  tst-nodeps1 \
>> +  tst-nodeps2 \
> 
> OK. Two new test.
> 
>>    tst-noload \
>>    tst-non-directory-path \
>>    tst-null-argv \
>> @@ -863,6 +865,8 @@ modules-names += \
>>    tst-nodelete-dlclose-plugin \
>>    tst-nodelete-opened-lib \
>>    tst-nodelete2mod \
>> +  tst-nodeps1-mod \
>> +  tst-nodeps2-mod \
> 
> OK. Two new modules.
> 
>>    tst-non-directory-mod \
>>    tst-null-argv-lib \
>>    tst-p_alignmod-base \
>> @@ -1030,6 +1034,8 @@ modules-names-nobuild += \
>>    tst-audit24bmod1 \
>>    tst-audit24bmod2 \
>>    tst-big-note-lib \
>> +  tst-nodeps1-mod \
>> +  tst-nodeps2-mod \
> 
> OK. Add modules.
> 
>>    tst-ro-dynamic-mod \
>>    # modules-names-nobuild
>>  
>> @@ -3009,3 +3015,18 @@ tst-env-setuid-ARGS = -- $(host-test-program-cmd)
>>  
>>  # Reuse a module with a SONAME, to specific as the LD_PROFILE.
>>  $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
>> +
>> +# The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
>> +$(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os
>> +	$(LINK.o) -nostartfiles -nostdlib -shared -o $@ $^
> 
> OK.
> 
>> +tst-nodeps1.so-no-z-defs = yes
>> +# Link libc.so before the test module with the IFUNC resolver reference.
>> +LDFLAGS-tst-nodeps1 = $(common-objpfx)libc.so $(objpfx)tst-nodeps1-mod.so
>> +$(objpfx)tst-nodeps1: $(objpfx)tst-nodeps1-mod.so
> 
> OK.
> 
>> +# Reuse the tst-nodeps1 module.  Link libc.so before the test module
>> +# with the IFUNC resolver reference.
>> +$(objpfx)tst-nodeps2-mod.so: $(common-objpfx)libc.so \
>> +  $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.os
>> +	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^
>> +$(objpfx)tst-nodeps2.out: \
>> +  $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.so
> 
> OK.
> 
>> diff --git a/elf/dl-open.c b/elf/dl-open.c
>> index 417d2fb948..b748c278ac 100644
>> --- a/elf/dl-open.c
>> +++ b/elf/dl-open.c
>> @@ -708,6 +708,17 @@ dl_open_worker_begin (void *a)
>>       them.  However, such relocation dependencies in IFUNC resolvers
>>       are undefined anyway, so this is not a problem.  */
>>  
>> +  /* Ensure that libc is relocated first.  This helps with the
>> +     execution of IFUNC resolvers in libc, and matters only to newly
>> +     created dlmopen namespaces.  Do not do this for static dlopen
>> +     because libc has relocations against ld.so, which may not have
>> +     been relocated at this point.  */
>> +#ifdef SHARED
>> +  if (GL(dl_ns)[args->nsid].libc_map != NULL)
>> +    _dl_open_relocate_one_object (args, r, GL(dl_ns)[args->nsid].libc_map,
>> +				  reloc_mode, &relocation_in_progress);
> 
> OK. Relocate libc first... in the right namespace.
> 
>> +#endif
>> +
>>    for (unsigned int i = last; i-- > first; )
>>      _dl_open_relocate_one_object (args, r, new->l_initfini[i], reloc_mode,
>>  				  &relocation_in_progress);
>> diff --git a/elf/rtld.c b/elf/rtld.c
>> index 0553c05edb..19bedcd4a6 100644
>> --- a/elf/rtld.c
>> +++ b/elf/rtld.c
>> @@ -2272,11 +2272,17 @@ dl_main (const ElfW(Phdr) *phdr,
>>       objects.  We do not re-relocate the dynamic linker itself in this
>>       loop because that could result in the GOT entries for functions we
>>       call being changed, and that would break us.  It is safe to relocate
>> -     the dynamic linker out of order because it has no copy relocs (we
>> -     know that because it is self-contained).  */
>> +     the dynamic linker out of order because it has no copy relocations.
>> +     Likewise for libc, which is relocated early to ensure that IFUNC
>> +     resolvers in libc work.  */
>>  
>>    int consider_profiling = GLRO(dl_profile) != NULL;
>>  
>> +  if (GL(dl_ns)[LM_ID_BASE].libc_map != NULL)
>> +    _dl_relocate_object (GL(dl_ns)[LM_ID_BASE].libc_map,
>> +			 GL(dl_ns)[LM_ID_BASE].libc_map->l_scope,
>> +			 GLRO(dl_lazy) ? RTLD_LAZY : 0, consider_profiling);
> 
> OK. Relocate libc early... always in the base namespace.
> 
>> +
>>    /* If we are profiling we also must do lazy reloaction.  */
>>    GLRO(dl_lazy) |= consider_profiling;
>>  
>> diff --git a/elf/tst-nodeps1-mod.c b/elf/tst-nodeps1-mod.c
>> new file mode 100644
>> index 0000000000..45c8e3c631
>> --- /dev/null
>> +++ b/elf/tst-nodeps1-mod.c
>> @@ -0,0 +1,25 @@
>> +/* Test module with no libc.so dependency and string function references.
>> +   Copyright (C) 2023 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 <string.h>
>> +
>> +/* Some references to libc symbols which are likely to have IFUNC
>> +   resolvers.  If they do not, this module does not exercise bug 31083.  */
>> +void *memcpy_pointer = memcpy;
>> +void *memmove_pointer = memmove;
>> +void *memset_pointer = memset;
> 
> OK. I don't have a better answer here either, other than some internal access to the
> IFUNC impl list and see what's in that list as a cross check here. This is good for now.
> 
>> diff --git a/elf/tst-nodeps1.c b/elf/tst-nodeps1.c
>> new file mode 100644
>> index 0000000000..1a8bde36cd
>> --- /dev/null
>> +++ b/elf/tst-nodeps1.c
>> @@ -0,0 +1,23 @@
>> +/* Test initially loaded module with implicit libc.so dependency (bug 31083).
>> +   Copyright (C) 2023 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/>.  */
>> +
>> +/* Testing happens before main.  */
>> +int
>> +main (void)
>> +{
>> +}
>> diff --git a/elf/tst-nodeps2-mod.c b/elf/tst-nodeps2-mod.c
>> new file mode 100644
>> index 0000000000..4913feee9b
>> --- /dev/null
>> +++ b/elf/tst-nodeps2-mod.c
>> @@ -0,0 +1 @@
>> +/* Empty test module which depends on tst-nodeps1-mod.so.  */
>> diff --git a/elf/tst-nodeps2.c b/elf/tst-nodeps2.c
>> new file mode 100644
>> index 0000000000..0bdc8eeb8c
>> --- /dev/null
>> +++ b/elf/tst-nodeps2.c
>> @@ -0,0 +1,29 @@
>> +/* Test dlmopen with implicit libc.so dependency (bug 31083).
>> +   Copyright (C) 2023 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/xdlfcn.h>
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  void *handle = xdlmopen (LM_ID_NEWLM, "tst-nodeps2-mod.so", RTLD_NOW);
> 
> OK. tst-nodeps2-mod.so -> {libc.so.6, tst-nodeps1-mod.so}
> 
>> +  xdlclose (handle);
>> +  return 0;
>> +}
>> +
>> +#include <support/test-driver.c>
>
Florian Weimer Dec. 13, 2023, 12:18 p.m. UTC | #3
* Stefan Liebler:

> Hi Florian,
>
> can you please have a look at elf/tst-nodeps2. Does it also fail on your
> side?
>
> If I configure with --enable-hardcoded-path-in-tests,
> it fails:
> - on x86_64: Didn't expect signal from child: got `Aborted'
> - on s390x: Didn't expect signal from child: got `Floating point exception'

Well, that's pretty worrisome, but it turns out to be a test issue.  I
think the reason for the crash is that the test escapes the test
environment during dlmopen:

     62632:     relocation processing: /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps2-mod.so
     62632:     object=/home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps2-mod.so [1]
     62632:      scope 0: /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps2-mod.so /lib64/libc.so.6 /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps1-mod.so /lib64/ld-linux-x86-64.so.2
     62632:     
     62632:     object=/lib64/libc.so.6 [1]
     62632:      scope 0: /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps2-mod.so /lib64/libc.so.6 /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps1-mod.so /lib64/ld-linux-x86-64.so.2
     62632:     
     62632:     object=/home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps1-mod.so [1]
     62632:      scope 0: /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps2-mod.so /lib64/libc.so.6 /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps1-mod.so /lib64/ld-linux-x86-64.so.2
     62632:     
     62632:     object=/lib64/ld-linux-x86-64.so.2 [1]
     62632:      scope 0: /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps2-mod.so /lib64/libc.so.6 /home/fweimer/src/gnu/glibc/build-master/elf/tst-nodeps1-mod.so /lib64/ld-linux-x86-64.so.2
     62632:     

That happens because the relevant objects are not linked with -rpath
(they are intentionally minimal).

I don't know what the best way to fix this.  Is there a makefile
variable to get the required -rpath arguments for
--enable-hardcoded-path-in-tests builds?  Or we could set
LD_LIBRARY_PATH:

tst-nodeps2-ENV = LD_LIBRARY_PATH=$(objpfx):$(common-objpfx)

Thanks,
Florian
Andreas Schwab Dec. 13, 2023, 12:32 p.m. UTC | #4
On Dez 13 2023, Florian Weimer wrote:

> I don't know what the best way to fix this.  Is there a makefile
> variable to get the required -rpath arguments for
> --enable-hardcoded-path-in-tests builds?

# Tests use -Wl,-rpath instead of -Wl,-rpath-link for
# build-hardcoded-path-in-tests.
ifeq (yes,$(build-hardcoded-path-in-tests))
link-libc-tests-rpath-link = $(link-libc-rpath)
link-test-modules-rpath-link = $(link-libc-rpath)
else
link-libc-tests-rpath-link = $(link-libc-rpath-link)
link-test-modules-rpath-link =
endif  # build-hardcoded-path-in-tests
Carlos O'Donell Jan. 10, 2024, 4:06 p.m. UTC | #5
On 12/13/23 07:32, Andreas Schwab wrote:
> On Dez 13 2023, Florian Weimer wrote:
> 
>> I don't know what the best way to fix this.  Is there a makefile
>> variable to get the required -rpath arguments for
>> --enable-hardcoded-path-in-tests builds?
> 
> # Tests use -Wl,-rpath instead of -Wl,-rpath-link for
> # build-hardcoded-path-in-tests.
> ifeq (yes,$(build-hardcoded-path-in-tests))
> link-libc-tests-rpath-link = $(link-libc-rpath)
> link-test-modules-rpath-link = $(link-libc-rpath)
> else
> link-libc-tests-rpath-link = $(link-libc-rpath-link)
> link-test-modules-rpath-link =
> endif  # build-hardcoded-path-in-tests
> 

I posted a patch to fix this. I also build with --enable-hardcoded-path-in-tests
and was seeing the issue.

https://patchwork.sourceware.org/project/glibc/patch/20240110155011.3930473-1-carlos@redhat.com/
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 3f7f89508e..afec7be084 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -433,6 +433,8 @@  tests += \
   tst-nodelete-dlclose \
   tst-nodelete-opened \
   tst-nodelete2 \
+  tst-nodeps1 \
+  tst-nodeps2 \
   tst-noload \
   tst-non-directory-path \
   tst-null-argv \
@@ -863,6 +865,8 @@  modules-names += \
   tst-nodelete-dlclose-plugin \
   tst-nodelete-opened-lib \
   tst-nodelete2mod \
+  tst-nodeps1-mod \
+  tst-nodeps2-mod \
   tst-non-directory-mod \
   tst-null-argv-lib \
   tst-p_alignmod-base \
@@ -1030,6 +1034,8 @@  modules-names-nobuild += \
   tst-audit24bmod1 \
   tst-audit24bmod2 \
   tst-big-note-lib \
+  tst-nodeps1-mod \
+  tst-nodeps2-mod \
   tst-ro-dynamic-mod \
   # modules-names-nobuild
 
@@ -3009,3 +3015,18 @@  tst-env-setuid-ARGS = -- $(host-test-program-cmd)
 
 # Reuse a module with a SONAME, to specific as the LD_PROFILE.
 $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
+
+# The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
+$(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os
+	$(LINK.o) -nostartfiles -nostdlib -shared -o $@ $^
+tst-nodeps1.so-no-z-defs = yes
+# Link libc.so before the test module with the IFUNC resolver reference.
+LDFLAGS-tst-nodeps1 = $(common-objpfx)libc.so $(objpfx)tst-nodeps1-mod.so
+$(objpfx)tst-nodeps1: $(objpfx)tst-nodeps1-mod.so
+# Reuse the tst-nodeps1 module.  Link libc.so before the test module
+# with the IFUNC resolver reference.
+$(objpfx)tst-nodeps2-mod.so: $(common-objpfx)libc.so \
+  $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.os
+	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^
+$(objpfx)tst-nodeps2.out: \
+  $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.so
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 417d2fb948..b748c278ac 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -708,6 +708,17 @@  dl_open_worker_begin (void *a)
      them.  However, such relocation dependencies in IFUNC resolvers
      are undefined anyway, so this is not a problem.  */
 
+  /* Ensure that libc is relocated first.  This helps with the
+     execution of IFUNC resolvers in libc, and matters only to newly
+     created dlmopen namespaces.  Do not do this for static dlopen
+     because libc has relocations against ld.so, which may not have
+     been relocated at this point.  */
+#ifdef SHARED
+  if (GL(dl_ns)[args->nsid].libc_map != NULL)
+    _dl_open_relocate_one_object (args, r, GL(dl_ns)[args->nsid].libc_map,
+				  reloc_mode, &relocation_in_progress);
+#endif
+
   for (unsigned int i = last; i-- > first; )
     _dl_open_relocate_one_object (args, r, new->l_initfini[i], reloc_mode,
 				  &relocation_in_progress);
diff --git a/elf/rtld.c b/elf/rtld.c
index 0553c05edb..19bedcd4a6 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2272,11 +2272,17 @@  dl_main (const ElfW(Phdr) *phdr,
      objects.  We do not re-relocate the dynamic linker itself in this
      loop because that could result in the GOT entries for functions we
      call being changed, and that would break us.  It is safe to relocate
-     the dynamic linker out of order because it has no copy relocs (we
-     know that because it is self-contained).  */
+     the dynamic linker out of order because it has no copy relocations.
+     Likewise for libc, which is relocated early to ensure that IFUNC
+     resolvers in libc work.  */
 
   int consider_profiling = GLRO(dl_profile) != NULL;
 
+  if (GL(dl_ns)[LM_ID_BASE].libc_map != NULL)
+    _dl_relocate_object (GL(dl_ns)[LM_ID_BASE].libc_map,
+			 GL(dl_ns)[LM_ID_BASE].libc_map->l_scope,
+			 GLRO(dl_lazy) ? RTLD_LAZY : 0, consider_profiling);
+
   /* If we are profiling we also must do lazy reloaction.  */
   GLRO(dl_lazy) |= consider_profiling;
 
diff --git a/elf/tst-nodeps1-mod.c b/elf/tst-nodeps1-mod.c
new file mode 100644
index 0000000000..45c8e3c631
--- /dev/null
+++ b/elf/tst-nodeps1-mod.c
@@ -0,0 +1,25 @@ 
+/* Test module with no libc.so dependency and string function references.
+   Copyright (C) 2023 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 <string.h>
+
+/* Some references to libc symbols which are likely to have IFUNC
+   resolvers.  If they do not, this module does not exercise bug 31083.  */
+void *memcpy_pointer = memcpy;
+void *memmove_pointer = memmove;
+void *memset_pointer = memset;
diff --git a/elf/tst-nodeps1.c b/elf/tst-nodeps1.c
new file mode 100644
index 0000000000..1a8bde36cd
--- /dev/null
+++ b/elf/tst-nodeps1.c
@@ -0,0 +1,23 @@ 
+/* Test initially loaded module with implicit libc.so dependency (bug 31083).
+   Copyright (C) 2023 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/>.  */
+
+/* Testing happens before main.  */
+int
+main (void)
+{
+}
diff --git a/elf/tst-nodeps2-mod.c b/elf/tst-nodeps2-mod.c
new file mode 100644
index 0000000000..4913feee9b
--- /dev/null
+++ b/elf/tst-nodeps2-mod.c
@@ -0,0 +1 @@ 
+/* Empty test module which depends on tst-nodeps1-mod.so.  */
diff --git a/elf/tst-nodeps2.c b/elf/tst-nodeps2.c
new file mode 100644
index 0000000000..0bdc8eeb8c
--- /dev/null
+++ b/elf/tst-nodeps2.c
@@ -0,0 +1,29 @@ 
+/* Test dlmopen with implicit libc.so dependency (bug 31083).
+   Copyright (C) 2023 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/xdlfcn.h>
+
+static int
+do_test (void)
+{
+  void *handle = xdlmopen (LM_ID_NEWLM, "tst-nodeps2-mod.so", RTLD_NOW);
+  xdlclose (handle);
+  return 0;
+}
+
+#include <support/test-driver.c>