Re: A collection of LD_AUDIT bugs that are important for tools (with better formatting for this list)

[Florian Weimer via Libc-alpha <libc-alpha@sourceware.org>]

* John Mellor-Crummey:

>  On Jun 17, 2021, at 3:09 PM, Florian Weimer <fweimer@redhat.com> wrote:
>
>> The issue is that the la_symbind interface is not very good at
>> communicating that PLT enter/exit hooks aren't available under these
>> circumstances.  
>
> This is a separate issue from the one we reported. The issue we reported
> was that la_symbind wasn’t called and LD_BIND_NOW was not used.

It's kind of related.  Our own example implementation looks like this:

uintptr_t
la_symbind (Elf_Sym *sym, unsigned int ndx, uintptr_t *refcook,
	    uintptr_t *defcook, unsigned int *flags, const char *symname)
{
  if (!do_exit)
    *flags = LA_SYMB_NOPLTEXIT;

  return sym->st_value;
}

Let's assume that we start calling la_symbind in places where there is
no support for enter/exit hooks.  We could initialize *flags with
LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT, but the code above would clear
the LA_SYMB_NOPLTENTER flag in !do_exit mode.

I want to increase LAV_CURRENT to 2 and call la_symbind in the BIND_NOW
cases only if la_version returned a value greater than 1.  This way, old
audit modules (which are supposed to return LAV_CURRENT from <link.h> in
la_version) will continue to work because they do not see any unexpected
la_symbind calls.

Once we call la_symbind in contexts where no enter/exit hooks are
available, we should initialize the flags to LA_SYMB_NOPLTENTER |
LA_SYMB_NOPLTEXIT (so that la_symbind can detect the situation), and
report a dlopen/loader error if those flags are cleared by la_symbind.
(With our example code, this would call pretty much all binding to fail,
which is why I think we need the LAV_CURRENT change.)

>> pthread_create interception becomes more difficult in glibc 2.34 because
>> the pthread_create symbol is no longer interposable.
>
> I don’t understand why pthread_create will no longer be interposable in 2.34.
> We have a set of other functions that we also need to intercept, shown below:
>
> _Exit
> _exit
> execl
> execle
> execlp
> execv
> execve
> execvp
> exit
> fork
> pthread_create
> pthread_exit
> pthread_sigmask
> sigaction
> signal
> sigprocmask
> sigtimedwait
> sigwait
> sigwaitinfo
> system
> vfork
>
> If by "pthread_create symbol is no longer interposable", that means we
> can’t insert a wrapper, then that is very bad for performance tools.

Once we merge librt and libanl into libc (patches for that have been
posted), mq_notify, the timer functions, and getaddrinfo_a will call
pthread_create using a direct call that cannot be intercepted in this
way.  There is precedent for making things interposable/interceptable
in the form of malloc

  <https://www.gnu.org/software/libc/manual/html_node/Replacing-malloc.html>

but we are currently do not plan to do this for pthread_create.  It
would not be an ABI change as such, so we could introduce the indirect
call as a later change based on user feedback.

You can already see this non-interceptable thread creation behavior
today (in glibc 2.33 and earlier) with thrd_create, which does not
result in a pthread_create call, either, despite creating a new thread
as if by pthread_create.

It's also the reason why your list contains the exec* functions and
system in addition to fork, vfork, and execve, even though system is
implemented on top of those functions: the internal direct calls are
invisible to auditors.  But posix_spawn, posix_spawnp, popen are
missing, too, so you will not trace all created processes.

Starting with glibc 2.32, thread signal masks can also be manipulated
using pthread_attr_setsigmask_np, and that might go unnoticed with your
present sets of intercepts (although the mask change would be visible
from a thread start routine wrapper injected via pthread_create).

Going back to trheading, I find it a bit curious that you intercept
pthread_create, but not pthread_join.  How do you detect thread exit?  I
assume you are interested in that event, too.  Merely wrapping the
thread start routine is insufficient because there are other ways for a
thread to exit besides returning from the start routine and calling
pthread_exit (e.g., thread cancellation and unwinding).

> Should we expect any problems for the other functions listed above in
> addition to pthread_create?

I don't think glibc 2.34 will bring any new problems in this area, but
there are some pre-existing issues around posix_spawn, popen,
pthread_attr_setsigmask_np.

Thanks,
Florian