[PATCH] Fix other places which relied on contiguous ownership of l_map_start..l_map_end region by one object

[PATCH] Fix other places which relied on contiguous ownership of l_map_start..l_map_end region by one object

[Jakub Jelinek]

Hi!

dladdr wasn't the only place which misbehaved if some old DSO with smaller
-z maxpagesize got mapped inside of ld.so's inter-segment hole.
As the patch below shows, dlsym/dlvsym/dlopen/dl_iterate_phdr also
relied on virtual memory range exclusivity.
As all ld.so mapped ET_DYN libraries have such exclusivity (they PROT_NONE
protect inter-segment holes), this patch optimizes that case and only
does the PT_LOAD segment walking for the executable (if it has holes)
or for objects mapped by the kernel, where we can't rely on anything
(ld.so itself, vDSO).  Not sure if _dl_addr_inside_object should be inlined
or if it is ok as is in the patch (and even when not inlined, the question
is whether to duplicate it as in the patch (fortunately te routine isn't
too large), move to a file on its own that will be linked into both
ld.so and libc.so or put it into ld.so/libc.a only and let libc.so call
it through _rtld_global.some_fn_ptr).

2007-06-18  Jakub Jelinek  <jakub@redhat.com>

	* elf/dl-addr.c (_dl_addr): Skip PT_LOAD checking if l_contiguous.
	Move PT_LOAD checking to...
	(_dl_addr_inside_object): ... here, new function.
	* elf/dl-sym.c (do_sym): If not l_contiguous, call _dl_addr_inside_object.
	* elf/dl-iteratephdr.c (__dl_iterate_phdr): Likewise.
	* dlfcn/dlinfo.c (dlinfo_doit): Likewise.
	* elf/dl-open.c (dl_open_worker): Likewise.
	(_dl_addr_inside_object): New function if IS_IN_rtld.
	* elf/dl-load.c (_dl_map_object_from_fd): Set l_contiguous if no
	holes are present or are PROT_NONE protected.
	* include/link.h (struct link_map): Add l_contiguous field.
	* sysdeps/generic/ldsodefs.h (_dl_addr_inside_object): New
	prototype.

--- libc/elf/dl-sym.c.jj	2007-05-21 23:33:49.000000000 +0200
+++ libc/elf/dl-sym.c	2007-06-18 15:26:54.000000000 +0200
@@ -98,10 +98,9 @@ do_sym (void *handle, const char *name, 
   for (Lmid_t ns = 0; ns < DL_NNS; ++ns)
     for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l != NULL;
 	 l = l->l_next)
-      if (caller >= l->l_map_start && caller < l->l_map_end)
+      if (caller >= l->l_map_start && caller < l->l_map_end
+	  && (l->l_contiguous || _dl_addr_inside_object (l, caller)))
 	{
-	  /* There must be exactly one DSO for the range of the virtual
-	     memory.  Otherwise something is really broken.  */
 	  match = l;
 	  break;
 	}
--- libc/elf/dl-addr.c.jj	2007-05-09 13:26:10.000000000 +0200
+++ libc/elf/dl-addr.c	2007-06-18 15:54:51.000000000 +0200
@@ -134,22 +134,12 @@ _dl_addr (const void *address, Dl_info *
   /* Find the highest-addressed object that ADDRESS is not below.  */
   for (Lmid_t ns = 0; ns < DL_NNS; ++ns)
     for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l; l = l->l_next)
-      if (addr >= l->l_map_start && addr < l->l_map_end)
+      if (addr >= l->l_map_start && addr < l->l_map_end
+	  && (l->l_contiguous || _dl_addr_inside_object (l, addr)))
 	{
-	  /* Make sure it lies within one of L's segments.  */
-	  int n = l->l_phnum;
-	  const ElfW(Addr) reladdr = addr - l->l_addr;
-	  while (--n >= 0)
-	    if (l->l_phdr[n].p_type == PT_LOAD)
-	      {
-		if (reladdr - l->l_phdr[n].p_vaddr >= 0
-		    && reladdr - l->l_phdr[n].p_vaddr < l->l_phdr[n].p_memsz)
-		  {
-		    determine_info (addr, l, info, mapp, symbolp);
-		    result = 1;
-		    goto out;
-		  }
-	      }
+	  determine_info (addr, l, info, mapp, symbolp);
+	  result = 1;
+	  goto out;
 	}
 
  out:
@@ -158,3 +148,19 @@ _dl_addr (const void *address, Dl_info *
   return result;
 }
 libc_hidden_def (_dl_addr)
+
+/* Return non-zero if ADDR lies within one of L's segments.  */
+int
+internal_function
+_dl_addr_inside_object (struct link_map *l, const ElfW(Addr) addr)
+{
+  int n = l->l_phnum;
+  const ElfW(Addr) reladdr = addr - l->l_addr;
+
+  while (--n >= 0)
+    if (l->l_phdr[n].p_type == PT_LOAD
+	&& reladdr - l->l_phdr[n].p_vaddr >= 0
+	&& reladdr - l->l_phdr[n].p_vaddr < l->l_phdr[n].p_memsz)
+      return 1;
+  return 0;
+}
--- libc/elf/dl-iteratephdr.c.jj	2006-10-31 23:05:29.000000000 +0100
+++ libc/elf/dl-iteratephdr.c	2007-06-18 15:44:19.000000000 +0200
@@ -1,5 +1,5 @@
 /* Get loaded objects program headers.
-   Copyright (C) 2001,2002,2003,2004,2006 Free Software Foundation, Inc.
+   Copyright (C) 2001,2002,2003,2004,2006,2007 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Jakub Jelinek <jakub@redhat.com>, 2001.
 
@@ -54,9 +54,9 @@ __dl_iterate_phdr (int (*callback) (stru
 	nloaded += GL(dl_ns)[cnt]._ns_nloaded;
 
 	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
-	     memory.  Otherwise something is really broken.  */
+	    && caller < (const void *) l->l_map_end
+	    && (l->l_contiguous
+		|| _dl_addr_inside_object (l, (ElfW(Addr)) caller)))
 	  ns = cnt;
       }
 
--- libc/elf/dl-open.c.jj	2007-05-21 23:33:49.000000000 +0200
+++ libc/elf/dl-open.c	2007-06-18 16:00:55.000000000 +0200
@@ -201,10 +201,10 @@ dl_open_worker (void *a)
       for (Lmid_t ns = 0; ns < DL_NNS; ++ns)
 	for (l = GL(dl_ns)[ns]._ns_loaded; l != NULL; l = l->l_next)
 	  if (caller_dlopen >= (const void *) l->l_map_start
-	      && caller_dlopen < (const void *) l->l_map_end)
+	      && caller_dlopen < (const void *) l->l_map_end
+	      && (l->l_contiguous
+		  || _dl_addr_inside_object (l, (ElfW(Addr)) caller_dlopen)))
 	    {
-	      /* There must be exactly one DSO for the range of the virtual
-		 memory.  Otherwise something is really broken.  */
 	      assert (ns == l->l_ns);
 	      call_map = l;
 	      goto found_caller;
@@ -662,3 +662,21 @@ show_scope (struct link_map *new)
     }
 }
 #endif
+
+#ifdef IS_IN_rtld
+/* Return non-zero if ADDR lies within one of L's segments.  */
+int
+internal_function
+_dl_addr_inside_object (struct link_map *l, const ElfW(Addr) addr)
+{
+  int n = l->l_phnum;
+  const ElfW(Addr) reladdr = addr - l->l_addr;
+
+  while (--n >= 0)
+    if (l->l_phdr[n].p_type == PT_LOAD
+	&& reladdr - l->l_phdr[n].p_vaddr >= 0
+	&& reladdr - l->l_phdr[n].p_vaddr < l->l_phdr[n].p_memsz)
+      return 1;
+  return 0;
+}
+#endif
--- libc/elf/dl-load.c.jj	2006-11-10 09:01:41.000000000 +0100
+++ libc/elf/dl-load.c	2007-06-18 15:32:01.000000000 +0200
@@ -1,5 +1,5 @@
 /* Map in a shared object's segments from the file.
-   Copyright (C) 1995-2005, 2006  Free Software Foundation, Inc.
+   Copyright (C) 1995-2005, 2006, 2007  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
@@ -1223,6 +1223,8 @@ cannot allocate TLS data structures for 
 		      loadcmds[nloadcmds - 1].mapstart - c->mapend,
 		      PROT_NONE);
 
+	l->l_contiguous = 1;
+
 	goto postmap;
       }
 
@@ -1242,6 +1244,7 @@ cannot allocate TLS data structures for 
     /* Remember which part of the address space this object uses.  */
     l->l_map_start = c->mapstart + l->l_addr;
     l->l_map_end = l->l_map_start + maplength;
+    l->l_contiguous = !has_holes;
 
     while (c < &loadcmds[nloadcmds])
       {
--- libc/include/link.h.jj	2007-05-21 23:33:49.000000000 +0200
+++ libc/include/link.h	2007-06-18 15:26:20.000000000 +0200
@@ -187,6 +187,9 @@ struct link_map
 				       is interested in the PLT interception.*/
     unsigned int l_removed:1;	/* Nozero if the object cannot be used anymore
 				   since it is removed.  */
+    unsigned int l_contiguous:1; /* Nonzero if inter-segment holes are
+				    mprotected or if no holes are present at
+				    all.  */
 
     /* Collected information about own RPATH directories.  */
     struct r_search_path_struct l_rpath_dirs;
--- libc/dlfcn/dlinfo.c.jj	2006-10-31 23:05:28.000000000 +0100
+++ libc/dlfcn/dlinfo.c	2007-06-18 15:28:36.000000000 +0200
@@ -1,5 +1,5 @@
 /* dlinfo -- Get information from the dynamic linker.
-   Copyright (C) 2003, 2004, 2006 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2006, 2007 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
@@ -56,9 +56,8 @@ dlinfo_doit (void *argsblock)
       /* Find the highest-addressed object that CALLER is not below.  */
       for (nsid = 0; nsid < DL_NNS; ++nsid)
 	for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)
-	  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.  */
+	  if (caller >= l->l_map_start && caller < l->l_map_end
+	      && (l->l_contiguous || _dl_addr_inside_object (l, caller)))
 	    break;
 
       if (l == NULL)
--- libc/sysdeps/generic/ldsodefs.h.jj	2007-05-21 23:33:53.000000000 +0200
+++ libc/sysdeps/generic/ldsodefs.h	2007-06-18 15:41:58.000000000 +0200
@@ -1061,6 +1061,8 @@ extern struct link_map *_dl_update_sloti
    but never touch anything.  Return null if it's not allocated yet.  */
 extern void *_dl_tls_get_addr_soft (struct link_map *l) internal_function;
 
+extern int _dl_addr_inside_object (struct link_map *l, const ElfW(Addr) addr)
+     internal_function attribute_hidden;
 
 __END_DECLS
 

	Jakub

Re: [PATCH] Fix other places which relied on contiguous ownership of l_map_start..l_map_end region by one object

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I think the patch is fine as is. No need to use pointers to avoid this
minimal duplication.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQFGeGA02ijCOnn/RHQRAuFVAKCspwfzMITp2ggV5kksYCJwjC3KQwCdF7on
b3jtW14K/FQ/IoApbhoIdL8=
=1mqD
-----END PGP SIGNATURE-----