[PATCH] elf: Support elf/tst-dlopen-aout in more configurations

[PATCH] elf: Support elf/tst-dlopen-aout in more configurations

[Florian Weimer]

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.

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;
 }
 

Re: [PATCH] elf: Support elf/tst-dlopen-aout in more configurations

[Carlos O'Donell]

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;
>   }
>   
> 


-- 
Cheers,
Carlos.

Re: [PATCH] elf: Support elf/tst-dlopen-aout in more configurations

[Florian Weimer]

* 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