public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Ulrich Drepper <drepper@redhat.com>
Cc: Glibc hackers <libc-hacker@sources.redhat.com>
Subject: [PATCH] Fix other places which relied on contiguous ownership of l_map_start..l_map_end region by one object
Date: Mon, 18 Jun 2007 15:06:00 -0000	[thread overview]
Message-ID: <20070618151007.GY3081@sunsite.mff.cuni.cz> (raw)

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

             reply	other threads:[~2007-06-18 15:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-18 15:06 Jakub Jelinek [this message]
2007-06-19 23:01 ` Ulrich Drepper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070618151007.GY3081@sunsite.mff.cuni.cz \
    --to=jakub@redhat.com \
    --cc=drepper@redhat.com \
    --cc=libc-hacker@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).