public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Florian Weimer <fweimer@redhat.com>
To: libc-alpha@sourceware.org
Subject: [PATCH 11/33] elf: Merge the three implementations of _dl_dst_substitute
Date: Tue, 04 Jul 2023 22:03:02 +0200	[thread overview]
Message-ID: <17f407415e7898f078bb3c9ad656403f9ce60ffe.1688499219.git.fweimer@redhat.com> (raw)
In-Reply-To: <cover.1688499219.git.fweimer@redhat.com>

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 --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>
-- 
2.41.0



  parent reply	other threads:[~2023-07-04 20:03 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-04 20:02 [PATCH 00/33] RFC: RELRO link maps Florian Weimer
2023-07-04 20:02 ` [PATCH 01/33] support: Add <support/memprobe.h> for protection flags probing Florian Weimer
2023-07-04 20:02 ` [PATCH 02/33] misc: Enable internal use of memory protection keys Florian Weimer
2023-07-04 20:02 ` [PATCH 03/33] elf: Remove _dl_sysdep_open_object hook function Florian Weimer
2023-07-04 20:02 ` [PATCH 04/33] elf: Eliminate second loop in find_version in dl-version.c Florian Weimer
2023-07-04 20:02 ` [PATCH 05/33] elf: In rtld_setup_main_map, assume ld.so has a DYNAMIC segment Florian Weimer
2023-07-04 20:02 ` [PATCH 06/33] elf: Remove version assert in check_match in elf/dl-lookup.c Florian Weimer
2023-07-04 20:02 ` [PATCH 07/33] elf: Disambiguate some failures in _dl_load_cache_lookup Florian Weimer
2023-07-04 20:02 ` [PATCH 08/33] elf: Eliminate alloca in open_verify Florian Weimer
2023-07-04 20:02 ` [PATCH 09/33] Do not export <alloc_buffer.h> functions from libc Florian Weimer
2023-07-04 20:02 ` [PATCH 10/33] elf: Make <alloc_buffer.h> usable in ld.so Florian Weimer
2023-07-04 20:03 ` Florian Weimer [this message]
2023-07-04 20:03 ` [PATCH 12/33] elf: _dl_find_object may return 1 during early startup (bug 30515) Florian Weimer
2023-07-04 20:03 ` [PATCH 13/33] elf: Move __rtld_malloc_init_stubs call into _dl_start_final Florian Weimer
2023-07-04 20:03 ` [PATCH 14/33] elf: Merge __dl_libc_freemem into __rtld_libc_freeres Florian Weimer
2023-07-04 20:03 ` [PATCH 15/33] elf: Use struct link_map_private for the internal link map Florian Weimer
2023-07-04 20:03 ` [PATCH 16/33] elf: Remove run-time-writable fields from struct link_map_private Florian Weimer
2023-07-04 20:03 ` [PATCH 17/33] elf: Move l_tls_offset into read-write part of link map Florian Weimer
2023-07-04 20:03 ` [PATCH 18/33] elf: Allocate auditor state after read-write " Florian Weimer
2023-07-04 20:03 ` [PATCH 19/33] elf: Move link map fields used by dependency sorting to writable part Florian Weimer
2023-07-04 20:03 ` [PATCH 20/33] elf: Split _dl_lookup_map, _dl_map_new_object from _dl_map_object Florian Weimer
2023-07-04 20:03 ` [PATCH 21/33] elf: Add l_soname accessor function for DT_SONAME values Florian Weimer
2023-07-04 20:03 ` [PATCH 22/33] elf: _dl_rtld_map should not exist in static builds Florian Weimer
2023-07-04 20:03 ` [PATCH 23/33] elf: Introduce GLPM accessor for the protected memory area Florian Weimer
2023-07-04 20:04 ` [PATCH 24/33] elf: Bootstrap allocation for future protected memory allocator Florian Weimer
2023-07-04 20:04 ` [PATCH 25/33] elf: Implement a basic " Florian Weimer
2023-07-04 20:04 ` [PATCH 26/33] elf: Move most of the _dl_find_object data to the protected heap Florian Weimer
2023-07-04 20:04 ` [PATCH 27/33] elf: Switch to a region-based protected memory allocator Florian Weimer
2023-07-04 20:04 ` [PATCH 28/33] elf: Determine the caller link map in _dl_open Florian Weimer
2023-07-04 20:04 ` [PATCH 29/33] elf: Add fast path to dlopen for fully-opened maps Florian Weimer
2023-07-04 20:04 ` [PATCH 30/33] elf: Use _dl_find_object instead of _dl_find_dso_for_object in dlopen Florian Weimer
2023-07-04 20:04 ` [PATCH 31/33] elf: Put critical _dl_find_object pointers into protected memory area Florian Weimer
2023-07-04 20:04 ` [PATCH 32/33] elf: Add hash tables to speed up DT_NEEDED, dlopen lookups Florian Weimer
2023-07-04 20:04 ` [PATCH 33/33] elf: Use memory protection keys for the protected memory allocator Florian Weimer
2023-07-04 20:07 ` [PATCH 00/33] RFC: RELRO link maps Florian Weimer
2023-07-05 15:54   ` Carlos O'Donell
2023-07-05 15:57     ` Florian Weimer
2023-07-05 17:48       ` Carlos O'Donell
2023-07-05 17:58         ` Adhemerval Zanella Netto
2023-07-07 11:10           ` Florian Weimer
2023-07-07 12:42             ` Florian Weimer
2023-07-07 12:48               ` Adhemerval Zanella Netto
2023-07-07 13:18                 ` Florian Weimer
2023-07-07 13:58                   ` Adhemerval Zanella Netto
2023-07-07 14:55                     ` Florian Weimer

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=17f407415e7898f078bb3c9ad656403f9ce60ffe.1688499219.git.fweimer@redhat.com \
    --to=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /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).