[roland/dl-fileid] Factor file identity rules out of generic rtld code.
diff mbox

Message ID 20150714204702.117892C39F1@topped-with-meat.com
State New
Headers show

Commit Message

Roland McGrath July 14, 2015, 8:47 p.m. UTC
I first posted this back in February.  I have gotten no objections.
I think this patch is entirely unchanged by the fresh rebasing.
I'd always intended to get this in before the 2.22 cut.

OK now?


Thanks,
Roland


2015-07-14  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/generic/dl-fileid.h: New file.
	* sysdeps/posix/dl-fileid.h: New file.
	* sysdeps/nacl/dl-fileid.h: New file.
	* include/link.h: Include <dl-fileid.h>.
	(struct link_map): Replace l_dev and l_ino with l_file_id.
	* elf/dl-load.c (_dl_map_object_from_fd): Use _dl_get_file_id rather
	than __fxstat64.  Use _dl_file_id_match_p rather than comparing l_dev
	and l_ino directly.  Initialize l_file_id rather than l_dev and l_ino.

Comments

Carlos O'Donell July 14, 2015, 8:53 p.m. UTC | #1
On 07/14/2015 04:47 PM, Roland McGrath wrote:
> I first posted this back in February.  I have gotten no objections.
> I think this patch is entirely unchanged by the fresh rebasing.
> I'd always intended to get this in before the 2.22 cut.
> 
> OK now?
> 
> 
> Thanks,
> Roland
> 
> 
> 2015-07-14  Roland McGrath  <roland@hack.frob.com>
> 
> 	* sysdeps/generic/dl-fileid.h: New file.
> 	* sysdeps/posix/dl-fileid.h: New file.
> 	* sysdeps/nacl/dl-fileid.h: New file.
> 	* include/link.h: Include <dl-fileid.h>.
> 	(struct link_map): Replace l_dev and l_ino with l_file_id.
> 	* elf/dl-load.c (_dl_map_object_from_fd): Use _dl_get_file_id rather
> 	than __fxstat64.  Use _dl_file_id_match_p rather than comparing l_dev
> 	and l_ino directly.  Initialize l_file_id rather than l_dev and l_ino.

OK.

c.
Roland McGrath July 14, 2015, 10:01 p.m. UTC | #2
Committed.


Thanks,
Roland

Patch
diff mbox

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 41b91fc..0c052e4 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -872,7 +872,6 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
   const ElfW(Phdr) *ph;
   size_t maplength;
   int type;
-  struct stat64 st;
   /* Initialize to keep the compiler happy.  */
   const char *errstring = NULL;
   int errval = 0;
@@ -880,7 +879,8 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
   bool make_consistent = false;
 
   /* Get file information.  */
-  if (__glibc_unlikely (__fxstat64 (_STAT_VER, fd, &st) < 0))
+  struct r_file_id id;
+  if (__glibc_unlikely (!_dl_get_file_id (fd, &id)))
     {
       errstring = N_("cannot stat shared object");
     call_lose_errno:
@@ -891,8 +891,8 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
     }
 
   /* Look again to see if the real name matched another already loaded.  */
-  for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
-    if (l->l_removed == 0 && l->l_ino == st.st_ino && l->l_dev == st.st_dev)
+  for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)
+    if (!l->l_removed && _dl_file_id_match_p (&l->l_file_id, &id))
       {
 	/* The object is already loaded.
 	   Just bump its reference count and return it.  */
@@ -910,8 +910,7 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
   /* When loading into a namespace other than the base one we must
      avoid loading ld.so since there can only be one copy.  Ever.  */
   if (__glibc_unlikely (nsid != LM_ID_BASE)
-      && ((st.st_ino == GL(dl_rtld_map).l_ino
-	   && st.st_dev == GL(dl_rtld_map).l_dev)
+      && (_dl_file_id_match_p (&id, &GL(dl_rtld_map).l_file_id)
 	  || _dl_name_match_p (name, &GL(dl_rtld_map))))
     {
       /* This is indeed ld.so.  Create a new link_map which refers to
@@ -1220,7 +1219,7 @@  cannot allocate TLS data structures for initial thread");
        l_map_start, l_map_end, l_addr, l_contiguous, l_text_end, l_phdr
      */
     errstring = _dl_map_segments (l, fd, header, type, loadcmds, nloadcmds,
-                                  maplength, has_holes, loader);
+				  maplength, has_holes, loader);
     if (__glibc_unlikely (errstring != NULL))
       goto call_lose;
   }
@@ -1390,8 +1389,7 @@  cannot enable executable stack as shared object requires");
     GL(dl_initfirst) = l;
 
   /* Finally the file information.  */
-  l->l_dev = st.st_dev;
-  l->l_ino = st.st_ino;
+  l->l_file_id = id;
 
   /* When we profile the SONAME might be needed for something else but
      loading.  Add it right away.  */
@@ -1890,7 +1888,7 @@  open_path (const char *name, size_t namelen, int mode,
 	free (sps->dirs);
 
       /* rtld_search_dirs and env_path_list are attribute_relro, therefore
-         avoid writing into it.  */
+	 avoid writing into it.  */
       if (sps != &rtld_search_dirs && sps != &env_path_list)
 	sps->dirs = (void *) -1;
     }
diff --git a/include/link.h b/include/link.h
index 7ccd9c3..423a695 100644
--- a/include/link.h
+++ b/include/link.h
@@ -40,6 +40,7 @@  extern unsigned int la_objopen (struct link_map *__map, Lmid_t __lmid,
 #include <stdint.h>
 #include <stddef.h>
 #include <bits/linkmap.h>
+#include <dl-fileid.h>
 #include <dl-lookupcfg.h>
 #include <tls.h>
 #include <bits/libc-lock.h>
@@ -235,8 +236,7 @@  struct link_map
 
     /* This information is kept to check for sure whether a shared
        object is the same as one already loaded.  */
-    dev_t l_dev;
-    ino64_t l_ino;
+    struct r_file_id l_file_id;
 
     /* Collected information about own RUNPATH directories.  */
     struct r_search_path_struct l_runpath_dirs;
diff --git a/sysdeps/generic/dl-fileid.h b/sysdeps/generic/dl-fileid.h
new file mode 100644
index 0000000..2cbd21d
--- /dev/null
+++ b/sysdeps/generic/dl-fileid.h
@@ -0,0 +1,46 @@ 
+/* File identity for the dynamic linker.  Stub version.
+   Copyright (C) 2015 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
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdbool.h>
+
+/* This type stores whatever information is fetched by _dl_get_file_id
+   and compared by _dl_file_id_match_p.  */
+struct r_file_id
+  {
+    /* In the stub version, we don't record anything at all.  */
+  };
+
+/* Sample FD to fill in *ID.  Returns true on success.
+   On error, returns false, with errno set.  */
+static inline bool
+_dl_get_file_id (int fd __attribute__ ((unused)),
+		 struct r_file_id *id __attribute__ ((unused)))
+{
+  return true;
+}
+
+/* Compare two results from _dl_get_file_id for equality.
+   It's crucial that this never return false-positive matches.
+   It's ideal that it never return false-negative mismatches either,
+   but lack of matches is survivable.  */
+static inline bool
+_dl_file_id_match_p (const struct r_file_id *a __attribute__ ((unused)),
+		     const struct r_file_id *b __attribute__ ((unused)))
+{
+  return false;
+}
diff --git a/sysdeps/nacl/dl-fileid.h b/sysdeps/nacl/dl-fileid.h
new file mode 100644
index 0000000..4c34581
--- /dev/null
+++ b/sysdeps/nacl/dl-fileid.h
@@ -0,0 +1,8 @@ 
+/* Bypass sysdeps/posix/dl-fileid.h, which relies on st_dev/st_ino being
+   reliable.  Under NaCl, we cannot always expect them to be useful.
+   Fortunately, in the ways NaCl is used it's far less likely that two
+   different names for the same file would be used in dlopen or the like,
+   so failing to notice re-opening the same file is not so likely to be a
+   problem in practice.  */
+
+#include <sysdeps/generic/dl-fileid.h>
diff --git a/sysdeps/posix/dl-fileid.h b/sysdeps/posix/dl-fileid.h
new file mode 100644
index 0000000..d0d5436
--- /dev/null
+++ b/sysdeps/posix/dl-fileid.h
@@ -0,0 +1,50 @@ 
+/* File identity for the dynamic linker.  Generic POSIX.1 version.
+   Copyright (C) 2015 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
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdbool.h>
+#include <sys/stat.h>
+
+/* For POSIX.1 systems, the pair of st_dev and st_ino constitute
+   a unique identifier for a file.  */
+struct r_file_id
+  {
+    dev_t dev;
+    ino64_t ino;
+  };
+
+/* Sample FD to fill in *ID.  Returns true on success.
+   On error, returns false, with errno set.  */
+static inline bool
+_dl_get_file_id (int fd, struct r_file_id *id)
+{
+  struct stat64 st;
+
+  if (__glibc_unlikely (__fxstat64 (_STAT_VER, fd, &st) < 0))
+    return false;
+
+  id->dev = st.st_dev;
+  id->ino = st.st_ino;
+  return true;
+}
+
+/* Compare two results from _dl_get_file_id for equality.  */
+static inline bool
+_dl_file_id_match_p (const struct r_file_id *a, const struct r_file_id *b)
+{
+  return a->dev == b->dev && a->ino == b->ino;
+}