diff mbox series

elf: Support elf/tst-dlopen-aout in more configurations

Message ID 87blwuo3mz.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series elf: Support elf/tst-dlopen-aout in more configurations | expand

Commit Message

Florian Weimer Aug. 12, 2019, 2:32 p.m. UTC
dlopen can no longer open PIE binaries, so it is not necessary
to link the executable as non-PIE to trigger a dlopen failure.

If we hard-code the path to the real executable, we can run the test
with and without hard-coded paths because the dlopen path will not
be recognized as the main program in both cases.  (With an explict
loader invocation, the loader currently adds argv[0] to l_libname
for the main map and the dlopen call suceeds as a result; it does
not do that in standard mode.)

2019-08-12  Florian Weimer  <fweimer@redhat.com>

	* elf/Makefile (tests): Unconditionally add tst-dlopen-aout.
	[$(build-hardcoded-path-in-tests)] (tst-dlopen-aout-no-pie): Do
	not set.
	* elf/tst-dlopen-aout.c: Do not included <assert.h>.
	(do_test): Open the executable using an absolute path.  Print
	error message to standard output.

Comments

Carlos O'Donell Aug. 12, 2019, 3:15 p.m. UTC | #1
On 8/12/19 10:32 AM, Florian Weimer wrote:
> dlopen can no longer open PIE binaries, so it is not necessary
> to link the executable as non-PIE to trigger a dlopen failure.

That fixes one case of the problem, which is a nice cleanup.

> If we hard-code the path to the real executable, we can run the test
> with and without hard-coded paths because the dlopen path will not
> be recognized as the main program in both cases.  (With an explict
> loader invocation, the loader currently adds argv[0] to l_libname
> for the main map and the dlopen call suceeds as a result; it does
> not do that in standard mode.)
> 

OK for master if you remove the wrong comment in tst-dlopen-aout.c.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 2019-08-12  Florian Weimer  <fweimer@redhat.com>
> 
> 	* elf/Makefile (tests): Unconditionally add tst-dlopen-aout.
> 	[$(build-hardcoded-path-in-tests)] (tst-dlopen-aout-no-pie): Do
> 	not set.
> 	* elf/tst-dlopen-aout.c: Do not included <assert.h>.
> 	(do_test): Open the executable using an absolute path.  Print
> 	error message to standard output.
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index a3eefd1b1f..e8c3458963 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -192,7 +192,7 @@ 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-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout

OK. It's great to see this added back.

>   #	 reldep9
>   tests-internal += loadtest unload unload2 circleload1 \
>   	 neededtest neededtest2 neededtest3 neededtest4 \
> @@ -200,10 +200,6 @@ tests-internal += loadtest unload unload2 circleload1 \
>   	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
>   	 tst-create_format1
>   tests-container += tst-pldd
> -ifeq ($(build-hardcoded-path-in-tests),yes)
> -tests += tst-dlopen-aout
> -tst-dlopen-aout-no-pie = yes
> -endif

OK.

>   test-srcs = tst-pathopt
>   selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
>   ifneq ($(selinux-enabled),1)
> diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-aout.c
> index deedd11ad0..4b3e3f111a 100644
> --- a/elf/tst-dlopen-aout.c
> +++ b/elf/tst-dlopen-aout.c
> @@ -23,10 +23,11 @@
>      Note: this test currently only fails when glibc is configured with
>      --enable-hardcoded-path-in-tests.  */

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This comment needs removing now, not to mention it's wrong.

>   
> -#include <assert.h>
>   #include <dlfcn.h>
> -#include <stdio.h>
>   #include <pthread.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <support/support.h>

OK.

>   #include <support/xthread.h>
>   
>   __thread int x;
> @@ -42,15 +43,21 @@ do_test (int argc, char *argv[])
>   {
>     int j;
>   
> +  /* Use the full path so that the dynamic loader does not recognize
> +     the main program as already loaded (even with an explicit ld.so
> +     invocation).  */
> +  char *path = xasprintf ("%s/%s", support_objdir_root, "tst-dlopen-aout");
> +  printf ("info: dlopen object: %s\n", path);

OK.

> +
>     for (j = 0; j < 100; ++j)
>       {
>         pthread_t thr;
>         void *p;
>   
> -      p = dlopen (argv[0], RTLD_LAZY);
> +      p = dlopen (path, RTLD_LAZY);

OK.

>         if (p != NULL)
>           {
> -          fprintf (stderr, "dlopen unexpectedly succeeded\n");
> +          puts ("error: dlopen succeeded unexpectedly");

OK.

>             return 1;
>           }
>         /* We create threads to force TLS allocation, which triggers
> @@ -60,6 +67,7 @@ do_test (int argc, char *argv[])
>         xpthread_join (thr);
>       }
>   
> +  free (path);
>     return 0;
>   }
>   
>
Florian Weimer Aug. 12, 2019, 6:32 p.m. UTC | #2
* Florian Weimer:

> +  /* Use the full path so that the dynamic loader does not recognize
> +     the main program as already loaded (even with an explicit ld.so
> +     invocation).  */
> +  char *path = xasprintf ("%s/%s", support_objdir_root, "tst-dlopen-aout");

The computed path is not correct, the elf/ component is missing.  So
this test does not the right thing anymore.

I'm working on fixing the explict loader invocation difference, then
this won't matter.

Thanks,
Florian
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index a3eefd1b1f..e8c3458963 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -192,7 +192,7 @@  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-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
@@ -200,10 +200,6 @@  tests-internal += loadtest unload unload2 circleload1 \
 	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
 	 tst-create_format1
 tests-container += tst-pldd
-ifeq ($(build-hardcoded-path-in-tests),yes)
-tests += tst-dlopen-aout
-tst-dlopen-aout-no-pie = yes
-endif
 test-srcs = tst-pathopt
 selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
 ifneq ($(selinux-enabled),1)
diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-aout.c
index deedd11ad0..4b3e3f111a 100644
--- a/elf/tst-dlopen-aout.c
+++ b/elf/tst-dlopen-aout.c
@@ -23,10 +23,11 @@ 
    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>
+#include <stdio.h>
+#include <stdlib.h>
+#include <support/support.h>
 #include <support/xthread.h>
 
 __thread int x;
@@ -42,15 +43,21 @@  do_test (int argc, char *argv[])
 {
   int j;
 
+  /* Use the full path so that the dynamic loader does not recognize
+     the main program as already loaded (even with an explicit ld.so
+     invocation).  */
+  char *path = xasprintf ("%s/%s", support_objdir_root, "tst-dlopen-aout");
+  printf ("info: dlopen object: %s\n", path);
+
   for (j = 0; j < 100; ++j)
     {
       pthread_t thr;
       void *p;
 
-      p = dlopen (argv[0], RTLD_LAZY);
+      p = dlopen (path, RTLD_LAZY);
       if (p != NULL)
         {
-          fprintf (stderr, "dlopen unexpectedly succeeded\n");
+          puts ("error: dlopen succeeded unexpectedly");
           return 1;
         }
       /* We create threads to force TLS allocation, which triggers
@@ -60,6 +67,7 @@  do_test (int argc, char *argv[])
       xpthread_join (thr);
     }
 
+  free (path);
   return 0;
 }