This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [patch] Fix BZ #16634 -- assert in ld.so when dlopen("a.out"...) is called repeatedly.
- From: Siddhesh Poyarekar <siddhesh dot poyarekar at gmail dot com>
- To: Paul Pluzhnikov <ppluzhnikov at google dot com>
- Cc: Siddhesh Poyarekar <siddhesh at redhat dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Sat, 22 Mar 2014 11:17:15 +0530
- Subject: Re: [patch] Fix BZ #16634 -- assert in ld.so when dlopen("a.out"...) is called repeatedly.
- Authentication-results: sourceware.org; auth=none
- References: <ye6qpplo78g2 dot fsf at elbrus2 dot mtv dot corp dot google dot com> <20140316105130 dot GJ1850 at spoyarek dot pnq dot redhat dot com> <CALoOobOt4q05ht82tWw3P=+TgR5sp3HW=DFpJC57O9knXzO6iQ at mail dot gmail dot com>
On 22 March 2014 04:09, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> I usually just use ChangeLog as commit log.
> Should I be using more verbose patch description instead?
What Mike said - it makes things much easier for us distro folks to do
backports and definitely infinitely easier for someone looking for the
rationale of a specific change.
> 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.
This is good, thanks.
>> 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.
OK, but please don't forget ;)
> 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.
The patch looks OK to me, but (I'm sorry it didn't occur to me in my
initial review) shouldn't there be a test case for this? I think you
could adapt the reproducer in the bz into a test case.
Also, please try to always post the patch inline like you did
originally - an attached patch does not come up in the reply template
and due to that it is a bit cumbersome to comment on snippets of
patches.
Thanks,
Siddhesh
--
http://siddhesh.in