[PATCH v2] Set NODELETE flag after checking for NULL pointer

[Aurelien Jarno <aurelien at aurel32 dot net>]

The commit b632bdd3 moved the setting of the DF_1_NODELETE flag earlier
in the dl_open_worker function. However when calling dlopen with both
RTLD_NODELETE and RTLD_NOLOAD, the pointer returned by _dl_map_object is
NULL. This condition is checked just after setting the flag, while it
should be done before. Fix that.

Changelog:
	[BZ #19810]
	* elf/dl-open.c (dl_open_worker): Set DF_1_NODELETE flag later.
	* elf/tst-noload.c: New test case.
	* elf/Makefile (tests): Add tst-noload.
---
 ChangeLog        |  7 ++++++
 elf/Makefile     |  3 ++-
 elf/dl-open.c    | 12 +++++-----
 elf/tst-noload.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 7 deletions(-)
 create mode 100644 elf/tst-noload.c

v1 -> v2:
  I have added a testcase as suggested by Florian Weimer. It tests the
  original issue, but also do some basic tests using RTLD_NOLOAD.

Tested on x86-64, no regression

diff --git a/ChangeLog b/ChangeLog
index 440b021..260f9b7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2016-03-12  Aurelien Jarno  <aurelien@aurel32.net>
+
+	[BZ #19810]
+	* elf/dl-open.c (dl_open_worker): Set DF_1_NODELETE flag later.
+	* elf/tst-noload.c: New test case.
+	* elf/Makefile (tests): Add tst-noload.
+
 2016-03-11  Rajalakshmi Srinivasaraghavan  <raji@linux.vnet.ibm.com>
 
 	* sysdeps/powerpc/powerpc32/power4/memcmp.S (memcmp): Rearrange
diff --git a/elf/Makefile b/elf/Makefile
index 7b6d6d7..5757225 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -149,7 +149,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-nodelete) \
 	 tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
 	 tst-ptrguard1 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened \
-	 tst-nodelete2 tst-audit11 tst-audit12
+	 tst-nodelete2 tst-audit11 tst-audit12 tst-noload
 #	 reldep9
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-dlopen-aout
@@ -546,6 +546,7 @@ $(objpfx)tst-null-argv: $(objpfx)tst-null-argv-lib.so
 $(objpfx)tst-tlsalign: $(objpfx)tst-tlsalign-lib.so
 $(objpfx)tst-nodelete-opened.out: $(objpfx)tst-nodelete-opened-lib.so
 $(objpfx)tst-nodelete-opened: $(libdl)
+$(objpfx)tst-noload: $(libdl)
 
 $(objpfx)tst-tlsalign-extern: $(objpfx)tst-tlsalign-vars.o
 $(objpfx)tst-tlsalign-extern-static: $(objpfx)tst-tlsalign-vars.o
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 6f178b3..3e5df48 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -226,12 +226,6 @@ dl_open_worker (void *a)
   args->map = new = _dl_map_object (call_map, file, lt_loaded, 0,
 				    mode | __RTLD_CALLMAP, args->nsid);
 
-  /* Mark the object as not deletable if the RTLD_NODELETE flags was passed.
-     Do this early so that we don't skip marking the object if it was
-     already loaded.  */
-  if (__glibc_unlikely (mode & RTLD_NODELETE))
-    new->l_flags_1 |= DF_1_NODELETE;
-
   /* If the pointer returned is NULL this means the RTLD_NOLOAD flag is
      set and the object is not already loaded.  */
   if (new == NULL)
@@ -240,6 +234,12 @@ dl_open_worker (void *a)
       return;
     }
 
+  /* Mark the object as not deletable if the RTLD_NODELETE flags was passed.
+     Do this early so that we don't skip marking the object if it was
+     already loaded.  */
+  if (__glibc_unlikely (mode & RTLD_NODELETE))
+    new->l_flags_1 |= DF_1_NODELETE;
+
   if (__glibc_unlikely (mode & __RTLD_SPROF))
     /* This happens only if we load a DSO for 'sprof'.  */
     return;
diff --git a/elf/tst-noload.c b/elf/tst-noload.c
new file mode 100644
index 0000000..21a7141
--- /dev/null
+++ b/elf/tst-noload.c
@@ -0,0 +1,73 @@
+/* Verify that RTLD_NOLOAD works as expected.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <dlfcn.h>
+#include <stdio.h>
+#include <gnu/lib-names.h>
+
+int
+do_test (void)
+{
+  /* Test that no object is loaded with RTLD_NOLOAD.  */
+  void *h1 = dlopen (LIBM_SO, RTLD_LAZY | RTLD_NOLOAD);
+  if (h1 != NULL)
+    {
+      printf ("h1: DSO has been loaded while it should have not\n");
+      return 1;
+    }
+
+  /* This used to segfault in some glibc versions.  */
+  void *h2 = dlopen (LIBM_SO, RTLD_LAZY | RTLD_NOLOAD | RTLD_NODELETE);
+  if (h2 != NULL)
+    {
+      printf ("h2: DSO has been loaded while it should have not\n");
+      return 1;
+    }
+
+  /* Test that loading an already loaded object returns the same.  */
+  void *h3 = dlopen (LIBM_SO, RTLD_LAZY);
+  if (h3 == NULL)
+    {
+      printf ("h3: failed to open DSO: %s\n", dlerror ());
+      return 1;
+    }
+  void *h4 = dlopen (LIBM_SO, RTLD_LAZY | RTLD_NOLOAD);
+  if (h4 == NULL)
+    {
+      printf ("h4: failed to open DSO: %s\n", dlerror ());
+      return 1;
+    }
+  if (h4 != h3)
+    {
+      printf ("h4: should return the same object\n");
+      return 1;
+    }
+
+  /* Cleanup */
+  if (dlclose (h3) != 0)
+    {
+      printf ("h3: dlclose failed: %s\n", dlerror ());
+      return 1;
+    }
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
-- 
2.7.0