[PATCH] elf: Do not signal LA_ACT_CONSISTENT for an empty namespace [BZ #26076]

[Carlos O'Donell]

On 7/6/20 4:47 PM, H.J. Lu wrote:
> On Mon, Jul 6, 2020 at 1:41 PM Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> On 7/6/20 4:40 PM, H.J. Lu via Libc-alpha wrote:
>>> On Mon, Jul 6, 2020 at 12:45 PM Florian Weimer via Libc-alpha
>>> <libc-alpha@sourceware.org> wrote:
>>>>
>>>> The auditing interface identifies namespaces by their first loaded
>>>> module.  Once the namespace is empty, it is no longer possible to signal
>>>> LA_ACT_CONSISTENT for it because the first loaded module is already gone
>>>> at that point.
>>>>
>>>> Tested on i686-linux-gnu and x86_64-linux-gnu.
>>>>
>>>> ---
>>>>  elf/dl-close.c | 10 ++++++++--
>>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/elf/dl-close.c b/elf/dl-close.c
>>>> index 73b2817bbf..8e146ecee1 100644
>>>> --- a/elf/dl-close.c
>>>> +++ b/elf/dl-close.c
>>>> @@ -781,8 +781,14 @@ _dl_close_worker (struct link_map *map, bool force)
>>>>    if (__glibc_unlikely (do_audit))
>>>>      {
>>>>        struct link_map *head = ns->_ns_loaded;
>>>> -      /* Do not call the functions for any auditing object.  */
>>>> -      if (head->l_auditing == 0)
>>>
>>> I assume that "head" can be NULL.  Do you have a testcase?
>>
>> Yes, it's tst-auditmany in trunk right now. It fails because of this issue.
> 
> I have seen tst-auditmany failure,  but not always.  The test was added
> more than 6 months ago. Why does it start failing now?

A combination of, IMO, Szabolcs surplus tls fixes, rseq increasing static
TLS usage, and Florian's recent (months ago) fixes to make unloading reliable.

The 9th audit module can fail to load if it runs out of static tls surplus
to use or if the audit module load fails (not enough namespaces).

I see this failing with Szabolcs recent patches to adjust tls surplus.

I have debugged the failure and it's like this:

(a) We try to load the auditor.
(b) The auditor load fails and we unwind all the loads, leaving an
    empty link namespace.
(c) We try to indicate consistent namespace for an empty namespace
    and crash.

With Florian's recent work to make unloading reliable we now end up with
empty link namespaces for failed to load audit modules. As they should be.

However this above code expects there to be *something* left in the link
namespace and there isn't.

We *could* arrange to call LA_ACT_CONSISTENT with a NULL cookie in this
case for all observing auditors. I'm not opposed to that.
 
>>>
>>>> +      /* If head is NULL, the namespace has become empty, and the
>>>> +        audit interface does not give us a way to signal
>>>> +        LA_ACT_CONSISTENT for it because the first loaded module is
>>>> +        used to identify the namespace.
>>>> +
>>>> +        Furthermore, do not notify auditors of the cleanup of a
>>>> +        failed audit module loading attempt.  */
>>>> +      if (head != NULL && head->l_auditing == 0)
>>>>         {
>>>>           struct audit_ifaces *afct = GLRO(dl_audit);
>>>>           for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
>>>>
>>>
>>>
>>
>>
>> --
>> Cheers,
>> Carlos.
>>
> 
> 


-- 
Cheers,
Carlos.