RFC: Auditor access to program headers

RFC: Auditor access to program headers

[Jonathon Anderson]

Hello all,

We (the HPCToolkit team) recently encountered a new complication using 
LD_AUDIT as part of the HPCToolkit performance tools. In one shared 
library we encountered, the program headers are mapped in a PT_LOAD 
segment to a different virtual offset than their file offset. This 
caused our auditor to crash since we had previously assumed these 
offsets would be identical. Without this assumption, we cannot access 
the mapped program headers for a binary. While we can work around the 
issue (with a serious performance hit) by opening the file to read the 
headers ourselves, it does concern us that there is no secure (or fast) 
way for an auditor to access the program headers for a loaded binary 
without horrific dl_iterate_phdr shims.

We propose two changes to support this use case:
  - Addition of a new dlinfo request that fills a dl_phdr_info (or 
similar) structure identically to how dl_iterate_phdr would. This would 
enable access to a load module's program headers without use of 
dl_iterate_phdr.
  - A fix for the bug that causes dl* functions to crash in early 
auditor notifications (one of our Tier 2 issues). This is required to 
allow use of dlinfo within an auditor. Fixing this involves adjusting 
rtld_active to return true during auditor notifications in dl_main.

These two changes should have no ABI impact. It would help us greatly if 
these changes were made and backported to supported Red Hat versions, 
including the upcoming Fedora 36 release.

For additional clarity, it would also be helpful to have either:
  - documentation that a link_map* is in fact a dlopen handle and thus 
can be passed to any dl* function in current and future versions of 
Glibc, or
  - the prototype of la_objopen altered to take a void* dlopen handle 
instead of a link_map* (which AFAIK currently has no or minimal ABI impact).

-Jonathon

Re: RFC: Auditor access to program headers

[Florian Weimer]

* Jonathon Anderson:

> We (the HPCToolkit team) recently encountered a new complication using
> LD_AUDIT as part of the HPCToolkit performance tools. In one shared 
> library we encountered, the program headers are mapped in a PT_LOAD
> segment to a different virtual offset than their file offset. This 
> caused our auditor to crash since we had previously assumed these
> offsets would be identical. Without this assumption, we cannot access 
> the mapped program headers for a binary. While we can work around the
> issue (with a serious performance hit) by opening the file to read the 
> headers ourselves, it does concern us that there is no secure (or
> fast) way for an auditor to access the program headers for a loaded
> binary without horrific dl_iterate_phdr shims.

I agree it's a gap in the interfaces.

> We propose two changes to support this use case:
>  - Addition of a new dlinfo request that fills a dl_phdr_info (or
> similar) structure identically to how dl_iterate_phdr would. This
> would enable access to a load module's program headers without use of 
> dl_iterate_phdr.

The existing dlinfo requests are problematic (origin and search path
information access suffers from buffer overflows), and these
type-polymorphic dispatchers are never great.  At least for program
header lookup, it's possible to detect the unsupported request even
though the current implementation does not report any errors in that
case (the dlinfo caller needs to write a null pointer and check if it
has been overwritten).  So it looks a feasible short-term solution.

>  - A fix for the bug that causes dl* functions to crash in early
> auditor notifications (one of our Tier 2 issues). This is required to 
> allow use of dlinfo within an auditor. Fixing this involves adjusting
> rtld_active to return true during auditor notifications in dl_main.

Almost all rtld_active calls can go away in glibc 2.34 because
GLRO (dl_dlfcn_hook) is now in read-only memory.  That change isn't
quite backportable because throughly breaks static dlopen.  But
rtld_active could simply check some other _rtld_global_ro field
in earlier glibc version.

> For additional clarity, it would also be helpful to have either:
>  - documentation that a link_map* is in fact a dlopen handle and thus
> can be passed to any dl* function in current and future versions of 
> Glibc, or

I think we can promise that every link map pointer provided by glibc can
be used as a dlopen handle (during its life-time).  The other direction
(every handle returned by dlopen is a link map) might be more
complicated.  But perhaps we can at least commit to providing the public
link map fields (as seen in the installed <link.h>).

>  - the prototype of la_objopen altered to take a void* dlopen handle
> instead of a link_map* (which AFAIK currently has no or minimal ABI
> impact).

It's a source-incompatible change, though, because we declare la_objopen
in <link.h>.  So perhaps we can avoid it.

Thanks,
Florian

Re: RFC: Auditor access to program headers

[Jonathon Anderson]

On 3/17/22 09:59, Florian Weimer wrote:
>> We propose two changes to support this use case:
>>   - Addition of a new dlinfo request that fills a dl_phdr_info (or
>> similar) structure identically to how dl_iterate_phdr would. This
>> would enable access to a load module's program headers without use of
>> dl_iterate_phdr.
> The existing dlinfo requests are problematic (origin and search path
> information access suffers from buffer overflows), and these
> type-polymorphic dispatchers are never great.  At least for program
> header lookup, it's possible to detect the unsupported request even
> though the current implementation does not report any errors in that
> case (the dlinfo caller needs to write a null pointer and check if it
> has been overwritten).  So it looks a feasible short-term solution.
The current implementation does error on unsupported requests, and 
AFAICT has since the initial implementation in 2003:

       switch (args->request)
         {
         case RTLD_DI_CONFIGADDR:
         default:
           _dl_signal_error (0, NULL, NULL, N_("unsupported dlinfo
    request"));
           break;

Adding a new dlinfo request should work well enough for our purposes, 
we'll keep a runtime fallback in case the request isn't supported.

For my own curiosity, what would you consider for a longer-term solution?

>>   - A fix for the bug that causes dl* functions to crash in early
>> auditor notifications (one of our Tier 2 issues). This is required to
>> allow use of dlinfo within an auditor. Fixing this involves adjusting
>> rtld_active to return true during auditor notifications in dl_main.
> Almost all rtld_active calls can go away in glibc 2.34 because
> GLRO (dl_dlfcn_hook) is now in read-only memory.  That change isn't
> quite backportable because throughly breaks static dlopen.  But
> rtld_active could simply check some other _rtld_global_ro field
> in earlier glibc version.
Great! We'll be happy to test when it's available.

>> For additional clarity, it would also be helpful to have either:
>>   - documentation that a link_map* is in fact a dlopen handle and thus
>> can be passed to any dl* function in current and future versions of
>> Glibc, or
> I think we can promise that every link map pointer provided by glibc can
> be used as a dlopen handle (during its life-time).
I believe this direction is the only one missing, 
dlinfo(RTLD_DI_LINKMAP) exists for the other direction.

> The other direction
> (every handle returned by dlopen is a link map) might be more
> complicated.  But perhaps we can at least commit to providing the public
> link map fields (as seen in the installed <link.h>).
>
>>   - the prototype of la_objopen altered to take a void* dlopen handle
>> instead of a link_map* (which AFAIK currently has no or minimal ABI
>> impact).
> It's a source-incompatible change, though, because we declare la_objopen
> in <link.h>.  So perhaps we can avoid it.
It's source-incompatible, though IMHO it's an easy configure test and a 
very minor source change. If Glibc can't commit to keeping link_map* 
usable as a dlopen handle (eg. for possible future security concerns), 
this prevents the possibility that auditors could get "locked out" of 
the dl* functions.

Thanks,
-Jonathon

Re: RFC: Auditor access to program headers

[Florian Weimer]

* Jonathon Anderson:

> The current implementation does error on unsupported requests, and
> AFAICT has since the initial implementation in 2003:
>
>    switch (args->request)
>      {
>      case RTLD_DI_CONFIGADDR:
>      default:
>        _dl_signal_error (0, NULL, NULL, N_("unsupported dlinfo request"));
>        break;

Oops, I had missed that.

> Adding a new dlinfo request should work well enough for our purposes,
> we'll keep a runtime fallback in case the request isn't supported.

I've posted a patch:

  [PATCH] dlfcn: Implement the RTLD_DI_PHDR request type for dlinfo
  <https://sourceware.org/pipermail/libc-alpha/2022-April/138012.html>

I couldn't find precedent for this in other loaders, so I made something
up from scratch.

> For my own curiosity, what would you consider for a longer-term
> solution?

Separate function calls instead of a type-polymorphic multiplexer.  (A
while back, I tried to write type-safety diagnostics for fcntl, but gave
up because of the complexity, but the diagnostics would have been worth
it—this was before the separate fcntl64 function.)

[rtld_active () crash]

> Great! We'll be happy to test when it's available.

I'll try to work on this next.

Thanks,
Florian