Split self-dlopen tests from elf/tst-dlopen-aout

Message ID 87v9so20im.fsf@oldenburg2.str.redhat.com
State Superseded
Headers

Commit Message

Florian Weimer Oct. 16, 2019, 3:06 p.m. UTC
  From the beginning, elf/tst-dlopen-aout has exercised two different
bugs: (a) failure to report errors for a dlopen of the executable
itself in some cases (bug 24900) and (b) incorrect rollback of the
TLS modid allocation in case of a dlopen failure (bug 16634).

This patch retains elf/tst-dlopen-aout for (b) and introduces a new
set of tests, elf/tst-dlopen-self, for (a).  The elf/tst-dlopen-aout
tests use the elf/tst-dlopen-self binaries (or iconv), so they are
no longer self-dlopen tests.

Tested on x86_64-linux-gnu and i686-linux-gnu, with a toolchain that
does not default to PIE.

-----
 elf/Makefile                    | 19 +++++++--
 elf/tst-dlopen-aout-container.c | 22 ++++++++++-
 elf/tst-dlopen-aout-pie.c       |  3 +-
 elf/tst-dlopen-aout.c           | 66 ++-----------------------------
 elf/tst-dlopen-aout.h           | 87 +++++++++++++++++++++++++++++++++++++++++
 elf/tst-dlopen-self-container.c | 19 +++++++++
 elf/tst-dlopen-self-pie.c       | 19 +++++++++
 elf/tst-dlopen-self.c           | 55 ++++++++++++++++++++++++++
 8 files changed, 221 insertions(+), 69 deletions(-)
  

Comments

Zack Weinberg Oct. 16, 2019, 5:10 p.m. UTC | #1
On Wed, Oct 16, 2019 at 11:06 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> From the beginning, elf/tst-dlopen-aout has exercised two different
> bugs: (a) failure to report errors for a dlopen of the executable
> itself in some cases (bug 24900) and (b) incorrect rollback of the
> TLS modid allocation in case of a dlopen failure (bug 16634).
>
> This patch retains elf/tst-dlopen-aout for (b) and introduces a new
> set of tests, elf/tst-dlopen-self, for (a).  The elf/tst-dlopen-aout
> tests use the elf/tst-dlopen-self binaries (or iconv), so they are
> no longer self-dlopen tests.

I don't know the dynamic linker well enough to review this change in
detail.  I like the idea of splitting up these two tests.  I want to
suggest that the (b) test should be renamed to something more
descriptive at the same time as (a) is moved to its own binary.
tst-dlopen-aout sounds like it tests something to do with an ELF
executable trying to dlopen() an a.out-format shared library.  (I
don't know if that was ever a thing that worked, but I could easily
believe that some ancient attempt to make it work had backward
compatibility implications that we're still stuck with, at least on
the older ABIs.)

zw
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index dea51ca182..39b7be7ff4 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -192,14 +192,15 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \
 	 tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \
 	 tst-unwind-ctor tst-unwind-main tst-audit13 \
-	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout
+	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout \
+	 tst-dlopen-self
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
 	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
 	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
 	 tst-create_format1
-tests-container += tst-pldd tst-dlopen-aout-container
+tests-container += tst-pldd tst-dlopen-aout-container tst-dlopen-self-container
 test-srcs = tst-pathopt
 selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
 ifneq ($(selinux-enabled),1)
@@ -308,8 +309,9 @@  test-xfail-tst-protected1b = yes
 endif
 ifeq (yesyes,$(have-fpie)$(build-shared))
 modules-names += tst-piemod1
-tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-dlopen-aout-pie
-tests-pie += tst-pie1 tst-pie2 tst-dlopen-aout-pie
+tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-dlopen-aout-pie \
+  tst-dlopen-self-pie
+tests-pie += tst-pie1 tst-pie2 tst-dlopen-aout-pie tst-dlopen-self-pie
 ifeq (yes,$(have-protected-data))
 tests += vismain
 tests-pie += vismain
@@ -1270,11 +1272,20 @@  $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)
 
 tst-tst-dlopen-aout-no-pie = yes
 $(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library)
+$(objpfx)tst-dlopen-aout.out: $(objpfx)tst-dlopen-self
 CFLAGS-tst-dlopen-aout-pie.c += $(pie-ccflag)
 $(objpfx)tst-dlopen-aout-pie: $(libdl) $(shared-thread-library)
+$(objpfx)tst-dlopen-aout-pie.out: $(objpfx)tst-dlopen-self-pie
 $(objpfx)tst-dlopen-aout-container: $(libdl) $(shared-thread-library)
 LDFLAGS-tst-dlopen-aout-container += -Wl,-rpath,\$$ORIGIN
 
+tst-tst-dlopen-self-no-pie = yes
+$(objpfx)tst-dlopen-self: $(libdl)
+CFLAGS-tst-dlopen-self-pie.c += $(pie-ccflag)
+$(objpfx)tst-dlopen-self-pie: $(libdl)
+$(objpfx)tst-dlopen-self-container: $(libdl)
+LDFLAGS-tst-dlopen-self-container += -Wl,-rpath,\$$ORIGIN
+
 CFLAGS-ifuncmain1pic.c += $(pic-ccflag)
 CFLAGS-ifuncmain1picstatic.c += $(pic-ccflag)
 CFLAGS-ifuncmain1staticpic.c += $(pic-ccflag)
diff --git a/elf/tst-dlopen-aout-container.c b/elf/tst-dlopen-aout-container.c
index 702dbe04c4..af282aedec 100644
--- a/elf/tst-dlopen-aout-container.c
+++ b/elf/tst-dlopen-aout-container.c
@@ -16,4 +16,24 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include "tst-dlopen-aout.c"
+/* This test use the iconv program as the test binary.  */
+
+#include <stdlib.h>
+#include <support/support.h>
+
+static char *iconv_path;
+
+static __attribute__ ((constructor)) void
+iconv_path_init (void)
+{
+  iconv_path = xasprintf ("%s/iconv", support_bindir_prefix);
+}
+
+static __attribute__ ((destructor)) void
+iconv_path_fini (void)
+{
+  free (iconv_path);
+}
+
+#define TST_DLOPEN_AOUT_PATH iconv_path
+#include "tst-dlopen-aout.h"
diff --git a/elf/tst-dlopen-aout-pie.c b/elf/tst-dlopen-aout-pie.c
index 8d2009c0f3..fb675bfb9d 100644
--- a/elf/tst-dlopen-aout-pie.c
+++ b/elf/tst-dlopen-aout-pie.c
@@ -16,4 +16,5 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include "tst-dlopen-aout.c"
+#define TST_DLOPEN_AOUT_PATH "tst-dlopen-self-pie"
+#include "tst-dlopen-aout.h"
diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-aout.c
index b86d082bc1..95b0e005c9 100644
--- a/elf/tst-dlopen-aout.c
+++ b/elf/tst-dlopen-aout.c
@@ -1,4 +1,4 @@ 
-/* Test case for BZ #16634 and BZ#24900.
+/* Test case for BZ #16634.  Non-PIE version.
 
    Verify that incorrectly dlopen()ing an executable without
    __RTLD_OPENEXEC does not cause assertion in ld.so, and that it
@@ -21,65 +21,5 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <dlfcn.h>
-#include <pthread.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <support/check.h>
-#include <support/support.h>
-#include <support/xthread.h>
-
-__thread int x;
-
-void *
-fn (void *p)
-{
-  return p;
-}
-
-/* Call dlopen on PATH and check that fails with an error message
-   indicating an attempt to open an ET_EXEC or PIE object.  */
-static void
-check_dlopen_failure (const char *path)
-{
-  void *handle = dlopen (path, RTLD_LAZY);
-  if (handle != NULL)
-    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", path);
-
-  const char *message = dlerror ();
-  TEST_VERIFY_EXIT (message != NULL);
-  if ((strstr (message,
-	       "cannot dynamically load position-independent executable")
-       == NULL)
-      && strstr (message, "cannot dynamically load executable") == NULL)
-    FAIL_EXIT1 ("invalid dlopen error message: \"%s\"", message);
-}
-
-static int
-do_test (int argc, char *argv[])
-{
-  int j;
-
-  for (j = 0; j < 100; ++j)
-    {
-      pthread_t thr;
-
-      check_dlopen_failure (argv[0]);
-
-      /* We create threads to force TLS allocation, which triggers
-	 the original bug i.e. running out of surplus slotinfo entries
-	 for TLS.  */
-      thr = xpthread_create (NULL, fn, NULL);
-      xpthread_join (thr);
-    }
-
-  /* The elf subdirectory (or $ORIGIN in the container case) is on the
-     library search path.  */
-  check_dlopen_failure ("tst-dlopen-aout");
-
-  return 0;
-}
-
-#define TEST_FUNCTION_ARGV do_test
-#include <support/test-driver.c>
+#define TST_DLOPEN_AOUT_PATH "tst-dlopen-self"
+#include "tst-dlopen-aout.h"
diff --git a/elf/tst-dlopen-aout.h b/elf/tst-dlopen-aout.h
new file mode 100644
index 0000000000..0352dae8d6
--- /dev/null
+++ b/elf/tst-dlopen-aout.h
@@ -0,0 +1,87 @@ 
+/* Common code for tst-dlopen-aout, tst-dlopen-aout-pie,
+   tst-dlopen-aout-container.
+
+   Verify that incorrectly dlopen()ing an executable without
+   __RTLD_OPENEXEC does not cause assertion in ld.so, and that it
+   actually results in an error.
+
+   Copyright (C) 2014-2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+/* Before including this file, the macro TST_DLOPEN_AOUT_PATH must be
+   defined, to specify the path used for the open operation.  */
+
+#include <dlfcn.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xthread.h>
+
+__thread int x;
+
+void *
+fn (void *p)
+{
+  return p;
+}
+
+/* Call dlopen and check that fails with an error message indicating
+   an attempt to open an ET_EXEC or PIE object.  */
+static void
+check_dlopen_failure (void)
+{
+  void *handle = dlopen (TST_DLOPEN_AOUT_PATH, RTLD_LAZY);
+  if (handle != NULL)
+    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", TST_DLOPEN_AOUT_PATH);
+
+  const char *message = dlerror ();
+  TEST_VERIFY_EXIT (message != NULL);
+  if ((strstr (message,
+	       "cannot dynamically load position-independent executable")
+       == NULL)
+      && strstr (message, "cannot dynamically load executable") == NULL)
+    FAIL_EXIT1 ("invalid dlopen error message: \"%s\"", message);
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+  int j;
+
+  for (j = 0; j < 100; ++j)
+    {
+      pthread_t thr;
+
+      check_dlopen_failure ();
+
+      /* We create threads to force TLS allocation, which triggers
+	 the original bug i.e. running out of surplus slotinfo entries
+	 for TLS.  */
+      thr = xpthread_create (NULL, fn, NULL);
+      xpthread_join (thr);
+    }
+
+  check_dlopen_failure ();
+
+  return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/elf/tst-dlopen-self-container.c b/elf/tst-dlopen-self-container.c
new file mode 100644
index 0000000000..b1db238dec
--- /dev/null
+++ b/elf/tst-dlopen-self-container.c
@@ -0,0 +1,19 @@ 
+/* Check dlopen'ing the executable itself fails (bug 24900); container version.
+   Copyright (C) 2014-2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include "tst-dlopen-self.c"
diff --git a/elf/tst-dlopen-self-pie.c b/elf/tst-dlopen-self-pie.c
new file mode 100644
index 0000000000..855bccc5b5
--- /dev/null
+++ b/elf/tst-dlopen-self-pie.c
@@ -0,0 +1,19 @@ 
+/* Check that dlopen'ing the executable itself fails (bug 24900); PIE version.
+   Copyright (C) 2014-2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include "tst-dlopen-self.c"
diff --git a/elf/tst-dlopen-self.c b/elf/tst-dlopen-self.c
new file mode 100644
index 0000000000..310f1b09d4
--- /dev/null
+++ b/elf/tst-dlopen-self.c
@@ -0,0 +1,55 @@ 
+/* Check that dlopen'ing the executable itself fails (bug 24900).
+   Copyright (C) 2014-2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <dlfcn.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+
+/* Call dlopen and check that fails with an error message indicating
+   an attempt to open an ET_EXEC or PIE object.  */
+static void
+check_dlopen_failure (const char *path)
+{
+  void *handle = dlopen (path, RTLD_LAZY);
+  if (handle != NULL)
+    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", path);
+
+  const char *message = dlerror ();
+  TEST_VERIFY_EXIT (message != NULL);
+  if ((strstr (message,
+	       "cannot dynamically load position-independent executable")
+       == NULL)
+      && strstr (message, "cannot dynamically load executable") == NULL)
+    FAIL_EXIT1 ("invalid dlopen error message: \"%s\"", message);
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+  check_dlopen_failure (argv[0]);
+
+  char *full_path = realpath (argv[0], NULL);
+  check_dlopen_failure  (full_path);
+  free (full_path);
+
+  return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>