Re: [PATCH] elf: Check objname before calling fatal_error

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

On Mon, Apr 8, 2024 at 10:18 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 08/04/24 13:58, H.J. Lu wrote:
> > On Mon, Apr 8, 2024 at 9:47 AM Adhemerval Zanella Netto
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 08/04/24 13:06, H.J. Lu wrote:
> >>> _dl_signal_error may be called with objname == NULL.  _dl_exception_create
> >>> checks objname == NULL.  But fatal_error doesn't.  Check objname before
> >>> calling fatal_error.  This fixes BZ #31596.
> >>
> >> Do we have a reproducer for this? The one from BZ#31596 does seems to trigger
> >> it.
> >
> > We don't.  But it may happen in theory.
>
> Why not add the test on fatal_error instead? Because if we are adding
> possible check for argument where we are not sure that it might trigger,
> it is clear to me to add where the issue might happen (besides that our
> policy is to avoid adding fixes without proper reproducers).

void
_dl_signal_exception (int errcode, struct dl_exception *exception,
                      const char *occasion)
{
  struct rtld_catch *lcatch = get_catch ();
  if (lcatch != NULL)
    {
      *lcatch->exception = *exception;
      *lcatch->errcode = errcode;

      /* We do not restore the signal mask because none was saved.  */
      __longjmp (lcatch->env[0].__jmpbuf, 1);
    }
  else
    fatal_error (errcode, exception->objname, occasion, exception->errstring);
}

Will exception->objname ever be NULL?

> Also, I think we should close the BZ#31596 as a notabug because the
> reproducer does not actually trigger an issue and it is misleading
> that this is a glibc issue.
>
> >
> >>> ---
> >>>  elf/dl-catch.c | 6 +++++-
> >>>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/elf/dl-catch.c b/elf/dl-catch.c
> >>> index 2109516dba..8ef7a4c706 100644
> >>> --- a/elf/dl-catch.c
> >>> +++ b/elf/dl-catch.c
> >>> @@ -126,7 +126,11 @@ _dl_signal_error (int errcode, const char *objname, const char *occasion,
> >>>        __longjmp (lcatch->env[0].__jmpbuf, 1);
> >>>      }
> >>>    else
> >>> -    fatal_error (errcode, objname, occasion, errstring);
> >>> +    {
> >>> +      if (objname == NULL)
> >>> +     objname = "";
> >>> +      fatal_error (errcode, objname, occasion, errstring);
> >>> +    }
> >>>  }
> >>>  rtld_hidden_def (_dl_signal_error)
> >>>
>
>


-- 
H.J.