Re: Should glibc be fully reentrant? What do we allow interposed symbols to do?

[Will Newton <will.newton@linaro.org>]

On 23 October 2014 15:40, Carlos O'Donell <carlos@redhat.com> wrote:

Hi Carlos,

> On 10/23/2014 04:23 AM, Will Newton wrote:
>> On 23 October 2014 03:29, Carlos O'Donell <carlos@redhat.com> wrote:
>>> On 10/21/2014 06:47 PM, Roland McGrath wrote:
>>>> But the slippery slope concerns me most of all.
>>>
>>> Any function the user interposes acts as a synchronous interrupt on the
>>> runtime.
>>>
>>> It is my opinion that users expect to be able to call any routine in the
>>> runtime without caution unless we tell them otherwise.
>>>
>>> Given that dlopen locks are recursive, as are stdio locks, I propose we
>>> fully support this notion that users already believe exists.
>>>
>>> The alternative is that we don't support it and treat interposed functions
>>> as if they were in a signal handler context, only being allowed to call
>>> async-signal-safe functions, and we might as well remove the recursive
>>> support from the locks such that users get useful backtraces from deadlocks.
>>> It is my opinion that such a direction would not help our users and would
>>> not help the project.
>>>
>>> The similar situations we need to clarify are LD_AUDIT modules, and
>>> IFUNC resolvers, but let us proceed orderly one topic at a time.
>>>
>>> In summary
>>> ==========
>>>
>>> Allow interposed functions to call back into the runtime, and fix any
>>> places where this breaks.
>>
>> Do you have a plan to fix dlsym similarly? ISTR that pretty reliably
>> trips on the same issue when used in a malloc hook.
>
> If you can write a test case I will look at it.
>
> I have a reproducer for dlopen.

I had mis-remembered the details of the issue. What I did find was
that using LD_PRELOAD to override malloc and calling dlsym can be a
problem. This was caused by the following code in dlfcn/dlerror.c:

      /* We don't use the static buffer and so we have a key.  Use it
         to get the thread-specific buffer.  */
      result = __libc_getspecific (key);
      if (result == NULL)
        {
          result = (struct dl_action_result *) calloc (1, sizeof (*result));
          if (result == NULL)
            /* We are out of memory.  Since this is no really critical
               situation we carry on by using the global variable.
               This might lead to conflicts between the threads but
               they soon all will have memory problems.  */
            result = &last_result;
          else
            /* Set the tsd.  */
            __libc_setspecific (key, result);

The calloc call can cause a loop and overflow the stack. If you use a
malloc hook and enable/disable the hook correctly then everything is
OK I think.

-- 
Will Newton
Toolchain Working Group, Linaro