Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170104193627.GO1555@brightrain.aerifal.cx>
Date: Wed, 4 Jan 2017 14:36:27 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Reviving planned ldso changes

On Wed, Jan 04, 2017 at 01:22:03AM -0500, Rich Felker wrote:
> On Wed, Jan 04, 2017 at 01:06:40AM -0500, Rich Felker wrote:
> > diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> > index c689084..cb82b2c 100644
> > --- a/ldso/dynlink.c
> > +++ b/ldso/dynlink.c
> > @@ -67,6 +67,7 @@ struct dso {
> >  	char constructed;
> >  	char kernel_mapped;
> >  	struct dso **deps, *needed_by;
> > +	size_t next_dep;
> >  	char *rpath_orig, *rpath;
> >  	struct tls_module tls;
> >  	size_t tls_id;
> > @@ -1211,13 +1212,14 @@ void __libc_exit_fini()
> >  static void do_init_fini(struct dso *p)
> >  {
> >  	size_t dyn[DYN_CNT];
> > -	int need_locking = libc.threads_minus_1;
> > -	/* Allow recursive calls that arise when a library calls
> > -	 * dlopen from one of its constructors, but block any
> > -	 * other threads until all ctors have finished. */
> > -	if (need_locking) pthread_mutex_lock(&init_fini_lock);
> > -	for (; p; p=p->prev) {
> > -		if (p->constructed) continue;
> > +	pthread_mutex_lock(&init_fini_lock);
> > +	while (!p->constructed) {
> > +		while (p->deps[p->next_dep] && p->deps[p->next_dep]->constructed)
> > +			p->next_dep++;
> > +		if (p->deps[p->next_dep]) {
> > +			p = p->deps[p->next_dep++];
> > +			continue;
> > +		}
> 
> I think this logic is probably broken in the case of circular
> dependencies, and will end up skipping ctors for some deps due to the
> increment in this line:
> 
> +			p = p->deps[p->next_dep++];
> 
> which happens before the new p's ctors have actually run. Omitting the
> increment here, however, would turn it into an infinite loop. We need
> some way to detect this case and let the dso with an indirect
> dependency on itself run its ctors once all non-circular deps have
> been satisfied. I don't now the right algorithm for this right off,
> though; suggestions would be welcome.

Here's a v2 of the patch with the above issues fixed, and some
comments that hopefully make it make sense. I still think there's more
logic needed to allow concurrent ctors from unrelated dlopen in
multiple threads, though.

Rich

View attachment "ctor_dep_order_v2.diff" of type "text/plain" (2682 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.