Fix BZ #16634.
authorPaul Pluzhnikov <ppluzhnikov@google.com>
Mon, 24 Mar 2014 17:58:26 +0000 (10:58 -0700)
committerPaul Pluzhnikov <ppluzhnikov@google.com>
Mon, 24 Mar 2014 17:58:26 +0000 (10:58 -0700)
An application that erroneously tries to repeatedly dlopen("a.out", ...)
may hit 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 exit with above assertion
failure.

2014-03-24  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.
* elf/Makefile (tests): Add tst-dlopen-aout.
* elf/tst-dlopen-aout.c: New test.

ChangeLog
NEWS
elf/Makefile
elf/dl-load.c
elf/tst-dlopen-aout.c [new file with mode: 0644]

index 745d50a..9937c19 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2014-03-24  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.
+       * elf/Makefile (tests): Add tst-dlopen-aout.
+       * elf/tst-dlopen-aout.c: New test.
+
 2014-03-24  Stefan Liebler <stli@linux.vnet.ibm.com>
 
        [BZ #16714]
diff --git a/NEWS b/NEWS
index a5540f1..79a298f 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -10,9 +10,9 @@ Version 2.20
 * The following bugs are resolved with this release:
 
   15347, 15804, 15894, 16002, 16284, 16447, 16532, 16545, 16574, 16600,
-  16609, 16610, 16611, 16613, 16623, 16632, 16639, 16642, 16649, 16670,
-  16674, 16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707, 16731,
-  16743.
+  16609, 16610, 16611, 16613, 16623, 16632, 16634, 16639, 16642, 16649,
+  16670, 16674, 16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707,
+  16731, 16743.
 
 * Running the testsuite no longer terminates as soon as a test fails.
   Instead, a file tests.sum (xtests.sum from "make xcheck") is generated,
index df138fc..d96cd40 100644 (file)
@@ -148,7 +148,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
         tst-stackguard1 tst-addr1 tst-thrlock \
         tst-unique1 tst-unique2 tst-unique3 tst-unique4 \
         tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
-        tst-ptrguard1
+        tst-ptrguard1 tst-dlopen-aout
 #       reldep9
 test-srcs = tst-pathopt
 selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
@@ -1056,6 +1056,7 @@ tst-leaks1-static-ENV = MALLOC_TRACE=$(objpfx)tst-leaks1-static.mtrace
 $(objpfx)tst-addr1: $(libdl)
 
 $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)
+$(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library)
 
 CFLAGS-ifuncmain1pic.c += $(pic-ccflag)
 CFLAGS-ifuncmain1picstatic.c += $(pic-ccflag)
index 8ebc128..9455df2 100644 (file)
@@ -1667,7 +1667,7 @@ print_search_path (struct r_search_path_elem **list,
    user might want to know about this.  */
 static int
 open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
-            int whatcode, bool *found_other_class, bool free_name)
+            int whatcode, int mode, bool *found_other_class, bool free_name)
 {
   /* This is the expected ELF header.  */
 #define ELF32_CLASS ELFCLASS32
@@ -1843,6 +1843,17 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
          errstring = N_("only ET_DYN and ET_EXEC can be loaded");
          goto call_lose;
        }
+      else if (__glibc_unlikely (ehdr->e_type == ET_EXEC
+                                && (mode & __RTLD_OPENEXEC) == 0))
+       {
+         /* BZ #16634. It is an error to dlopen ET_EXEC (unless
+            __RTLD_OPENEXEC is explicitly set).  We return error here
+            so that code in _dl_map_object_from_fd does not try to set
+            l_tls_modid for this module.  */
+
+         errstring = N_("cannot dynamically load executable");
+         goto call_lose;
+       }
       else if (__builtin_expect (ehdr->e_phentsize, sizeof (ElfW(Phdr)))
               != sizeof (ElfW(Phdr)))
        {
@@ -1928,7 +1939,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
    if MAY_FREE_DIRS is true.  */
 
 static int
-open_path (const char *name, size_t namelen, int secure,
+open_path (const char *name, size_t namelen, int mode,
           struct r_search_path_struct *sps, char **realname,
           struct filebuf *fbp, struct link_map *loader, int whatcode,
           bool *found_other_class)
@@ -1980,8 +1991,8 @@ open_path (const char *name, size_t namelen, int secure,
          if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))
            _dl_debug_printf ("  trying file=%s\n", buf);
 
-         fd = open_verify (buf, fbp, loader, whatcode, found_other_class,
-                           false);
+         fd = open_verify (buf, fbp, loader, whatcode, mode,
+                           found_other_class, false);
          if (this_dir->status[cnt] == unknown)
            {
              if (fd != -1)
@@ -2010,7 +2021,7 @@ open_path (const char *name, size_t namelen, int secure,
          /* Remember whether we found any existing directory.  */
          here_any |= this_dir->status[cnt] != nonexisting;
 
-         if (fd != -1 && __builtin_expect (secure, 0)
+         if (fd != -1 && __builtin_expect (mode & __RTLD_SECURE, 0)
              && INTUSE(__libc_enable_secure))
            {
              /* This is an extra security effort to make sure nobody can
@@ -2184,7 +2195,7 @@ _dl_map_object (struct link_map *loader, const char *name,
          for (l = loader; l; l = l->l_loader)
            if (cache_rpath (l, &l->l_rpath_dirs, DT_RPATH, "RPATH"))
              {
-               fd = open_path (name, namelen, mode & __RTLD_SECURE,
+               fd = open_path (name, namelen, mode,
                                &l->l_rpath_dirs,
                                &realname, &fb, loader, LA_SER_RUNPATH,
                                &found_other_class);
@@ -2200,7 +2211,7 @@ _dl_map_object (struct link_map *loader, const char *name,
              && main_map != NULL && main_map->l_type != lt_loaded
              && cache_rpath (main_map, &main_map->l_rpath_dirs, DT_RPATH,
                              "RPATH"))
-           fd = open_path (name, namelen, mode & __RTLD_SECURE,
+           fd = open_path (name, namelen, mode,
                            &main_map->l_rpath_dirs,
                            &realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
                            &found_other_class);
@@ -2208,7 +2219,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 
       /* Try the LD_LIBRARY_PATH environment variable.  */
       if (fd == -1 && env_path_list.dirs != (void *) -1)
-       fd = open_path (name, namelen, mode & __RTLD_SECURE, &env_path_list,
+       fd = open_path (name, namelen, mode, &env_path_list,
                        &realname, &fb,
                        loader ?: GL(dl_ns)[LM_ID_BASE]._ns_loaded,
                        LA_SER_LIBPATH, &found_other_class);
@@ -2217,7 +2228,7 @@ _dl_map_object (struct link_map *loader, const char *name,
       if (fd == -1 && loader != NULL
          && cache_rpath (loader, &loader->l_runpath_dirs,
                          DT_RUNPATH, "RUNPATH"))
-       fd = open_path (name, namelen, mode & __RTLD_SECURE,
+       fd = open_path (name, namelen, mode,
                        &loader->l_runpath_dirs, &realname, &fb, loader,
                        LA_SER_RUNPATH, &found_other_class);
 
@@ -2267,7 +2278,8 @@ _dl_map_object (struct link_map *loader, const char *name,
                {
                  fd = open_verify (cached,
                                    &fb, loader ?: GL(dl_ns)[nsid]._ns_loaded,
-                                   LA_SER_CONFIG, &found_other_class, false);
+                                   LA_SER_CONFIG, mode, &found_other_class,
+                                   false);
                  if (__glibc_likely (fd != -1))
                    {
                      realname = local_strdup (cached);
@@ -2287,7 +2299,7 @@ _dl_map_object (struct link_map *loader, const char *name,
          && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL
              || __builtin_expect (!(l->l_flags_1 & DF_1_NODEFLIB), 1))
          && rtld_search_dirs.dirs != (void *) -1)
-       fd = open_path (name, namelen, mode & __RTLD_SECURE, &rtld_search_dirs,
+       fd = open_path (name, namelen, mode, &rtld_search_dirs,
                        &realname, &fb, l, LA_SER_DEFAULT, &found_other_class);
 
       /* Add another newline when we are tracing the library loading.  */
@@ -2305,7 +2317,7 @@ _dl_map_object (struct link_map *loader, const char *name,
       else
        {
          fd = open_verify (realname, &fb,
-                           loader ?: GL(dl_ns)[nsid]._ns_loaded, 0,
+                           loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, mode,
                            &found_other_class, true);
          if (__builtin_expect (fd, 0) == -1)
            free (realname);
diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-aout.c
new file mode 100644 (file)
index 0000000..d353425
--- /dev/null
@@ -0,0 +1,61 @@
+/* Test case for BZ #16634.
+
+   Copyright (C) 2014 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/>.
+
+   Note: this test currently only fails when glibc is configured with
+   --enable-hardcoded-path-in-tests.  */
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stdio.h>
+#include <pthread.h>
+
+__thread int x;
+
+void *
+fn (void *p)
+{
+  return p;
+}
+
+int
+main (int argc, char *argv[])
+{
+  int j;
+
+  for (j = 0; j < 100; ++j)
+    {
+      pthread_t thr;
+      void *p;
+      int rc;
+
+      p = dlopen (argv[0], RTLD_LAZY);
+      if (p != NULL)
+        {
+          fprintf (stderr, "dlopen unexpectedly succeeded\n");
+          return 1;
+        }
+      rc = pthread_create (&thr, NULL, fn, NULL);
+      assert (rc == 0);
+
+      rc = pthread_join (thr, NULL);
+      assert (rc == 0);
+    }
+
+  return 0;
+}