misc/151861: dlclose() of library causes separately opened libraries to unload as well

[Kostik Belousov]

The following reply was made to PR misc/151861; it has been noted by GNATS.

From: Kostik Belousov <kostikbel at gmail.com>
To: Jaakko Heinonen <jh at freebsd.org>
Cc: Arjan van Leeuwen <freebsd-maintainer at opera.com>, bug-followup at freebsd.org,
        kan at freebsd.org
Subject: Re: misc/151861: dlclose() of library causes separately opened libraries to unload as well
Date: Tue, 2 Nov 2010 22:13:06 +0200

 --kQl6KHw6oLQq4hVK
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable
 
 On Tue, Nov 02, 2010 at 07:36:54PM +0200, Jaakko Heinonen wrote:
 > On 2010-11-01, Arjan van Leeuwen wrote:
 > > Assume we have a library liba.so, containing a function a(), and a
 > > library libb.so, containing function b(). liba.so needs functionality
 > > from libb.so, so liba.so links in libb.so.
 > >=20
 > > An application doesn't know about the relation between these
 > > libraries, but needs to call a() and b(). It dlopen()s libb.so and
 > > obtains a pointer to b(), and it dlopen()s liba.so and obtains a
 > > pointer to a().
 > >=20
 > > As soon as the application doesn't need a() anymore, it dlclose()s
 > > liba.so.
 > >=20
 > > Expected result: the pointer to b() is still valid and can be called
 > > Actual result: the pointer to b() has become invalid, even though the
 > > application did not dlclose() the handle to libb.so. On calling b(),
 > > the application crashes with a segmentation fault.
 > >=20
 > > Extract the attached shar archive and execute 'make test'.
 >=20
 > Thank you for providing the test case.
 >=20
 > > This will cause a crash on FreeBSD, and will print 'success' on Linux.
 >=20
 > There is a problem with reference counting in dlopen(). If an object has
 > been loaded by load_needed_objects() its dagmembers list may be empty
 > after loading. If the list is empty, the ref_dag() call done for already
 > loaded objects in dlopen() doesn't have effect.
 >=20
 > Here is a patch to demonstrate the problem. The test passes with the
 > patch applied.
 >=20
 > %%%
 > Index: libexec/rtld-elf/rtld.c
 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 > --- libexec/rtld-elf/rtld.c	(revision 214676)
 > +++ libexec/rtld-elf/rtld.c	(working copy)
 > @@ -2046,7 +2046,10 @@ dlopen(const char *name, int mode)
 >  	} else {
 > =20
 >  	    /* Bump the reference counts for objects on this DAG. */
 > -	    ref_dag(obj);
 > +	    if (STAILQ_EMPTY(&obj->dagmembers))
 > +		init_dag(obj);
 > +	    else
 > +		ref_dag(obj);
 > =20
 >  	    if (ld_tracing)
 >  		goto trace;
 > %%%
 >=20
 > I have cc'd kan@ and kib at . Do you have ideas how to fix this correctly?
 
 Why do you think that your patch is not correct ?
 I feel that more explicit handling of the state of the DAG is cleaner.
 
 diff --git a/libexec/rtld-elf/rtld.c b/libexec/rtld-elf/rtld.c
 index 8bc8692..f1ffc3e 100644
 --- a/libexec/rtld-elf/rtld.c
 +++ b/libexec/rtld-elf/rtld.c
 @@ -1275,8 +1275,11 @@ init_dag(Obj_Entry *root)
  {
      DoneList donelist;
 =20
 +    if (root->dag_inited)
 +	    return;
      donelist_init(&donelist);
      init_dag1(root, root, &donelist);
 +    root->dag_inited =3D true;
  }
 =20
  static void
 @@ -2045,8 +2048,16 @@ dlopen(const char *name, int mode)
  	    }
  	} else {
 =20
 -	    /* Bump the reference counts for objects on this DAG. */
 -	    ref_dag(obj);
 +	    /*
 +	     * Bump the reference counts for objects on this DAG.  If
 +	     * this is the first dlopen() call for the object that was
 +	     * already loaded as a dependency, initialize the dag
 +	     * starting at it.
 +	     */
 +	    if (obj->dl_refcount =3D=3D 1)
 +		init_dag(obj);
 +	    else
 +		ref_dag(obj);
 =20
  	    if (ld_tracing)
  		goto trace;
 diff --git a/libexec/rtld-elf/rtld.h b/libexec/rtld-elf/rtld.h
 index 6bae2f0..faa84e2 100644
 --- a/libexec/rtld-elf/rtld.h
 +++ b/libexec/rtld-elf/rtld.h
 @@ -222,6 +222,7 @@ typedef struct Struct_Obj_Entry {
      bool ref_nodel : 1;		/* Refcount increased to prevent dlclose */
      bool init_scanned: 1;	/* Object is already on init list. */
      bool on_fini_list: 1;	/* Object is already on fini list. */
 +    bool dag_inited : 1;	/* Object has its DAG initialized. */
 =20
      struct link_map linkmap;	/* For GDB and dlinfo() */
      Objlist dldags;		/* Object belongs to these dlopened DAGs (%) */
 
 --kQl6KHw6oLQq4hVK
 Content-Type: application/pgp-signature
 Content-Disposition: inline
 
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.11 (FreeBSD)
 
 iEYEARECAAYFAkzQcNEACgkQC3+MBN1Mb4jABgCg7KYATJ1CUDLOwOPHYufcx3P6
 lXEAnjRvZPn3H1yYOZnFO9htGSIFywnj
 =Kfrv
 -----END PGP SIGNATURE-----
 
 --kQl6KHw6oLQq4hVK--