[v4,07/14] elf: Merge the three implementations of _dl_dst_substitute

Message ID accd109f7264467148600d3e793fa40989d8ca90.1738530302.git.fweimer@redhat.com (mailing list archive)
State New
Headers
Series RELRO link maps |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Florian Weimer Feb. 2, 2025, 9:13 p.m. UTC
  Use one implementation to perform the copying and the counting.
Use l_origin of the main program as a fallback for objects
that have not been dlopen'ed (such as ld.so itself, or the vDSO).
These should not end up as dlopen callers normally, but they
might one day if a dlopen variant with an explicit caller
argument is supported for application use.
---
 elf/dl-deps.c                       |  70 ++++++--------
 elf/dl-dst.h                        |  56 -----------
 elf/dl-load.c                       | 145 ++++++++++++----------------
 elf/dl-open.c                       |   1 -
 elf/dl-origin.c                     |   3 -
 sysdeps/generic/ldsodefs.h          |   9 +-
 sysdeps/unix/sysv/linux/dl-origin.c |   1 -
 7 files changed, 93 insertions(+), 192 deletions(-)
 delete mode 100644 elf/dl-dst.h
  

Patch

diff --git a/elf/dl-deps.c b/elf/dl-deps.c
index 1504f8d606..3c8a5ebced 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-dst.h b/elf/dl-dst.h
deleted file mode 100644
index 50afd3ad93..0000000000
--- a/elf/dl-dst.h
+++ /dev/null
@@ -1,56 +0,0 @@ 
-/* Handling of dynamic string tokens.
-   Copyright (C) 1999-2025 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
-   <https://www.gnu.org/licenses/>.  */
-
-#include "trusted-dirs.h"
-
-/* Guess from the number of DSTs the length of the result string.  */
-#define DL_DST_REQUIRED(l, name, len, cnt) \
-  ({									      \
-    size_t __len = (len);						      \
-    size_t __cnt = (cnt);						      \
-									      \
-    if (__cnt > 0)							      \
-      {									      \
-	size_t dst_len;							      \
-	/* Now we make a guess how many extra characters on top of the	      \
-	   length of S we need to represent the result.  We know that	      \
-	   we have CNT replacements.  Each at most can use		      \
-	     MAX (MAX (strlen (ORIGIN), strlen (_dl_platform)),		      \
-		  strlen (DL_DST_LIB))					      \
-	   minus 4 (which is the length of "$LIB").			      \
-									      \
-	   First get the origin string if it is not available yet.	      \
-	   This can only happen for the map of the executable or, when	      \
-	   auditing, in ld.so.  */					      \
-	if ((l)->l_origin == NULL)					      \
-	  {								      \
-	    assert ((l)->l_name[0] == '\0' || is_rtld_link_map (l));	      \
-	    (l)->l_origin = _dl_get_origin ();				      \
-	    dst_len = ((l)->l_origin && (l)->l_origin != (char *) -1	      \
-			  ? strlen ((l)->l_origin) : 0);		      \
-	  }								      \
-	else								      \
-	  dst_len = (l)->l_origin == (char *) -1			      \
-	    ? 0 : strlen ((l)->l_origin);				      \
-	dst_len = MAX (MAX (dst_len, GLRO(dl_platformlen)),		      \
-		       strlen (DL_DST_LIB));				      \
-	if (dst_len > 4)						      \
-	  __len += __cnt * (dst_len - 4);				      \
-      }									      \
-									      \
-    __len; })
diff --git a/elf/dl-load.c b/elf/dl-load.c
index b583714c96..2e6c58dfcc 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -33,6 +33,7 @@ 
 #include <sys/types.h>
 #include <gnu/lib-names.h>
 #include <dl-tunables.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
@@ -68,7 +69,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>
@@ -218,61 +218,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
     {
@@ -309,7 +280,15 @@  _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.  If pre-loaded DSOs end up as
+		       the dlopen caller, use the main program for
+		       obtaining the origin.  */
+		      l->l_origin = _dl_get_origin ();
+		  repl = l->l_origin;
+		}
 
 	      check_for_trusted = (__libc_enable_secure
 				   && l->l_type == lt_executable);
@@ -321,7 +300,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)
@@ -329,16 +310,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');
@@ -353,15 +338,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;
 }
 
 
@@ -373,30 +362,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 60a1dce9de..4fb77e3ff7 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/elf/dl-origin.c b/elf/dl-origin.c
index 9f6b921b01..5d06f5bbe3 100644
--- a/elf/dl-origin.c
+++ b/elf/dl-origin.c
@@ -21,9 +21,6 @@ 
 #include <sys/param.h>
 #include <ldsodefs.h>
 
-#include <dl-dst.h>
-
-
 const char *
 _dl_get_origin (void)
 {
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index c0785cba04..e8418973ed 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1223,12 +1223,11 @@  extern struct link_map * _dl_get_dl_main_map (void) 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 decdd8ae9e..9c87ca3208 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>