diff mbox series

[v2,11/32] elf: Merge the three implementations of _dl_dst_substitute

Message ID a62727f2c2317f89688eb2b809a29110de81eb98.1688741159.git.fweimer@redhat.com
State New
Headers show
Series RELRO link maps | expand

Commit Message

Florian Weimer July 7, 2023, 6:48 p.m. UTC
Use one implementation to perform the copying and the counting.

The IS_RTLD check for l_origin processing is eliminated because
$ORIGIN cannot used with the ld.so link map (and not with the
vDSO link map either) because there is no user code associated
with it that might call dlopen with an $ORIGIN path.
---
 elf/dl-deps.c                       |  70 ++++++-------
 elf/dl-load.c                       | 146 ++++++++++++----------------
 elf/dl-open.c                       |   1 -
 sysdeps/generic/ldsodefs.h          |   9 +-
 sysdeps/unix/sysv/linux/dl-origin.c |   1 -
 5 files changed, 94 insertions(+), 133 deletions(-)
diff mbox series

Patch

diff --git a/elf/dl-deps.c b/elf/dl-deps.c
index 0549b4a4ff..110c8953bd 100644
--- a/elf/dl-deps.c
+++ b/elf/dl-deps.c
@@ -28,8 +28,7 @@ 
 #include <sys/param.h>
 #include <ldsodefs.h>
 #include <scratch_buffer.h>
-
-#include <dl-dst.h>
+#include <alloc_buffer.h>
 
 /* Whether an shared object references one or more auxiliary objects
    is signaled by the AUXTAG entry in l_info.  */
@@ -80,47 +79,34 @@  struct list
   };
 
 
-/* Macro to expand DST.  It is an macro since we use `alloca'.  */
+/* Macro to expand DST.  It is an macro since we use `alloca'.
+   See expand_dynamic_string_token in dl-load.c.  */
 #define expand_dst(l, str, fatal) \
-  ({									      \
-    const char *__str = (str);						      \
-    const char *__result = __str;					      \
-    size_t __dst_cnt = _dl_dst_count (__str);				      \
-									      \
-    if (__dst_cnt != 0)							      \
-      {									      \
-	char *__newp;							      \
-									      \
-	/* DST must not appear in SUID/SGID programs.  */		      \
-	if (__libc_enable_secure)					      \
-	  _dl_signal_error (0, __str, NULL, N_("\
-DST not allowed in SUID/SGID programs"));				      \
-									      \
-	__newp = (char *) alloca (DL_DST_REQUIRED (l, __str, strlen (__str),  \
-						   __dst_cnt));		      \
-									      \
-	__result = _dl_dst_substitute (l, __str, __newp);		      \
-									      \
-	if (*__result == '\0')						      \
-	  {								      \
-	    /* The replacement for the DST is not known.  We can't	      \
-	       processed.  */						      \
-	    if (fatal)							      \
-	      _dl_signal_error (0, __str, NULL, N_("\
-empty dynamic string token substitution"));				      \
-	    else							      \
-	      {								      \
-		/* This is for DT_AUXILIARY.  */			      \
-		if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))   \
-		  _dl_debug_printf (N_("\
-cannot load auxiliary `%s' because of empty dynamic string token "	      \
-					    "substitution\n"), __str);	      \
-		continue;						      \
-	      }								      \
-	  }								      \
-      }									      \
-									      \
-    __result; })
+  ({									\
+    struct alloc_buffer __buf = {};					\
+    size_t __size = _dl_dst_substitute ((l), (str), &__buf);		\
+    char *__result = alloca (__size);					\
+    __buf = alloc_buffer_create (__result, __size);			\
+    if (_dl_dst_substitute ((l), (str), &__buf) == 0)			\
+      {									\
+	/* The replacement for the DST is not known.  We can't		\
+	   processed.  */						\
+	if (fatal)							\
+	  _dl_signal_error (0, str, NULL, N_("\
+empty dynamic string token substitution"));				\
+	else								\
+	  {								\
+	    /* This is for DT_AUXILIARY.  */				\
+	    if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))	\
+	      _dl_debug_printf (N_("\
+cannot load auxiliary `%s' because of empty dynamic string token "	\
+				   "substitution\n"), str);		\
+	    continue;							\
+	  }								\
+      }									\
+    assert (!alloc_buffer_has_failed (&__buf));				\
+    __result;								\
+  })									\
 
 static void
 preload (struct list *known, unsigned int *nlist, struct link_map *map)
diff --git a/elf/dl-load.c b/elf/dl-load.c
index fda0fe8000..f46af28944 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -32,6 +32,7 @@ 
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <gnu/lib-names.h>
+#include <alloc_buffer.h>
 
 /* Type for the buffer we put the ELF header and hopefully the program
    header.  This buffer does not really have to be too large.  In most
@@ -67,7 +68,6 @@  struct filebuf
 #include <libc-pointer-arith.h>
 #include <array_length.h>
 
-#include <dl-dst.h>
 #include <dl-load.h>
 #include <dl-map-segments.h>
 #include <dl-unmap-segments.h>
@@ -227,61 +227,32 @@  is_dst (const char *input, const char *ref)
     return rlen;
 }
 
-/* INPUT should be the start of a path e.g DT_RPATH or name e.g.
-   DT_NEEDED.  The return value is the number of known DSTs found.  We
-   count all known DSTs regardless of __libc_enable_secure; the caller
-   is responsible for enforcing the security of the substitution rules
-   (usually _dl_dst_substitute).  */
-size_t
-_dl_dst_count (const char *input)
-{
-  size_t cnt = 0;
-
-  input = strchr (input, '$');
-
-  /* Most likely there is no DST.  */
-  if (__glibc_likely (input == NULL))
-    return 0;
-
-  do
-    {
-      size_t len;
-
-      ++input;
-      /* All DSTs must follow ELF gABI rules, see is_dst ().  */
-      if ((len = is_dst (input, "ORIGIN")) != 0
-	  || (len = is_dst (input, "PLATFORM")) != 0
-	  || (len = is_dst (input, "LIB")) != 0)
-	++cnt;
-
-      /* There may be more than one DST in the input.  */
-      input = strchr (input + len, '$');
-    }
-  while (input != NULL);
-
-  return cnt;
-}
-
-/* Process INPUT for DSTs and store in RESULT using the information
+/* Process INPUT for DSTs and store in *RESULT using the information
    from link map L to resolve the DSTs. This function only handles one
    path at a time and does not handle colon-separated path lists (see
-   fillin_rpath ()).  Lastly the size of result in bytes should be at
-   least equal to the value returned by DL_DST_REQUIRED.  Note that it
-   is possible for a DT_NEEDED, DT_AUXILIARY, and DT_FILTER entries to
-   have colons, but we treat those as literal colons here, not as path
-   list delimiters.  */
-char *
-_dl_dst_substitute (struct link_map *l, const char *input, char *result)
+   fillin_rpath ()).
+
+   A caller is expected to call this function twice, first with an
+   empty *RESULT buffer to obtain the total length (including the
+   terminating null byte) that is returned by this function.  The
+   second call should be made with a properly sized buffer, and this
+   function will write the expansion to *RESULT.  If that second call
+   returns 0, it means that the expansion is not valid and should be
+   ignored.
+
+   Note that it is possible for a DT_NEEDED,
+   DT_AUXILIARY, and DT_FILTER entries to have colons, but we treat
+   those as literal colons here, not as path list delimiters.  */
+size_t
+_dl_dst_substitute (struct link_map *l, const char *input,
+		    struct alloc_buffer *result)
 {
   /* Copy character-by-character from input into the working pointer
-     looking for any DSTs.  We track the start of input and if we are
-     going to check for trusted paths, all of which are part of $ORIGIN
-     handling in SUID/SGID cases (see below).  In some cases, like when
-     a DST cannot be replaced, we may set result to an empty string and
-     return.  */
-  char *wp = result;
+     looking for any DSTs.  */
   const char *start = input;
+  char *result_start = alloc_buffer_next (result, char);
   bool check_for_trusted = false;
+  size_t length = 0;
 
   do
     {
@@ -318,7 +289,16 @@  _dl_dst_substitute (struct link_map *l, const char *input, char *result)
 		       && (input[len] == '\0' || input[len] == '/')))
 		repl = (const char *) -1;
 	      else
-	        repl = l->l_origin;
+		{
+		  if (l->l_origin == NULL)
+		    {
+		      /* For loaded DSOs, the l_origin field is set in
+			 _dl_new_object.  */
+		      assert (l->l_name[0] == '\0');
+		      l->l_origin = _dl_get_origin ();
+		    }
+		  repl = l->l_origin;
+		}
 
 	      check_for_trusted = (__libc_enable_secure
 				   && l->l_type == lt_executable);
@@ -330,7 +310,9 @@  _dl_dst_substitute (struct link_map *l, const char *input, char *result)
 
 	  if (repl != NULL && repl != (const char *) -1)
 	    {
-	      wp = __stpcpy (wp, repl);
+	      size_t repl_len = strlen (repl);
+	      length += repl_len;
+	      alloc_buffer_copy_bytes (result, repl, repl_len);
 	      input += len;
 	    }
 	  else if (len != 0)
@@ -338,16 +320,20 @@  _dl_dst_substitute (struct link_map *l, const char *input, char *result)
 	      /* We found a valid DST that we know about, but we could
 	         not find a replacement value for it, therefore we
 		 cannot use this path and discard it.  */
-	      *result = '\0';
-	      return result;
+	      alloc_buffer_mark_failed (result);
+	      return 0;
 	    }
 	  else
-	    /* No DST we recognize.  */
-	    *wp++ = '$';
+	    {
+	      /* No DST we recognize.  */
+	      ++length;
+	      alloc_buffer_add_byte (result, '$');
+	    }
 	}
       else
 	{
-	  *wp++ = *input++;
+	  ++length;
+	  alloc_buffer_add_byte (result, *input++);
 	}
     }
   while (*input != '\0');
@@ -362,15 +348,19 @@  _dl_dst_substitute (struct link_map *l, const char *input, char *result)
      this way because it may be manipulated in some ways with hard
      links.  */
   if (__glibc_unlikely (check_for_trusted)
-      && !is_trusted_path_normalize (result, wp - result))
+      && !alloc_buffer_has_failed (result)
+      && !is_trusted_path_normalize (result_start,
+				     alloc_buffer_next (result, char)
+				     - result_start))
     {
-      *result = '\0';
-      return result;
+      alloc_buffer_mark_failed (result);
+      return 0;
     }
 
-  *wp = '\0';
+  ++length;
+  alloc_buffer_add_byte (result, 0);
 
-  return result;
+  return length;
 }
 
 
@@ -382,30 +372,18 @@  _dl_dst_substitute (struct link_map *l, const char *input, char *result)
 static char *
 expand_dynamic_string_token (struct link_map *l, const char *input)
 {
-  /* We make two runs over the string.  First we determine how large the
-     resulting string is and then we copy it over.  Since this is no
-     frequently executed operation we are looking here not for performance
-     but rather for code size.  */
-  size_t cnt;
-  size_t total;
-  char *result;
-
-  /* Determine the number of DSTs.  */
-  cnt = _dl_dst_count (input);
-
-  /* If we do not have to replace anything simply copy the string.  */
-  if (__glibc_likely (cnt == 0))
-    return __strdup (input);
-
-  /* Determine the length of the substituted string.  */
-  total = DL_DST_REQUIRED (l, input, strlen (input), cnt);
-
-  /* Allocate the necessary memory.  */
-  result = (char *) malloc (total + 1);
+  struct alloc_buffer buf = {};
+  size_t size = _dl_dst_substitute (l, input, &buf);
+  char *result = malloc (size);
   if (result == NULL)
     return NULL;
-
-  return _dl_dst_substitute (l, input, result);
+  buf = alloc_buffer_create (result, size);
+  if (_dl_dst_substitute (l, input, &buf) == 0)
+    /* Mark the expanded string as to be ignored.  */
+    *result = '\0';
+  else
+    assert (!alloc_buffer_has_failed (&buf));
+  return result;
 }
 
 
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 2d985e21d8..d88e6b5f6b 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -38,7 +38,6 @@ 
 #include <gnu/lib-names.h>
 #include <dl-find_object.h>
 
-#include <dl-dst.h>
 #include <dl-prop.h>
 
 
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 5941da3ec1..3698e34bc0 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1224,12 +1224,11 @@  extern void _dl_nothread_init_static_tls (struct link_map *) attribute_hidden;
 /* Find origin of the executable.  */
 extern const char *_dl_get_origin (void) attribute_hidden;
 
-/* Count DSTs.  */
-extern size_t _dl_dst_count (const char *name) attribute_hidden;
-
 /* Substitute DST values.  */
-extern char *_dl_dst_substitute (struct link_map *l, const char *name,
-				 char *result) attribute_hidden;
+struct alloc_buffer;
+size_t _dl_dst_substitute (struct link_map *l, const char *name,
+			   struct alloc_buffer *result)
+     attribute_hidden __nonnull ((1, 2, 3));
 
 /* Open the shared object NAME, relocate it, and run its initializer if it
    hasn't already been run.  MODE is as for `dlopen' (see <dlfcn.h>).  If
diff --git a/sysdeps/unix/sysv/linux/dl-origin.c b/sysdeps/unix/sysv/linux/dl-origin.c
index d87e89335d..82298c28f4 100644
--- a/sysdeps/unix/sysv/linux/dl-origin.c
+++ b/sysdeps/unix/sysv/linux/dl-origin.c
@@ -17,7 +17,6 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <assert.h>
-#include <dl-dst.h>
 #include <fcntl.h>
 #include <ldsodefs.h>
 #include <sysdep.h>