tunable: Ignore GLIBC_TUNABLES for AT_SECURE

TODO

Signed-off-by: Siddhesh Poyarekar  <siddhesh@sourceware.org>
Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

9 files changed, 303 insertions, 161 deletions

diff --git a/elf/Makefile b/elf/Makefile
index 9176cbf1e3..c824f7dab7 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -263,7 +263,6 @@ tests-static-normal := \
   tst-dl-iter-static \
   tst-dst-static \
   tst-env-setuid \
-  tst-env-setuid-tunables \
   tst-getauxval-static \
   tst-linkall-static \
   tst-single_threaded-pthread-static \
@@ -276,10 +275,12 @@ tests-static-normal := \
 tests-static-internal := \
   tst-dl-printf-static \
   tst-dl_find_object-static \
+  tst-env-setuid-tunables \
   tst-ptrguard1-static \
   tst-stackguard1-static \
   tst-tls1-static \
   tst-tls1-static-non-pie \
+  tst-tunables \
   # tests-static-internal
 
 CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o
@@ -2696,6 +2697,8 @@ $(objpfx)tst-glibc-hwcaps-mask.out: \
 # tst-glibc-hwcaps-cache.
 $(objpfx)tst-glibc-hwcaps-cache.out: $(objpfx)tst-glibc-hwcaps
 
+tst-tunables-ARGS = -- $(host-test-program-cmd)
+
 $(objpfx)list-tunables.out: tst-rtld-list-tunables.sh $(objpfx)ld.so
 	$(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' \
 	    '$(run_program_env)' > $(objpfx)/tst-rtld-list-tunables.out
diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
index c88332657e..62d6d9e629 100644
--- a/elf/dl-tunable-types.h
+++ b/elf/dl-tunable-types.h
@@ -64,16 +64,6 @@ struct _tunable
   tunable_val_t val;			/* The value.  */
   bool initialized;			/* Flag to indicate that the tunable is
 					   initialized.  */
-  tunable_seclevel_t security_level;	/* Specify the security level for the
-					   tunable with respect to AT_SECURE
-					   programs.  See description of
-					   tunable_seclevel_t to see a
-					   description of the values.
-
-					   Note that even if the tunable is
-					   read, it may not get used by the
-					   target module if the value is
-					   considered unsafe.  */
   /* Compatibility elements.  */
   const char env_alias[TUNABLE_ALIAS_MAX]; /* The compatibility environment
 					   variable name.  */
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 24252af22c..a83bd2b8bc 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -154,50 +154,51 @@ __tunable_set_val (tunable_id_t id, tunable_val_t *valp, tunable_num_t *minp,
   do_tunable_update_val (cur, valp, minp, maxp);
 }
 
-/* Parse the tunable string TUNESTR and adjust it to drop any tunables that may
-   be unsafe for AT_SECURE processes so that it can be used as the new
-   environment variable value for GLIBC_TUNABLES.  VALSTRING is the original
-   environment variable string which we use to make NULL terminated values so
-   that we don't have to allocate memory again for it.  */
+/* Parse the tunable string VALSTRING.  VALSTRING is a duplicated values,
+   where delimiters ':' are replaced with '\0', so string tunables are null
+   terminated.  */
 static void
-parse_tunables (char *tunestr, char *valstring)
+parse_tunables (char *valstring)
 {
-  if (tunestr == NULL || *tunestr == '\0')
+  if (valstring == NULL || *valstring == '\0')
     return;
 
-  char *p = tunestr;
-  size_t off = 0;
+  char *p = valstring;
+  bool done = false;
 
-  while (true)
+  while (!done)
     {
       char *name = p;
-      size_t len = 0;
 
       /* First, find where the name ends.  */
-      while (p[len] != '=' && p[len] != ':' && p[len] != '\0')
-	len++;
+      while (*p != '=' && *p != ':' && *p != '\0')
+	p++;
 
       /* If we reach the end of the string before getting a valid name-value
 	 pair, bail out.  */
-      if (p[len] == '\0')
+      if (*p == '\0')
 	break;
 
       /* We did not find a valid name-value pair before encountering the
 	 colon.  */
-      if (p[len]== ':')
+      if (*p == ':')
 	{
-	  p += len + 1;
+	  p++;
 	  continue;
 	}
 
-      p += len + 1;
+      /* Skip the ':' or '='.  */
+      p++;
 
-      /* Take the value from the valstring since we need to NULL terminate it.  */
-      char *value = &valstring[p - tunestr];
-      len = 0;
+      const char *value = p;
 
-      while (p[len] != ':' && p[len] != '\0')
-	len++;
+      while (*p != ':' && *p != '\0')
+	p++;
+
+      if (*p == '\0')
+	done = true;
+      else
+	*p++ = '\0';
 
       /* Add the tunable if it exists.  */
       for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
@@ -206,50 +207,11 @@ parse_tunables (char *tunestr, char *valstring)
 
 	  if (tunable_is_name (cur->name, name))
 	    {
-	      /* If we are in a secure context (AT_SECURE) then ignore the
-		 tunable unless it is explicitly marked as secure.  Tunable
-		 values take precedence over their envvar aliases.  We write
-		 the tunables that are not SXID_ERASE back to TUNESTR, thus
-		 dropping all SXID_ERASE tunables and any invalid or
-		 unrecognized tunables.  */
-	      if (__libc_enable_secure)
-		{
-		  if (cur->security_level != TUNABLE_SECLEVEL_SXID_ERASE)
-		    {
-		      if (off > 0)
-			tunestr[off++] = ':';
-
-		      const char *n = cur->name;
-
-		      while (*n != '\0')
-			tunestr[off++] = *n++;
-
-		      tunestr[off++] = '=';
-
-		      for (size_t j = 0; j < len; j++)
-			tunestr[off++] = value[j];
-		    }
-
-		  if (cur->security_level != TUNABLE_SECLEVEL_NONE)
-		    break;
-		}
-
-	      value[len] = '\0';
 	      tunable_initialize (cur, value);
 	      break;
 	    }
 	}
-
-      /* We reached the end while processing the tunable string.  */
-      if (p[len] == '\0')
-	break;
-
-      p += len + 1;
     }
-
-  /* Terminate tunestr before we leave.  */
-  if (__libc_enable_secure)
-    tunestr[off] = '\0';
 }
 
 /* Initialize the tunables list from the environment.  For now we only use the
@@ -263,16 +225,16 @@ __tunables_init (char **envp)
   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)
     {
       if (tunable_is_name ("GLIBC_TUNABLES", envname))
 	{
-	  char *new_env = tunables_strdup (envname);
-	  if (new_env != NULL)
-	    parse_tunables (new_env + len + 1, envval);
-	  /* Put in the updated envval.  */
-	  *prev_envp = new_env;
+	  parse_tunables (tunables_strdup (envval));
 	  continue;
 	}
 
@@ -290,39 +252,6 @@ __tunables_init (char **envp)
 	  /* We have a match.  Initialize and move on to the next line.  */
 	  if (tunable_is_name (name, envname))
 	    {
-	      /* For AT_SECURE binaries, we need to check the security settings of
-		 the tunable and decide whether we read the value and also whether
-		 we erase the value so that child processes don't inherit them in
-		 the environment.  */
-	      if (__libc_enable_secure)
-		{
-		  if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE)
-		    {
-		      /* Erase the environment variable.  */
-		      char **ep = prev_envp;
-
-		      while (*ep != NULL)
-			{
-			  if (tunable_is_name (name, *ep))
-			    {
-			      char **dp = ep;
-
-			      do
-				dp[0] = dp[1];
-			      while (*dp++);
-			    }
-			  else
-			    ++ep;
-			}
-		      /* Reset the iterator so that we read the environment again
-			 from the point we erased.  */
-		      envp = prev_envp;
-		    }
-
-		  if (cur->security_level != TUNABLE_SECLEVEL_NONE)
-		    continue;
-		}
-
 	      tunable_initialize (cur, envval);
 	      break;
 	    }
diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
index 44baf10eaa..fe26c76e1a 100644
--- a/elf/dl-tunables.list
+++ b/elf/dl-tunables.list
@@ -21,16 +21,6 @@
 # minval: Optional minimum acceptable value
 # maxval: Optional maximum acceptable value
 # env_alias: An alias environment variable
-# security_level: Specify security level of the tunable for AT_SECURE binaries.
-# 		  Valid values are as follows. There must be a strong, well
-# 		  documented reason for a tunable to be marked SXID_IGNORE or
-# 		  SXID_NONE:
-#
-# 	     SXID_ERASE: (default) Do not read and do not pass on to
-# 	     child processes.
-# 	     SXID_IGNORE: Do not read, but retain for non-AT_SECURE
-# 	     subprocesses.
-# 	     NONE: Read all the time.
 
 glibc {
   malloc {
diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
index 79795cdce7..1716c97088 100644
--- a/elf/tst-env-setuid-tunables.c
+++ b/elf/tst-env-setuid-tunables.c
@@ -15,14 +15,10 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-/* Verify that tunables correctly filter out unsafe tunables like
-   glibc.malloc.check and glibc.malloc.mmap_threshold but also retain
-   glibc.malloc.mmap_threshold in an unprivileged child.  */
-
-#define _LIBC 1
-#include "config.h"
-#undef _LIBC
+/* Verify that GLIBC_TUNABLES is kept unchanged but no tunable is actually
+   enabled for AT_SECURE processes.  */
 
+#include <dl-tunables.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <stdlib.h>
@@ -40,7 +36,7 @@
 #include <support/test-driver.h>
 #include <support/capture_subprocess.h>
 
-const char *teststrings[] =
+static const char *teststrings[] =
 {
   "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
   "glibc.malloc.check=2:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
@@ -64,20 +60,38 @@ static int
 test_child (int off)
 {
   const char *val = getenv ("GLIBC_TUNABLES");
+  int ret = 1;
 
   printf ("    [%d] GLIBC_TUNABLES is %s\n", off, val);
   fflush (stdout);
-  if (val != NULL && val[0] == '\0')
-    return 0;
-
-  if (val != NULL)
-    printf ("    [%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
-  else
+  if (val == NULL)
     printf ("    [%d] GLIBC_TUNABLES environment variable absent\n", off);
+  else
+    {
+      if (strcmp (val, teststrings[off]) != 0)
+	printf ("    [%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
+      else
+	ret = 0;
+    }
+  fflush (stdout);
+
+  int32_t check = TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL);
+  int32_t mmap_threshold = TUNABLE_GET_FULL (glibc, malloc, mmap_threshold,
+					     int32_t, NULL);
+  int32_t perturb = TUNABLE_GET_FULL (glibc, malloc, perturb, int32_t, NULL);
 
+  printf ("    [%d] glibc.malloc.check=%d\n", off, check);
+  fflush (stdout);
+  printf ("    [%d] glibc.malloc.mmap_threshold=%d\n", off, mmap_threshold);
   fflush (stdout);
+  printf ("    [%d] glibc.malloc.perturb=%d\n", off, perturb);
+  fflush (stdout);
+
+  ret |= check != 0;
+  ret |= mmap_threshold != 0;
+  ret |= perturb != 0;
 
-  return 1;
+  return ret;
 }
 
 static int
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
new file mode 100644
index 0000000000..0a9381ba1f
--- /dev/null
+++ b/elf/tst-tunables.c
@@ -0,0 +1,240 @@
+/* Check GLIBC_TUNABLES parsing.
+   Copyright (C) 2023 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 <array_length.h>
+#include <dl-tunables.h>
+#include <getopt.h>
+#include <intprops.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+
+static int restart;
+#define CMDLINE_OPTIONS \
+  { "restart", no_argument, &restart, 1 },
+
+static const struct test_t
+{
+  const char *env;
+  int32_t expected_malloc_check;
+  int32_t expected_mmap_threshold;
+  int32_t expected_perturb;
+} tests[] =
+{
+  /* Expected tunable format.  */
+  {
+    "glibc.malloc.check=2",
+    2,
+    0,
+    0,
+  },
+  {
+    "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+    2,
+    4096,
+    0,
+  },
+  /* Empty tunable are ignored.  */
+  {
+    "glibc.malloc.check=2::glibc.malloc.mmap_threshold=4096",
+    2,
+    4096,
+    0,
+  },
+  /* As well empty values.  */
+  {
+    "glibc.malloc.check=:glibc.malloc.mmap_threshold=4096",
+    0,
+    4096,
+    0,
+  },
+  /* Tunable are processed from left to right, so last one is the one set.  */
+  {
+    "glibc.malloc.check=1:glibc.malloc.check=2",
+    2,
+    0,
+    0,
+  },
+  {
+    "glibc.malloc.check=2:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+    2,
+    4096,
+    0,
+  },
+  {
+    "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
+    2,
+    4096,
+    0,
+  },
+  /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged.  */
+  {
+    "glibc.malloc.perturb=0x800",
+    0,
+    0,
+    0,
+  },
+  {
+    "glibc.malloc.perturb=0x55",
+    0,
+    0,
+    0x55,
+  },
+  /* Out of range values are just ignored.  */
+  {
+    "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
+    0,
+    4096,
+    0,
+  },
+  /* Invalid keys are ignored.  */
+  {
+    ":glibc.malloc.garbage=2:glibc.malloc.check=1",
+    1,
+    0,
+    0,
+  },
+  {
+    "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+    0,
+    4096,
+    0,
+  },
+  {
+    "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",
+    0,
+    4096,
+    0,
+  },
+  {
+    "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+    0,
+    4096,
+    0,
+  },
+  /* Invalid subkeys are ignored.  */
+  {
+    "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",
+    2,
+    0,
+    0,
+  },
+  {
+    "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",
+    0,
+    0,
+    0,
+  },
+  {
+    "not_valid.malloc.check=2",
+    0,
+    0,
+    0,
+  },
+  {
+    "glibc.not_valid.check=2",
+    0,
+    0,
+    0,
+  },
+  /* 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.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
+    0,
+    0,
+    0,
+  },
+  /* The ill-formatted tunable is also skipped.  */
+  {
+    "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
+    2,
+    0,
+    0,
+  },
+  /* For an integer tunable, parse will stop on non number character.  */
+  {
+    "glibc.malloc.check=2=2",
+    2,
+    0,
+    0,
+  },
+  {
+    "glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
+    2,
+    4096,
+    0,
+  }
+};
+
+static int
+handle_restart (int i)
+{
+  TEST_COMPARE (tests[i].expected_malloc_check,
+		TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL));
+  TEST_COMPARE (tests[i].expected_mmap_threshold,
+		TUNABLE_GET_FULL (glibc, malloc, mmap_threshold, int32_t, NULL));
+  TEST_COMPARE (tests[i].expected_perturb,
+		TUNABLE_GET_FULL (glibc, malloc, perturb, int32_t, NULL));
+  return 0;
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+  /* We must have either:
+     - One our fource parameters left if called initially:
+       + path to ld.so         optional
+       + "--library-path"      optional
+       + the library path      optional
+       + the application name
+       + the test to check  */
+
+  TEST_VERIFY_EXIT (argc == 2 || argc == 5);
+
+  if (restart)
+    return handle_restart (atoi (argv[1]));
+
+  char nteststr[INT_BUFSIZE_BOUND (int)];
+
+  char *spargv[10];
+  int i = 0;
+  for (; i < argc - 1; i++)
+    spargv[i] = argv[i + 1];
+  spargv[i++] = (char *) "--direct";
+  spargv[i++] = (char *) "--restart";
+  spargv[i++] = nteststr;
+  spargv[i] = NULL;
+
+  for (int i = 0; i < array_length (tests); i++)
+    {
+      snprintf (nteststr, sizeof nteststr, "%d", i);
+
+      printf ("[%d] Spawned test for %s\n", i, tests[i].env);
+      setenv ("GLIBC_TUNABLES", tests[i].env, 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);
+    }
+
+  return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/manual/README.tunables b/manual/README.tunables
index 605ddd78cd..72ae00dc02 100644
--- a/manual/README.tunables
+++ b/manual/README.tunables
@@ -59,15 +59,6 @@ The list of allowed attributes are:
 
 - env_alias:		An alias environment variable
 
-- security_level:	Specify security level of the tunable for AT_SECURE
-			binaries.  Valid values are:
-
-			SXID_ERASE: (default) Do not read and do not pass on to
-			child processes.
-			SXID_IGNORE: Do not read, but retain for non-AT_SECURE
-			child processes.
-			NONE: Read all the time.
-
 2. Use TUNABLE_GET/TUNABLE_SET/TUNABLE_SET_WITH_BOUNDS to get and set tunables.
 
 3. OPTIONAL: If tunables in a namespace are being used multiple times within a
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index d6de100df0..1e9d6b534e 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -61,9 +61,6 @@ $1 == "}" {
     if (!env_alias[top_ns,ns,tunable]) {
       env_alias[top_ns,ns,tunable] = "{0}"
     }
-    if (!security_level[top_ns,ns,tunable]) {
-      security_level[top_ns,ns,tunable] = "SXID_ERASE"
-    }
     len = length(top_ns"."ns"."tunable)
     if (len > max_name_len)
       max_name_len = len
@@ -118,17 +115,6 @@ $1 == "}" {
     if (len > max_alias_len)
       max_alias_len = len
   }
-  else if (attr == "security_level") {
-    if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") {
-      security_level[top_ns,ns,tunable] = val
-    }
-    else {
-      printf("Line %d: Invalid value (%s) for security_level: %s, ", NR, val,
-	     $0)
-      print("Allowed values are 'SXID_ERASE', 'SXID_IGNORE', or 'NONE'")
-      exit 1
-    }
-  }
   else if (attr == "default") {
     if (types[top_ns,ns,tunable] == "STRING") {
       default_val[top_ns,ns,tunable] = sprintf(".strval = \"%s\"", val);
@@ -177,9 +163,9 @@ END {
     n = indices[2];
     m = indices[3];
     printf ("  {TUNABLE_NAME_S(%s, %s, %s)", t, n, m)
-    printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, false, TUNABLE_SECLEVEL_%s, %s},\n",
+    printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, false, %s},\n",
 	    types[t,n,m], minvals[t,n,m], maxvals[t,n,m],
-	    default_val[t,n,m], security_level[t,n,m], env_alias[t,n,m]);
+	    default_val[t,n,m], env_alias[t,n,m]);
   }
   print "};"
   print "#endif"
diff --git a/sysdeps/x86_64/64/dl-tunables.list b/sysdeps/x86_64/64/dl-tunables.list
index 0aab52e662..54a216a461 100644
--- a/sysdeps/x86_64/64/dl-tunables.list
+++ b/sysdeps/x86_64/64/dl-tunables.list
@@ -23,7 +23,6 @@ glibc {
       minval: 0
       maxval: 1
       env_alias: LD_PREFER_MAP_32BIT_EXEC
-      security_level: SXID_IGNORE
     }
   }
 }