elf: Do not duplicate the GLIBC_TUNABLES string

The tunable parsing duplicates the tunable environment variable so it
null-terminates each one since it simplifies the later parsing. It has
the drawback of adding another point of failure (__minimal_malloc
failing), and the memory copy requires tuning the compiler to avoid mem
operations calls.

The parsing now tracks the tunable start and its size. The
dl-tunable-parse.h adds helper functions to help parsing, like a strcmp
that also checks for size and an iterator for suboptions that are
comma-separated (used on hwcap parsing by x86, powerpc, and s390x).

Since the environment variable is allocated on the stack by the kernel,
it is safe to keep the references to the suboptions for later parsing
of string tunables (as done by set_hwcaps by multiple architectures).

Checked on x86_64-linux-gnu, powerpc64le-linux-gnu, and
aarch64-linux-gnu.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

3 files changed, 99 insertions, 53 deletions

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 644d21d1b0..3d41e8e28e 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -36,31 +36,8 @@
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
 
-#include <not-errno.h>
-
-static char *
-tunables_strdup (const char *in)
-{
-  size_t i = 0;
-
-  while (in[i++] != '\0');
-  char *out = __minimal_malloc (i + 1);
-
-  /* For most of the tunables code, we ignore user errors.  However,
-     this is a system error - and running out of memory at program
-     startup should be reported, so we do.  */
-  if (out == NULL)
-    _dl_fatal_printf ("failed to allocate memory to process tunables\n");
-
-  while (i-- > 0)
-    out[i] = in[i];
-
-  return out;
-}
-
 static char **
-get_next_env (char **envp, char **name, size_t *namelen, char **val,
-	      char ***prev_envp)
+get_next_env (char **envp, char **name, char **val, char ***prev_envp)
 {
   while (envp != NULL && *envp != NULL)
     {
@@ -76,7 +53,6 @@ get_next_env (char **envp, char **name, size_t *namelen, char **val,
 	continue;
 
       *name = envline;
-      *namelen = len;
       *val = &envline[len + 1];
       *prev_envp = prev;
 
@@ -134,14 +110,14 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
 /* Validate range of the input value and initialize the tunable CUR if it looks
    good.  */
 static void
-tunable_initialize (tunable_t *cur, const char *strval)
+tunable_initialize (tunable_t *cur, const char *strval, size_t len)
 {
-  tunable_val_t val;
+  tunable_val_t val = { 0 };
 
   if (cur->type.type_code != TUNABLE_TYPE_STRING)
     val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
   else
-    val.strval = strval;
+    val.strval = (struct tunable_str_t) { strval, len };
   do_tunable_update_val (cur, &val, NULL, NULL);
 }
 
@@ -165,29 +141,29 @@ struct tunable_toset_t
 {
   tunable_t *t;
   const char *value;
+  size_t len;
 };
 
 enum { tunables_list_size = array_length (tunable_list) };
 
 /* Parse the tunable string VALSTRING and set TUNABLES with the found tunables
-   and their respective strings.  VALSTRING is a duplicated values,  where
-   delimiters ':' are replaced with '\0', so string tunables are null
-   terminated.
+   and their respective values.  The VALSTRING is parsed in place, with the
+   tunable start and size recorded in TUNABLES.
    Return the number of tunables found (including 0 if the string is empty)
    or -1 if for an ill-formatted definition.  */
 static int
-parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
+parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
 {
   if (valstring == NULL || *valstring == '\0')
     return 0;
 
-  char *p = valstring;
+  const char *p = valstring;
   bool done = false;
   int ntunables = 0;
 
   while (!done)
     {
-      char *name = p;
+      const char *name = p;
 
       /* First, find where the name ends.  */
       while (*p != '=' && *p != ':' && *p != '\0')
@@ -209,7 +185,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
       /* Skip the '='.  */
       p++;
 
-      char *value = p;
+      const char *value = p;
 
       while (*p != '=' && *p != ':' && *p != '\0')
 	p++;
@@ -218,8 +194,6 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
 	return -1;
       else if (*p == '\0')
 	done = true;
-      else
-	*p++ = '\0';
 
       /* Add the tunable if it exists.  */
       for (size_t i = 0; i < tunables_list_size; i++)
@@ -228,7 +202,8 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
 
 	  if (tunable_is_name (cur->name, name))
 	    {
-	      tunables[ntunables++] = (struct tunable_toset_t) { cur, value };
+	      tunables[ntunables++] =
+		(struct tunable_toset_t) { cur, value, p - value };
 	      break;
 	    }
 	}
@@ -238,7 +213,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
 }
 
 static void
-parse_tunables (char *valstring)
+parse_tunables (const char *valstring)
 {
   struct tunable_toset_t tunables[tunables_list_size];
   int ntunables = parse_tunables_string (valstring, tunables);
@@ -250,7 +225,7 @@ parse_tunables (char *valstring)
     }
 
   for (int i = 0; i < ntunables; i++)
-    tunable_initialize (tunables[i].t, tunables[i].value);
+    tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
 }
 
 /* Initialize the tunables list from the environment.  For now we only use the
@@ -261,19 +236,20 @@ __tunables_init (char **envp)
 {
   char *envname = NULL;
   char *envval = NULL;
-  size_t len = 0;
   char **prev_envp = envp;
 
   /* Ignore tunables for AT_SECURE programs.  */
   if (__libc_enable_secure)
     return;
 
-  while ((envp = get_next_env (envp, &envname, &len, &envval,
-			       &prev_envp)) != NULL)
+  while ((envp = get_next_env (envp, &envname, &envval, &prev_envp)) != NULL)
     {
+      /* The environment variable is allocated on the stack by the kernel, so
+	 it is safe to keep the references to the suboptions for later parsing
+	 of string tunables.  */
       if (tunable_is_name ("GLIBC_TUNABLES", envname))
 	{
-	  parse_tunables (tunables_strdup (envval));
+	  parse_tunables (envval);
 	  continue;
 	}
 
@@ -291,7 +267,11 @@ __tunables_init (char **envp)
 	  /* We have a match.  Initialize and move on to the next line.  */
 	  if (tunable_is_name (name, envname))
 	    {
-	      tunable_initialize (cur, envval);
+	      size_t envvallen = 0;
+	      /* The environment variable is always null-terminated.  */
+	      for (const char *p = envval; *p != '\0'; p++, envvallen++);
+
+	      tunable_initialize (cur, envval, envvallen);
 	      break;
 	    }
 	}
@@ -305,7 +285,7 @@ __tunables_print (void)
     {
       const tunable_t *cur = &tunable_list[i];
       if (cur->type.type_code == TUNABLE_TYPE_STRING
-	  && cur->val.strval == NULL)
+	  && cur->val.strval.str == NULL)
 	_dl_printf ("%s:\n", cur->name);
       else
 	{
@@ -331,7 +311,9 @@ __tunables_print (void)
 			  (size_t) cur->type.max);
 	      break;
 	    case TUNABLE_TYPE_STRING:
-	      _dl_printf ("%s\n", cur->val.strval);
+	      _dl_printf ("%.*s\n",
+			  (int) cur->val.strval.len,
+			  cur->val.strval.str);
 	      break;
 	    default:
 	      __builtin_unreachable ();
@@ -364,7 +346,7 @@ __tunable_get_default (tunable_id_t id, void *valp)
 	}
     case TUNABLE_TYPE_STRING:
 	{
-	  *((const char **)valp) = cur->def.strval;
+	  *((const struct tunable_str_t **)valp) = &cur->def.strval;
 	  break;
 	}
     default:
@@ -399,7 +381,7 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback)
 	}
     case TUNABLE_TYPE_STRING:
 	{
-	  *((const char **)valp) = cur->val.strval;
+	  *((const struct tunable_str_t **) valp) = &cur->val.strval;
 	  break;
 	}
     default:
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 0df4dde24e..5d5ee2c3aa 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -30,7 +30,11 @@ typedef intmax_t tunable_num_t;
 typedef union
 {
   tunable_num_t numval;
-  const char *strval;
+  struct tunable_str_t
+  {
+    const char *str;
+    size_t len;
+  } strval;
 } tunable_val_t;
 
 typedef void (*tunable_callback_t) (tunable_val_t *);
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
index e1ad44f27c..188345b070 100644
--- a/elf/tst-tunables.c
+++ b/elf/tst-tunables.c
@@ -31,7 +31,8 @@ static int restart;
 
 static const struct test_t
 {
-  const char *env;
+  const char *name;
+  const char *value;
   int32_t expected_malloc_check;
   size_t expected_mmap_threshold;
   int32_t expected_perturb;
@@ -39,12 +40,14 @@ static const struct test_t
 {
   /* Expected tunable format.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2",
     2,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
     2,
     4096,
@@ -52,6 +55,7 @@ static const struct test_t
   },
   /* Empty tunable are ignored.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2::glibc.malloc.mmap_threshold=4096",
     2,
     4096,
@@ -59,6 +63,7 @@ static const struct test_t
   },
   /* As well empty values.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=:glibc.malloc.mmap_threshold=4096",
     0,
     4096,
@@ -66,18 +71,21 @@ static const struct test_t
   },
   /* Tunable are processed from left to right, so last one is the one set.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=1:glibc.malloc.check=2",
     2,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=1:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
     2,
     4096,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=1",
     1,
     4096,
@@ -85,12 +93,14 @@ static const struct test_t
   },
   /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.perturb=0x800",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.perturb=0x55",
     0,
     0,
@@ -98,6 +108,7 @@ static const struct test_t
   },
   /* Out of range values are just ignored.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
     0,
     4096,
@@ -105,24 +116,28 @@ static const struct test_t
   },
   /* Invalid keys are ignored.  */
   {
+    "GLIBC_TUNABLES",
     ":glibc.malloc.garbage=2:glibc.malloc.check=1",
     1,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
     0,
     4096,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",
     0,
     4096,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
     0,
     4096,
@@ -130,24 +145,28 @@ static const struct test_t
   },
   /* Invalid subkeys are ignored.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",
     2,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "not_valid.malloc.check=2",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.not_valid.check=2",
     0,
     0,
@@ -156,6 +175,7 @@ static const struct test_t
   /* An ill-formatted tunable in the for key=key=value will considere the
      value as 'key=value' (which can not be parsed as an integer).  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
     0,
     0,
@@ -163,41 +183,77 @@ static const struct test_t
   },
   /* Ill-formatted tunables string is not parsed.  */
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2=2",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2=2:glibc.malloc.check=2",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
     0,
     0,
     0,
   },
   {
+    "GLIBC_TUNABLES",
     "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
     0,
     0,
     0,
   },
+  /* Also check some tunable aliases.  */
+  {
+    "MALLOC_CHECK_",
+    "2",
+    2,
+    0,
+    0,
+  },
+  {
+    "MALLOC_MMAP_THRESHOLD_",
+    "4096",
+    0,
+    4096,
+    0,
+  },
+  {
+    "MALLOC_PERTURB_",
+    "0x55",
+    0,
+    0,
+    0x55,
+  },
+  /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged.  */
+  {
+    "MALLOC_PERTURB_",
+    "0x800",
+    0,
+    0,
+    0,
+  },
 };
 
 static int
@@ -245,13 +301,17 @@ do_test (int argc, char *argv[])
     {
       snprintf (nteststr, sizeof nteststr, "%d", i);
 
-      printf ("[%d] Spawned test for %s\n", i, tests[i].env);
-      setenv ("GLIBC_TUNABLES", tests[i].env, 1);
+      printf ("[%d] Spawned test for %s=%s\n",
+	      i,
+	      tests[i].name,
+	      tests[i].value);
+      setenv (tests[i].name, tests[i].value, 1);
       struct support_capture_subprocess result
 	= support_capture_subprogram (spargv[0], spargv);
       support_capture_subprocess_check (&result, "tst-tunables", 0,
 					sc_allow_stderr);
       support_capture_subprocess_free (&result);
+      unsetenv (tests[i].name);
     }
 
   return 0;