Re: [PATCH 1/2] elf: Honour skip_ifunc for cross-object IFUNC relocations [BZ #34428]

On 02/08/26 22:04, H.J. Lu wrote:
> On Mon, Aug 3, 2026 at 8:28 AM H.J. Lu <[email protected]> wrote:
>>
>> On Mon, Aug 3, 2026 at 8:11 AM H.J. Lu <[email protected]> wrote:
>>>
>>> On Mon, Aug 3, 2026 at 7:45 AM Adhemerval Zanella
>>> <[email protected]> wrote:
>>>>
>>>> Commit 63b31c05a8a dropped the skip_ifunc argument from elf_dynamic_do_Rel,
>>>> assuming the elf_dynamic_do_Rel_irelative pass handles every relocation
>>>> that may run an IFUNC resolver.  That only holds for IFUNC symbols defined
>>>> in the object being relocated, a reference to an IFUNC in another object is
>>>> an ordinary JMP_SLOT or GLOB_DAT against an undefined symbol, and being
>>>> IFUNC is only known after symbol resolution (elf_machine_rel).  Those
>>>> relocations stay in the regular pass, which no longer propagated
>>>> skip_ifunc, so __RTLD_NOIFUNC was ignored for them.
>>>>
>>>> ldd -u forces non-lazy binding (GLRO(dl_lazy) = 0 for DL_DEBUG_UNUSED), so
>>>> the resolver was called and the diagnostic emitted:
>>>>
>>>>   $ ldd -u /bin/ls
>>>>   /bin/ls: Relink `' with `/usr/lib64/libc.so.6' for IFUNC symbol `__mempcpy_chk'
>>>>
>>>> ldd -r with LD_BIND_NOW is affected in the same way.
>>>>
>>>> Restore the skip_ifunc parameter and pass it through _ELF_DYNAMIC_DO_RELOC
>>>> and ELF_DYNAMIC_RELOCATE_NOIFUNC.
>>>>
>>>> Checked on x86_64-linux-gnu, aarch64-linux-gnu, and i686-linux-gnu. I
>>>> also built for all supported architectures and run the elf tests on
>>>> qemu-system for armhf, alpha, hppa, loongarch64, mipsel, mips64le,
>>>> powerpc*, riscv64, s390x, and sparc64.
>>>> ---
>>>>  elf/Makefile                      | 28 +++++++++++++++++++++++
>>>>  elf/dl-reloc-static-pie.c         |  2 +-
>>>>  elf/dl-reloc.c                    |  3 ++-
>>>>  elf/do-rel.h                      | 24 ++++++++++++-------
>>>>  elf/dynamic-link.h                | 32 ++++++++++++++++----------
>>>>  elf/tst-ifunc-fault-dep-bindnow.c | 19 ++++++++++++++++
>>>>  elf/tst-ifunc-fault-dep-lazy.c    | 27 ++++++++++++++++++++++
>>>>  elf/tst-ifunc-fault-mod.c         | 38 +++++++++++++++++++++++++++++++
>>>>  8 files changed, 151 insertions(+), 22 deletions(-)
>>>>  create mode 100644 elf/tst-ifunc-fault-dep-bindnow.c
>>>>  create mode 100644 elf/tst-ifunc-fault-dep-lazy.c
>>>>  create mode 100644 elf/tst-ifunc-fault-mod.c
>>>>
>>>> diff --git a/elf/Makefile b/elf/Makefile
>>>> index 94c5b7e6ed8..6cfb1b5cf30 100644
>>>> --- a/elf/Makefile
>>>> +++ b/elf/Makefile
>>>> @@ -1396,6 +1396,13 @@ modules-names += \
>>>>    tst-ifunc-tls-write-lib \
>>>>    tst-tls-tdata-reloc-lib \
>>>>    # modules-names
>>>> +ifeq (yes,$(have-gcc-ifunc))
>>>> +tests += \
>>>> +  tst-ifunc-fault-dep-bindnow \
>>>> +  tst-ifunc-fault-dep-lazy \
>>>> +  # tests
>>>> +modules-names += tst-ifunc-fault-mod
>>>> +endif
>>>>  ifneq (no,$(have-test-mtls-descriptor))
>>>>  tests += tst-ifunc-tls-init-tlsdesc
>>>>  modules-names += tst-ifunc-tls-init-tlsdesc-lib
>>>> @@ -2545,6 +2552,27 @@ $(objpfx)tst-ifunc-fault-bindnow.out: $(objpfx)tst-ifunc-fault-bindnow \
>>>>     $(objpfx)ld.so
>>>>         $(tst-ifunc-fault-script)
>>>>
>>>> +LDFLAGS-tst-ifunc-fault-dep-lazy = -Wl,-z,lazy
>>>> +LDFLAGS-tst-ifunc-fault-dep-bindnow = -Wl,-z,now
>>>> +define tst-ifunc-fault-dep-script
>>>> +( $(test-wrapper) $(rtld-prefix) --verify $< \
>>>> +  && $(test-wrapper-env) LD_TRACE_LOADED_OBJECTS=1 $(rtld-prefix) $< \
>>>> +  && $(test-wrapper-env) LD_TRACE_LOADED_OBJECTS=1 LD_DEBUG=unused \
>>>> +                        $(rtld-prefix) $< \
>>>> +  && $(test-wrapper-env) LD_TRACE_LOADED_OBJECTS=1 LD_WARN=yes LD_BIND_NOW=1 \
>>>> +                        $(rtld-prefix) $< \
>>>> +) > $@; $(evaluate-test)
>>>> +endef
>>>> +$(objpfx)tst-ifunc-fault-dep-lazy: $(objpfx)tst-ifunc-fault-mod.so
>>>> +$(objpfx)tst-ifunc-fault-dep-bindnow: $(objpfx)tst-ifunc-fault-mod.so
>>>> +$(objpfx)tst-ifunc-fault-dep-lazy.out: $(objpfx)tst-ifunc-fault-dep-lazy \
>>>> +   $(objpfx)tst-ifunc-fault-mod.so $(objpfx)ld.so
>>>> +       $(tst-ifunc-fault-dep-script)
>>>> +$(objpfx)tst-ifunc-fault-dep-bindnow.out: \
>>>> +   $(objpfx)tst-ifunc-fault-dep-bindnow \
>>>> +   $(objpfx)tst-ifunc-fault-mod.so $(objpfx)ld.so
>>>> +       $(tst-ifunc-fault-dep-script)
>>>> +
>>>>  LDFLAGS-tst-ifunc-plt-lib.so = -Wl,-z,lazy
>>>>
>>>>  tst-ifunc-plt-bindnow-ENV = LD_BIND_NOW=1
>>>> diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
>>>> index 8463e46147d..53260b57372 100644
>>>> --- a/elf/dl-reloc-static-pie.c
>>>> +++ b/elf/dl-reloc-static-pie.c
>>>> @@ -80,7 +80,7 @@ _dl_relocate_static_pie (void)
>>>>
>>>>    /* Relocate ourselves so we can do normal function calls and data access
>>>>       using the global offset table.  IRELATIVE entries are deferred.  */
>>>> -  ELF_DYNAMIC_RELOCATE_NOIFUNC (main_map, NULL, 0, 0);
>>>> +  ELF_DYNAMIC_RELOCATE_NOIFUNC (main_map, NULL, 0, 0, 0);
>>>>
>>>>    /* Initialize _r_debug_extended.  */
>>>>    struct r_debug *r = _dl_debug_initialize (0, LM_ID_BASE);
>>>> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
>>>> index 15a6a4cffef..80cd7e25d16 100644
>>>> --- a/elf/dl-reloc.c
>>>> +++ b/elf/dl-reloc.c
>>>> @@ -278,7 +278,8 @@ _dl_relocate_object_no_relro (struct link_map *l, struct r_scope_elem *scope[],
>>>>         IFUNC resolvers.  Without this, a resolver would see the unrelocated
>>>>         initialiser bytes that were placed into the slot by the early
>>>>         _dl_allocate_tls_init.  */
>>>> -    ELF_DYNAMIC_RELOCATE_NOIFUNC (l, scope, lazy, consider_profiling);
>>>> +    ELF_DYNAMIC_RELOCATE_NOIFUNC (l, scope, lazy, consider_profiling,
>>>> +                                 skip_ifunc);
>>>
>>> ELF_DYNAMIC_RELOCATE_NOIFUNC name is confusing.
>>> If it is NOIFUNC, why isn't IFUNC skipped?
>>>
>>
>> There are
>>
>> /* Like ELF_DYNAMIC_RELOCATE but only processes the non-IRELATIVE pass.
>>    The IRELATIVE pass must be completed later via ELF_DYNAMIC_RELOCATE_IFUNC.
>>    Used by the static-pie startup so the TCB and stack-protector canary can
>>    be initialised between the two passes.  */
>> # define ELF_DYNAMIC_RELOCATE_NOIFUNC(map, scope, lazy, consider_profile)
>>
>> This change makes the comments incorrect.
> 
> Can ELF_DYNAMIC_RELOCATE_NOIFUNC/ELF_DYNAMIC_RELOCATE_IFUNC
> be renamed to ELF_DYNAMIC_RELOCATE_PASS:
> 
> # define ELF_DYNAMIC_RELOCATE_PASS(pass, map, scope, lazy, skip_ifunc)
I don't have a strong opinion, I will send a new version with the proposed
name.