[PATCH v2] elf: Add optimization barrier for __ehdr_start and _end

[H.J. Lu]

On Mon, Jun 16, 2025 at 2:31 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Mon, Jun 16, 2025 at 1:48 PM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > On Mon, Jun 16, 2025 at 12:55 PM Florian Weimer <fweimer@redhat.com> wrote:
> >> >>
> >> >> * H. J. Lu:
> >> >>
> >> >> > diff --git a/elf/rtld.c b/elf/rtld.c
> >> >> > index 8e8f0e6253..e6a181dc31 100644
> >> >> > --- a/elf/rtld.c
> >> >> > +++ b/elf/rtld.c
> >> >> > @@ -477,7 +477,10 @@ _dl_start_final (void *arg, struct dl_start_final_info *info)
> >> >> >    _dl_setup_hash (&_dl_rtld_map);
> >> >> >    _dl_rtld_map.l_real = &_dl_rtld_map;
> >> >> >    _dl_rtld_map.l_map_start = (ElfW(Addr)) &__ehdr_start;
> >> >> > +  /* Prevent run-time relocations against __ehdr_start and _end.  */
> >> >> > +  asm ("" : "+g" (_dl_rtld_map.l_map_start));
> >> >> >    _dl_rtld_map.l_map_end = (ElfW(Addr)) _end;
> >> >> > +  asm ("" : "+g" (_dl_rtld_map.l_map_end));
> >> >> >    /* Copy the TLS related data if necessary.  */
> >> >> >  #ifndef DONT_USE_BOOTSTRAP_MAP
> >> >> >  # if NO_TLS_OFFSET != 0
> >> >>
> >> >> This needs a better comment about why we are doing this, and exactly how
> >> >> it prevents this problem.
> >> >
> >> > How about
> >> >
> >> > Prevent compiler vectorizer from loading hidden variables into vector registers.
> >>
> >> It doesn't say why "g" is the appropriate constraint here.
> >
> > "+g" is the least restricted constraint.  It tells compilers that the
> > operand is both input and output.
>
> If it's least restricted, why can't this trigger the use of a vector
> register?  It seems to increase the risk of the bug happening, not
> suppress it.
>
> >> > Vector instructions are OK in rtld.c:
> >>
> >> For the most part, yes, but are they in performance-critical code?
> >
> > rtld.c is used by all programs.  Every bit of performance helps.
>
> I'm not sure if the use of vector registers by GCC is an optimization
> here.  It requires additional non-shareable memory and relocations.
>
> >> If we do not use -mgeneral-regs-only, there is bound to be another
> >> combination of compiler option that tries to load pointers directly into
> >> vector registers before initial self-relocation.
> >
> > The list of these hidden variables is small, which can be checked by
> > the newly added test.
>
> There are additional places where GCC uses this technique to load
> pointers, notably this place:
>
> /* Dummy allocation object used if allocating the message buffer
>    fails.  */
> static void
> oom_exception (struct dl_exception *exception)
> {
>   exception->objname = "";
>   exception->errstring = _dl_out_of_memory;
>   exception->message_buffer = NULL;
> }
>

If this is called before ELF_DYNAMIC_RELOCATE, we have a much
bigger problem.

> And this (the rfv variable):
>
> /* Functions for resolving symbols in the VDSO link map.  */
> static inline void *
> dl_vdso_vsym (const char *name)
> {
>   struct link_map *map = GLRO (dl_sysinfo_map);
>   if (map == NULL)
>     return NULL;
>
>   /* Use a WEAK REF so we don't error out if the symbol is not found.  */
>   ElfW (Sym) wsym = { 0 };
>   wsym.st_info = (unsigned char) ELFW (ST_INFO (STB_WEAK, STT_NOTYPE));
>
>   const struct r_found_version rfv = { VDSO_NAME, VDSO_HASH, 1, NULL };
>
>   /* Search the scope of the vdso map.  */
>   const ElfW (Sym) *ref = &wsym;
>   lookup_t result = GLRO (dl_lookup_symbol_x) (name, map, &ref,
>                                                map->l_local_scope,
>                                                &rfv, 0, 0, NULL);
>   return ref != NULL ? DL_SYMBOL_ADDRESS (result, ref) : NULL;
> }

Isn't this called after ELF_DYNAMIC_RELOCATE?

> These two do not even involved named objects, the relocations are
> against string literals.
>
> I think the new test should cover all of ld.so, and only accept the
> IRELATIVE relocation and the initialization of _rtld_global_ro, and no
> other run-time relocations.  (Maybe it has to be restricted to !Hurd,
> though.)
>

We only need to check run-time relocations before ELF_DYNAMIC_RELOCATE.
So far I only identified __ehdr_start  and _end.

-- 
H.J.