Re: V2: [PATCH] Call _dl_open_check after relocation [BZ #24259]

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Jul 1, 2019 at 9:42 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > diff --git a/elf/dl-open.c b/elf/dl-open.c
> > index 12a4f8b853..e18ee398cb 100644
> > --- a/elf/dl-open.c
> > +++ b/elf/dl-open.c
> > @@ -292,8 +292,6 @@ dl_open_worker (void *a)
> >    _dl_debug_state ();
> >    LIBC_PROBE (map_complete, 3, args->nsid, r, new);
> >
> > -  _dl_open_check (new);
> > -
> >    /* Print scope information.  */
> >    if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_SCOPES))
> >      _dl_show_scope (new, 0);
> > @@ -366,6 +364,12 @@ dl_open_worker (void *a)
> >       _dl_relocate_object (l, l->l_scope, reloc_mode, 0);
> >      }
> >
> > +  /* NB: Workaround for [BZ #20839] which doesn't remove the NODELETE
> > +     object when _dl_open_check throws an exception.  Move it after
> > +     relocation to avoid leaving the NODELETE object mapped without
> > +     relocation.  */
> > +  _dl_open_check (new);
> > +
> >    /* If the file is not loaded now as a dependency, add the search
> >       list of the newly loaded object to the scope.  */
> >    bool any_tls = false;
>
> The downside is that an undefined symbol will now mask the CET failure.
> Is this a problem?

Since both problems will lead to load failure, there is no difference
in run-time
behavior except that the CET failure won't cause program crash now.

> Overall, I can't say I'm happy about this patch, but if it addresses
> important breakage, I think this is okay for now.  If we fix the state
> rollback properly, then hopefully we can move the check further up
> again, where it probably belongs.
>

Sure.  I am checking in my patch now.

Thanks.

-- 
H.J.