[PATCH][V3][BZ #17833][PING] _dl_close_worker() does not release inconsistent objects.

[Pavel Kopyl <p dot kopyl at samsung dot com>]

On 03/03/2015 12:13 PM, Pavel Kopyl wrote:
> On 03/01/2015 10:17 PM, Mike Frysinger wrote:
> > On 27 Feb 2015 16:32, Pavel Kopyl wrote:
> > > --- /dev/null
> > > +++ b/elf/tst-unique5lib.cc
> > > @@ -0,0 +1,13 @@
> > > +
> > i know existing tests are bad examples, but lets try and start fixing 
> > that.
> > namely, there should be a header here giving a quick overview of what 
> > it is
> > exactly you're testing for, and a BZ reference.
> >
> > > +extern int not_exist ();
> > > +
> > > +inline int make_unique ()
> > > +{
> > > +  static int unique;
> > > +  return ++unique;
> > > +}
> > > +
> > > +int foo ()
> > > +{
> > > +  return make_unique () + not_exist ();
> > > +}
> > i don't know if this is just copy & pasting, but prototypes that do 
> > not intend
> > to take args should always be (void).
> > -mike
>
> Thanks for review, I fixed that in patch v3.

Ping.

commit 0d8ff75e18fad28123dca0df29f23942299dac56
Author: Pavel Kopyl <p.kopyl@samsung.com>
Date:   Mon Mar 2 23:05:48 2015 +0300

    2015-03-02  Pavel Kopyl  <p.kopyl@samsung.com>
    	    Mikhail Ilin  <m.ilin@samsung.com>
    
    	[BZ #17833]
    	* elf/Makefile: Add build test commands.
    	* elf/dl-close.c (_dl_close_worker): Implement force object deletion.
    	* elf/dl-open.c (_dl_open): Forced _dl_close_worker call.
    	(_dl_close): Default _dl_close_worker call.
    	* elf/tst-unique5.cc: Test executable.  New file.
    	* elf/tst-unique5lib.cc: Test library.  New file.
    	* include/dlfcn.h (_dl_close_worker): Add new parameter.

diff --git a/elf/Makefile b/elf/Makefile
index 711beed..f3aaf4d 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -144,7 +144,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
 	 unload3 unload4 unload5 unload6 unload7 unload8 tst-global1 order2 \
 	 tst-audit1 tst-audit2 tst-audit8 tst-audit9 \
 	 tst-stackguard1 tst-addr1 tst-thrlock \
-	 tst-unique1 tst-unique2 tst-unique3 tst-unique4 \
+	 tst-unique1 tst-unique2 tst-unique3 tst-unique4 tst-unique5 \
 	 tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
 	 tst-ptrguard1
 #	 reldep9
@@ -206,7 +206,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
 		tst-unique3lib tst-unique3lib2 \
-		tst-unique4lib \
+		tst-unique4lib tst-unique5lib \
 		tst-initordera1 tst-initorderb1 \
 		tst-initordera2 tst-initorderb2 \
 		tst-initordera3 tst-initordera4 \
@@ -580,6 +580,7 @@ ifuncmod5.so-no-z-defs = yes
 ifuncmod6.so-no-z-defs = yes
 tst-auditmod9a.so-no-z-defs = yes
 tst-auditmod9b.so-no-z-defs = yes
+tst-unique5lib.so-no-z-defs = yes
 
 ifeq ($(build-shared),yes)
 # Build all the modules even when not actually running test programs.
@@ -1138,6 +1139,9 @@ $(objpfx)tst-unique3.out: $(objpfx)tst-unique3lib2.so
 
 $(objpfx)tst-unique4: $(objpfx)tst-unique4lib.so
 
+$(objpfx)tst-unique5: $(libdl)
+$(objpfx)tst-unique5.out: $(objpfx)tst-unique5lib.so
+
 $(objpfx)tst-initorder-cmp.out: tst-initorder.exp $(objpfx)tst-initorder.out
 	cmp $^ > $@; \
 	$(evaluate-test)
diff --git a/elf/dl-close.c b/elf/dl-close.c
index cf8f9e0..d640254 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -108,7 +108,7 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
 
 
 void
-_dl_close_worker (struct link_map *map)
+_dl_close_worker (struct link_map *map, bool force)
 {
   /* One less direct use.  */
   --map->l_direct_opencount;
@@ -152,6 +152,10 @@ _dl_close_worker (struct link_map *map)
       l->l_idx = idx;
       maps[idx] = l;
       ++idx;
+
+      /* clear DF_1_NODELETE to force object deletion.  */
+      if (force)
+	l->l_flags_1 &= ~DF_1_NODELETE;
     }
   assert (idx == nloaded);
 
@@ -635,6 +639,31 @@ _dl_close_worker (struct link_map *map)
 		}
 	    }
 
+	  /* Reset unique symbols if forced.  */
+	  if (force)
+	    {
+	      struct unique_sym_table *tab = &ns->_ns_unique_sym_table;
+	      __rtld_lock_lock_recursive (tab->lock);
+	      struct unique_sym *entries = tab->entries;
+	      if (entries != NULL)
+		{
+		  size_t idx, size = tab->size;
+		  for (idx = 0; idx < size; ++idx)
+		    {
+		      /* Clear unique symbol entries
+		       that belong to this object.  */
+		      if (entries[idx].name != NULL
+			  && entries[idx].map == imap)
+			{
+			  entries[idx].name = NULL;
+			  entries[idx].hashval = 0;
+			  tab->n_elements--;
+			}
+		    }
+		}
+		__rtld_lock_unlock_recursive (tab->lock);
+	    }
+
 	  /* We can unmap all the maps at once.  We determined the
 	     start address and length when we loaded the object and
 	     the `munmap' call does the rest.  */
@@ -772,7 +801,7 @@ _dl_close (void *_map)
   /* Acquire the lock.  */
   __rtld_lock_lock_recursive (GL(dl_load_lock));
 
-  _dl_close_worker (map);
+  _dl_close_worker (map, false);
 
   __rtld_lock_unlock_recursive (GL(dl_load_lock));
 }
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 47b4cb5..d71c454 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -674,7 +674,7 @@ no more namespaces available for dlmopen()"));
 	  if ((mode & __RTLD_AUDIT) == 0)
 	    GL(dl_tls_dtv_gaps) = true;
 
-	  _dl_close_worker (args.map);
+	  _dl_close_worker (args.map, true);
 	}
 
       assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
diff --git a/elf/tst-unique5.cc b/elf/tst-unique5.cc
new file mode 100644
index 0000000..628897a
--- /dev/null
+++ b/elf/tst-unique5.cc
@@ -0,0 +1,15 @@
+#include "../dlfcn/dlfcn.h"
+#include <stdlib.h>
+
+/* unique5lib.so library contains both STB_GNU_UNIQUE and undefined symbols.
+   First dlopen call fails because of undefined symbols.  Test checks that
+   library does not remain locked in the memory in this case. [BZ #17833]  */
+
+int
+main (void)
+{
+  void *h;
+  dlopen ("$ORIGIN/tst-unique5lib.so", RTLD_NOW);
+  h = dlopen ("$ORIGIN/tst-unique5lib.so", RTLD_LAZY | RTLD_NOLOAD);
+  return h != NULL;
+}
diff --git a/elf/tst-unique5lib.cc b/elf/tst-unique5lib.cc
new file mode 100644
index 0000000..c292a23
--- /dev/null
+++ b/elf/tst-unique5lib.cc
@@ -0,0 +1,14 @@
+/* tst-unique5lib.so library for tst-unique5.cc.  */
+
+extern int not_exist (void);
+
+inline int make_unique (void)
+{
+  static int unique;
+  return ++unique;
+}
+
+int foo (void)
+{
+  return make_unique () + not_exist ();
+}
diff --git a/include/dlfcn.h b/include/dlfcn.h
index a67b2e3..0ce0af5 100644
--- a/include/dlfcn.h
+++ b/include/dlfcn.h
@@ -54,7 +54,8 @@ struct link_map;
 extern void _dl_close (void *map) attribute_hidden;
 /* Same as above, but without locking and safety checks for user
    provided map arguments.  */
-extern void _dl_close_worker (struct link_map *map) attribute_hidden;
+extern void _dl_close_worker (struct link_map *map, bool force)
+    attribute_hidden;
 
 /* Look up NAME in shared object HANDLE (which may be RTLD_DEFAULT or
    RTLD_NEXT).  WHO is the calling function, for RTLD_NEXT.  Returns