Re: [patch] Fix BZ #16634 -- assert in ld.so when dlopen("a.out"...) is called repeatedly.

[Paul Pluzhnikov <ppluzhnikov at google dot com>]

On Sun, Mar 16, 2014 at 3:51 AM, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:
> On Fri, Mar 14, 2014 at 02:54:05PM -0700, Paul Pluzhnikov wrote:
>> Greetings,
>>
>> Attached patch fixes BZ #16634 by moving sanity check for dlopen()ing
>> a.out before we call _dl_next_tls_modid() for it.
>>
>> Tested on Linux/x86_64; no new failures.
>
> A more detailed description please, assuming that it's going to be
> used for the commit log.

I usually just use ChangeLog as commit log.
Should I be using more verbose patch description instead?

An application that erroneously tries to repeatedly dlopen("a.out", ...)
hits assertion failure:

  Inconsistency detected by ld.so: dl-tls.c: 474: _dl_allocate_tls_init:
  Assertion `listp != ((void *)0)' failed!

dlopen() actually fails with  "./a.out: cannot dynamically load executable",
but it does so after incrementing dl_tls_max_dtv_idx.

Once we run out of TLS_SLOTINFO_SURPLUS (62), we crash.

>>       BZ #16634
>
> In square brackets.

Done.

>> +       /* For BZ #16634, return early.  */
>
> Likewise, please explain the condition instead of just quoting the bz
> number.

Done.

>> -       if (fd != -1 && __builtin_expect (secure, 0)
>> +       if (fd != -1 && __builtin_expect (mode & __RTLD_SECURE, 0)
>
> Use __glibc_unlikely.

There are a lot of __builtin_expect()s in this file.

I would prefer to convert them to __glibc_{un}likely in a separate pass.

Thanks,
-- 
Paul Pluzhnikov


2014-03-21  Paul Pluzhnikov  <ppluzhnikov@google.com>

	[BZ #16634]
	* elf/dl-load.c (open_verify): Add mode parameter.
        Error early when ET_EXEC and mode does not have __RTLD_OPENEXEC.
        (open_path): Change from boolean 'secure' to complete flag 'mode'
        (_dl_map_object): Adjust.

Attachment: glibc-PR16634-20140321.txt
Description: Text document