diff mbox series

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

Message ID 87v9so20im.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series Split self-dlopen tests from elf/tst-dlopen-aout | expand

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
diff mbox series

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>