This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

[PATCH] Restore dlsym(RTLD_NEXT, ...) behaviour (take 3)


On Wed, May 16, 2001 at 10:48:21PM -0700, Ulrich Drepper wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > Ok, here are two variants of the patch.
> 
> Finally managed to look at the patches... and don't like them.  Code like
> 
> 
> +             if (! _dl_loaded
> +                 || _dl_loaded->l_addr != 0
> +                 || caller < _dl_loaded->l_map_start)
> 
> 
> makes certain assumptions about the memory layout and load addresses.
> This should not happen.  There is no reason why the application code
> should always be below all dynamically loaded code.  In fact, isn't
> the x86 emulation on IA-64 doing something funny like this?

How about this?
Passed glibc bootstrap and make check.
I couldn't find any code which would rely on l_map_end of main program being
set to ~0 and it does not cost that much to initialize it properly.
Further, the l->l_addr != 0 checks might work now but with prelinking they
don't work at all (l_addr == 0 is then the common desirable case).
In dl-open.c the comment close to the l_addr != 0 check said:
  Make sure we do not currently set this map up in this moment.
but this test happens at the start of dl_open_worker, so this dlopen has not
created any new maps yet and as it is protected with the _dl_load_lock,
there cannot be anyone else creating/removing maps at the same time and all
maps should be thus set up properly before dl_open_worker starts.

2001-05-21  Jakub Jelinek  <jakub@redhat.com>

	* elf/rtld.c (dl_main): Compute l_map_end for the main program.
	* elf/dlsym.c (_dl_sym): Don't check for l_addr == 0.
	If match == _dl_loaded, caller can still come from the main program.
	(_dl_vsym): Likewise.
	* elf/dl-open.c (dl_open_worker): Don't check for l_addr == 0.
	* elf/dl-error.c (_dl_signal_error): Change NULL objname into "".
	* elf/restest2.c: New test.
	* elf/Makefile (tests): Add restest2.
	(restest2, LDFLAGS-restest2): Add.

--- libc/elf/rtld.c.jj	Mon May 21 11:36:03 2001
+++ libc/elf/rtld.c	Mon May 21 11:42:24 2001
@@ -542,8 +542,7 @@ of this helper program; chances are you 
 	 information for the program.  */
     }
 
-  /* It is not safe to load stuff after the main program.  */
-  _dl_loaded->l_map_end = ~0;
+  _dl_loaded->l_map_end = 0;
   /* Perhaps the executable has no PT_LOAD header entries at all.  */
   _dl_loaded->l_map_start = ~0;
 
@@ -593,13 +592,18 @@ of this helper program; chances are you 
       case PT_LOAD:
 	/* Remember where the main program starts in memory.  */
 	{
-	  ElfW(Addr) mapstart;
+	  ElfW(Addr) mapstart, allocend;
 	  mapstart = _dl_loaded->l_addr + (ph->p_vaddr & ~(ph->p_align - 1));
+	  allocend = _dl_loaded->l_addr + ph->p_vaddr + ph->p_memsz;
 	  if (_dl_loaded->l_map_start > mapstart)
 	    _dl_loaded->l_map_start = mapstart;
+	  if (_dl_loaded->l_map_end < allocend)
+	    _dl_loaded->l_map_end = allocend;
 	}
 	break;
       }
+  if (! _dl_loaded->l_map_end)
+    _dl_loaded->l_map_end = ~0;
   if (! _dl_rtld_map.l_libname && _dl_rtld_map.l_name)
     {
       /* We were invoked directly, so the program might not have a
--- libc/elf/dl-sym.c.jj	Mon May 21 11:36:03 2001
+++ libc/elf/dl-sym.c	Mon May 21 11:42:24 2001
@@ -41,7 +41,7 @@ _dl_sym (void *handle, const char *name,
 
   /* Find the highest-addressed object that CALLER is not below.  */
   for (l = _dl_loaded; l != NULL; l = l->l_next)
-    if (l->l_addr != 0 && caller >= l->l_map_start && caller < l->l_map_end)
+    if (caller >= l->l_map_start && caller < l->l_map_end)
       {
 	/* There must be exactly one DSO for the range of the virtual
 	   memory.  Otherwise something is really broken.  */
@@ -65,8 +65,13 @@ _dl_sym (void *handle, const char *name,
       else
 	{
 	  if (__builtin_expect (match == _dl_loaded, 0))
-	    _dl_signal_error (0, NULL, N_("\
+	    {
+	      if (! _dl_loaded
+		  || caller < _dl_loaded->l_map_start
+		  || caller >= _dl_loaded->l_map_end)
+	        _dl_signal_error (0, NULL, N_("\
 RTLD_NEXT used in code not dynamically loaded"));
+	    }
 
 	  l = match;
 	  while (l->l_loader != NULL)
@@ -107,7 +112,7 @@ _dl_vsym (void *handle, const char *name
 
   /* Find the highest-addressed object that CALLER is not below.  */
   for (l = _dl_loaded; l != NULL; l = l->l_next)
-    if (l->l_addr != 0 && caller >= l->l_map_start && caller < l->l_map_end)
+    if (caller >= l->l_map_start && caller < l->l_map_end)
       {
 	/* There must be exactly one DSO for the range of the virtual
 	   memory.  Otherwise something is really broken.  */
@@ -121,9 +126,14 @@ _dl_vsym (void *handle, const char *name
 					  &vers, 0, 0);
   else if (handle == RTLD_NEXT)
     {
-      if (match == _dl_loaded)
-	_dl_signal_error (0, NULL, N_("\
+      if (__builtin_expect (match == _dl_loaded, 0))
+	{
+	  if (! _dl_loaded
+	      || caller < _dl_loaded->l_map_start
+	      || caller >= _dl_loaded->l_map_end)
+	    _dl_signal_error (0, NULL, N_("\
 RTLD_NEXT used in code not dynamically loaded"));
+	}
 
       l = match;
       while (l->l_loader != NULL)
--- libc/elf/dl-open.c.jj	Mon May 21 11:36:02 2001
+++ libc/elf/dl-open.c	Mon May 21 11:42:24 2001
@@ -188,13 +188,10 @@ dl_open_worker (void *a)
 	_dl_signal_error (0, "dlopen",
 			  N_("DST not allowed in SUID/SGID programs"));
 
-      /* We have to find out from which object the caller is calling.
-	 Find the highest-addressed object that ADDRESS is not below.  */
+      /* We have to find out from which object the caller is calling.  */
       call_map = NULL;
       for (l = _dl_loaded; l; l = l->l_next)
-	if (l->l_addr != 0 /* Make sure we do not currently set this map up
-			      in this moment.  */
-	    && caller >= (const void *) l->l_map_start
+	if (caller >= (const void *) l->l_map_start
 	    && caller < (const void *) l->l_map_end)
 	  {
 	    /* There must be exactly one DSO for the range of the virtual
--- libc/elf/dl-error.c.jj	Mon May 21 11:43:57 2001
+++ libc/elf/dl-error.c	Mon May 21 11:44:06 2001
@@ -71,6 +71,8 @@ _dl_signal_error (int errcode, const cha
     errstring = N_("DYNAMIC LINKER BUG!!!");
 
   lcatch = tsd_getspecific ();
+  if (objname == NULL)
+    objname = "";
   if (lcatch != NULL)
     {
       /* We are inside _dl_catch_error.  Return to it.  We have to
@@ -100,7 +102,7 @@ _dl_signal_error (int errcode, const cha
       _dl_fatal_printf ("\
 %s: error while loading shared libraries: %s%s%s%s%s\n",
 			_dl_argv[0] ?: "<program name unknown>",
-			objname ?: "", objname && *objname ? ": " : "",
+			objname, *objname ? ": " : "",
 			errstring, errcode ? ": " : "",
 			(errcode
 			 ? __strerror_r (errcode, buffer, sizeof buffer)
--- libc/elf/restest2.c.jj	Mon May 21 11:44:06 2001
+++ libc/elf/restest2.c	Mon May 21 11:44:06 2001
@@ -0,0 +1,33 @@
+#include <sys/types.h>
+#include <dlfcn.h>
+#include <error.h>
+#include <mcheck.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+pid_t pid, pid2;
+
+pid_t getpid(void)
+{
+  pid_t (*f)(void);
+  f = (pid_t (*)(void)) dlsym (RTLD_NEXT, "getpid");
+  if (f == NULL)
+    error (EXIT_FAILURE, 0, "dlsym (RTLD_NEXT, \"getpid\"): %s", dlerror ());
+  return (pid2 = f()) + 26;
+}
+
+int
+main (void)
+{
+  pid_t (*f)(void);
+
+  mtrace ();
+
+  f = (pid_t (*)(void)) dlsym (RTLD_DEFAULT, "getpid");
+  if (f == NULL)
+    error (EXIT_FAILURE, 0, "dlsym (RTLD_DEFAULT, \"getpid\"): %s", dlerror ());
+  pid = f();
+  if (pid != pid2 + 26)
+    error (EXIT_FAILURE, 0, "main getpid() not called");
+  return 0;
+}
--- libc/elf/Makefile.jj	Mon May 21 11:43:57 2001
+++ libc/elf/Makefile	Mon May 21 11:44:06 2001
@@ -101,7 +101,8 @@ tests = loadtest restest1 preloadtest lo
 	constload1 order $(tests-vis-$(have-protected)) noload filter unload \
 	reldep reldep2 reldep3 next $(tests-nodelete-$(have-z-nodelete)) \
 	$(tests-nodlopen-$(have-z-nodlopen)) neededtest neededtest2 \
-	neededtest3 neededtest4 unload2 lateglobal initfirst global
+	neededtest3 neededtest4 unload2 lateglobal initfirst global \
+	restest2
 test-srcs = tst-pathopt
 tests-vis-yes = vismain
 tests-nodelete-yes = nodelete
@@ -302,6 +303,9 @@ $(objpfx)neededtest4.out: $(objpfx)neede
 
 $(objpfx)restest1: $(objpfx)testobj1.so $(objpfx)testobj1_1.so $(libdl)
 LDFLAGS-restest1 = -rdynamic
+
+$(objpfx)restest2: $(libdl)
+LDFLAGS-restest2 = -rdynamic
 
 $(objpfx)restest1.out: $(test-modules)
 


	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]