[v2,03/19] elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries

The tunable privilege levels were a retrofit to try and keep the malloc
tunable environment variables' behavior unchanged across security
boundaries.  However, CVE-2023-4911 shows how tricky can be
tunable parsing in a security-sensitive environment.

Not only parsing, but the malloc tunable essentially changes some
semantics on setuid/setgid processes.  Although it is not a direct
security issue, allowing users to change setuid/setgid semantics is not
a good security practice, and requires extra code and analysis to check
if each tunable is safe to use on all security boundaries.

It also means that security opt-in features, like aarch64 MTE, would
need to be explicit enabled by an administrator with a wrapper script
or with a possible future system-wide tunable setting.

Co-authored-by: Siddhesh Poyarekar  <siddhesh@sourceware.org>
---
 elf/Makefile                       |   5 +-
 elf/dl-tunable-types.h             |  10 --
 elf/dl-tunables.c                  | 127 ++++-----------
 elf/dl-tunables.list               |   9 --
 elf/tst-env-setuid-tunables.c      |  29 +++-
 elf/tst-tunables.c                 | 244 +++++++++++++++++++++++++++++
 manual/README.tunables             |   9 --
 scripts/gen-tunables.awk           |  18 +--
 sysdeps/x86_64/64/dl-tunables.list |   1 -
 9 files changed, 299 insertions(+), 153 deletions(-)
 create mode 100644 elf/tst-tunables.c

On 2023-10-17 09:05, Adhemerval Zanella wrote:
> The tunable privilege levels were a retrofit to try and keep the malloc
> tunable environment variables' behavior unchanged across security
> boundaries.  However, CVE-2023-4911 shows how tricky can be
> tunable parsing in a security-sensitive environment.
> 
> Not only parsing, but the malloc tunable essentially changes some
> semantics on setuid/setgid processes.  Although it is not a direct
> security issue, allowing users to change setuid/setgid semantics is not
> a good security practice, and requires extra code and analysis to check
> if each tunable is safe to use on all security boundaries.
> 
> It also means that security opt-in features, like aarch64 MTE, would
> need to be explicit enabled by an administrator with a wrapper script
> or with a possible future system-wide tunable setting.
> 
> Co-authored-by: Siddhesh Poyarekar  <siddhesh@sourceware.org>

I'll leave this for someone else to review since I wrote some parts of this.

Thanks,
Sid

> ---
>   elf/Makefile                       |   5 +-
>   elf/dl-tunable-types.h             |  10 --
>   elf/dl-tunables.c                  | 127 ++++-----------
>   elf/dl-tunables.list               |   9 --
>   elf/tst-env-setuid-tunables.c      |  29 +++-
>   elf/tst-tunables.c                 | 244 +++++++++++++++++++++++++++++
>   manual/README.tunables             |   9 --
>   scripts/gen-tunables.awk           |  18 +--
>   sysdeps/x86_64/64/dl-tunables.list |   1 -
>   9 files changed, 299 insertions(+), 153 deletions(-)
>   create mode 100644 elf/tst-tunables.c
> 
> 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 695ba7192e..471895af7b 100644
> --- a/elf/dl-tunables.list
> +++ b/elf/dl-tunables.list
> @@ -21,14 +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:
> -#
> -# 	     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 {
> @@ -158,7 +150,6 @@ glibc {
>         type: INT_32
>         minval: 0
>         maxval: 255
> -      security_level: SXID_IGNORE
>       }
>     }
>   
> diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
> index 2603007b7b..ca02dbba58 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",
> @@ -74,6 +70,23 @@ test_child (int off)
>       ret = 0;
>     fflush (stdout);
>   
> +  /* Also check if the set tunables are effectively unchanged.  */
> +  int32_t check = TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL);
> +  size_t mmap_threshold = TUNABLE_GET_FULL (glibc, malloc, mmap_threshold,
> +					    size_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=%zu\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 ret;
>   }
>   
> diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
> new file mode 100644
> index 0000000000..57cf532fc4
> --- /dev/null
> +++ b/elf/tst-tunables.c
> @@ -0,0 +1,244 @@
> +/* 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 <stdint.h>
> +#include <stdlib.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;
> +  size_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, size_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
>       }
>     }
>   }

On 2023-10-18 09:04, Siddhesh Poyarekar wrote:
> On 2023-10-17 09:05, Adhemerval Zanella wrote:
>> The tunable privilege levels were a retrofit to try and keep the malloc
>> tunable environment variables' behavior unchanged across security
>> boundaries.  However, CVE-2023-4911 shows how tricky can be
>> tunable parsing in a security-sensitive environment.
>>
>> Not only parsing, but the malloc tunable essentially changes some
>> semantics on setuid/setgid processes.  Although it is not a direct
>> security issue, allowing users to change setuid/setgid semantics is not
>> a good security practice, and requires extra code and analysis to check
>> if each tunable is safe to use on all security boundaries.
>>
>> It also means that security opt-in features, like aarch64 MTE, would
>> need to be explicit enabled by an administrator with a wrapper script
>> or with a possible future system-wide tunable setting.
>>
>> Co-authored-by: Siddhesh Poyarekar  <siddhesh@sourceware.org>
> 
> I'll leave this for someone else to review since I wrote some parts of 
> this.

Carlos, can you please review this?

Thanks,
Sid

> 
> Thanks,
> Sid
> 
>> ---
>>   elf/Makefile                       |   5 +-
>>   elf/dl-tunable-types.h             |  10 --
>>   elf/dl-tunables.c                  | 127 ++++-----------
>>   elf/dl-tunables.list               |   9 --
>>   elf/tst-env-setuid-tunables.c      |  29 +++-
>>   elf/tst-tunables.c                 | 244 +++++++++++++++++++++++++++++
>>   manual/README.tunables             |   9 --
>>   scripts/gen-tunables.awk           |  18 +--
>>   sysdeps/x86_64/64/dl-tunables.list |   1 -
>>   9 files changed, 299 insertions(+), 153 deletions(-)
>>   create mode 100644 elf/tst-tunables.c
>>
>> 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 695ba7192e..471895af7b 100644
>> --- a/elf/dl-tunables.list
>> +++ b/elf/dl-tunables.list
>> @@ -21,14 +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:
>> -#
>> -#          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 {
>> @@ -158,7 +150,6 @@ glibc {
>>         type: INT_32
>>         minval: 0
>>         maxval: 255
>> -      security_level: SXID_IGNORE
>>       }
>>     }
>> diff --git a/elf/tst-env-setuid-tunables.c 
>> b/elf/tst-env-setuid-tunables.c
>> index 2603007b7b..ca02dbba58 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",
>> @@ -74,6 +70,23 @@ test_child (int off)
>>       ret = 0;
>>     fflush (stdout);
>> +  /* Also check if the set tunables are effectively unchanged.  */
>> +  int32_t check = TUNABLE_GET_FULL (glibc, malloc, check, int32_t, 
>> NULL);
>> +  size_t mmap_threshold = TUNABLE_GET_FULL (glibc, malloc, 
>> mmap_threshold,
>> +                        size_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=%zu\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 ret;
>>   }
>> diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
>> new file mode 100644
>> index 0000000000..57cf532fc4
>> --- /dev/null
>> +++ b/elf/tst-tunables.c
>> @@ -0,0 +1,244 @@
>> +/* 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 <stdint.h>
>> +#include <stdlib.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;
>> +  size_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, size_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
>>       }
>>     }
>>   }
>

One semi-related question.
Two spelling nits.
One comment nit.
Some suggestions for improving the test cases.

Ok with those addressed.

Reviewed-by: DJ Delorie <dj@redhat.com>

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> 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

Ok.

>  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)
> +

Ok.

> 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.  */

Ok.  Should there be a related patch to remove "security_level" entries
from dl-tunables.list?  I see support for parsing it in gen-tunables is
removed below, but leaving in the entry may create a false sense of
security.

I see a patch from the 3rd that does this, but it doesn't seem committed
yet.

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

Spelling: "a values" should be "a value" ?

> +   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;

Ok.

> -  char *p = tunestr;
> -  size_t off = 0;
> +  char *p = valstring;
> +  bool done = false;
>  
> -  while (true)
> +  while (!done)
>      {

Ok.

>        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++;

Ok, p points to the end of the tunable name

>        /* 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;

Missing value, ok.

>        /* We did not find a valid name-value pair before encountering the
>  	 colon.  */
> -      if (p[len]== ':')
> +      if (*p == ':')
>  	{
> -	  p += len + 1;
> +	  p++;
>  	  continue;
>  	}

Missing value, ok.

> -      p += len + 1;
> +      /* Skip the ':' or '='.  */
> +      p++;

Nit: This can't skip the ':' because we know there isn't one (above).

p now points to the start of the value.  Ok.

> -      /* Take the value from the valstring since we need to NULL terminate it.  */
> -      char *value = &valstring[p - tunestr];
> -      len = 0;
> +      const char *value = p;

"value" points to value, "valstring" still points to name.  Ok.

> -      while (p[len] != ':' && p[len] != '\0')
> -	len++;
> +      while (*p != ':' && *p != '\0')
> +	p++;
> +
> +      if (*p == '\0')
> +	done = true;
> +      else
> +	*p++ = '\0';

Ok.

> @@ -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;
>  	    }
>  	}

Ok.

> -
> -      /* We reached the end while processing the tunable string.  */
> -      if (p[len] == '\0')
> -	break;
> -
> -      p += len + 1;
>      }

Ok.  The "done" replaces this.

> -
> -  /* Terminate tunestr before we leave.  */
> -  if (__libc_enable_secure)
> -    tunestr[off] = '\0';
>  }

Ok.

> @@ -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;
> +

Ok.

>    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;
>  	}

We don't free these, but that's OK.

> @@ -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;
>  	    }

Ok.

> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
> index 695ba7192e..471895af7b 100644
> --- a/elf/dl-tunables.list
> +++ b/elf/dl-tunables.list
> @@ -21,14 +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:
> -#
> -# 	     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.

Ok.

> @@ -158,7 +150,6 @@ glibc {
>        type: INT_32
>        minval: 0
>        maxval: 255
> -      security_level: SXID_IGNORE
>      }
>    }

I assume this is based on some other patch which removes the rest, so ok.

> diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
> index 2603007b7b..ca02dbba58 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.  */

Ok.

> +#include <dl-tunables.h>

Ok.

> @@ -40,7 +36,7 @@
>  #include <support/test-driver.h>
>  #include <support/capture_subprocess.h>
>  
> -const char *teststrings[] =
> +static const char *teststrings[] =

Ok.

> @@ -74,6 +70,23 @@ test_child (int off)
>      ret = 0;
>    fflush (stdout);
>  
> +  /* Also check if the set tunables are effectively unchanged.  */
> +  int32_t check = TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL);
> +  size_t mmap_threshold = TUNABLE_GET_FULL (glibc, malloc, mmap_threshold,
> +					    size_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=%zu\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;
> +

Ok.

> diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
> new file mode 100644
> index 0000000000..57cf532fc4
> --- /dev/null
> +++ b/elf/tst-tunables.c
> @@ -0,0 +1,244 @@
> +/* 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 <stdint.h>
> +#include <stdlib.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>

Ok.

> +static int restart;
> +#define CMDLINE_OPTIONS \
> +  { "restart", no_argument, &restart, 1 },

Ok.

> +static const struct test_t
> +{
> +  const char *env;
> +  int32_t expected_malloc_check;
> +  size_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",

Suggest different values for check, so you know which is used.

> +    2,
> +    4096,
> +    0,
> +  },
> +  {
> +    "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",

Likewise.

> +    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,

Better to put different numbers here, so you know which is parsed, but ok.

> +    0,
> +    0,
> +  },
> +  {
> +    "glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
> +    2,
> +    4096,
> +    0,
> +  }
> +};

Ok.

> +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, size_t, NULL));
> +  TEST_COMPARE (tests[i].expected_perturb,
> +		TUNABLE_GET_FULL (glibc, malloc, perturb, int32_t, NULL));
> +  return 0;
> +}

Ok.

> +static int
> +do_test (int argc, char *argv[])
> +{
> +  /* We must have either:
> +     - One our fource parameters left if called initially:

Spelling?

> +       + 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);

Ok.

> +  if (restart)
> +    return handle_restart (atoi (argv[1]));

Set by --restart, ok.

> +  char nteststr[INT_BUFSIZE_BOUND (int)];

Ok.

> +  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;
> +  }

No overflow prevention, but this is internal, so ok.

> +  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;
> +}

Ok.

> +#define TEST_FUNCTION_ARGV do_test
> +#include <support/test-driver.c>

Ok.

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

Ok.

> 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"
> -    }

Ok.

> @@ -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
> -    }
> -  }

Ok.

> @@ -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]);
>    }

Ok.

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

Ok.

On 27/10/23 23:14, DJ Delorie wrote:
> 
> One semi-related question.
> Two spelling nits.
> One comment nit.
> Some suggestions for improving the test cases.
> 
> Ok with those addressed.

Thanks!

> 
> Reviewed-by: DJ Delorie <dj@redhat.com>
> 
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> 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
> 
> Ok.
> 
>>  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)
>> +
> 
> Ok.
> 
>> 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.  */
> 
> Ok.  Should there be a related patch to remove "security_level" entries
> from dl-tunables.list?  I see support for parsing it in gen-tunables is
> removed below, but leaving in the entry may create a false sense of
> security.
> 
> I see a patch from the 3rd that does this, but it doesn't seem committed
> yet.

It was an onverlook from my patch, I send a newer version with
security_level removed from all tunables list files.

> 
>> 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,
> 
> Spelling: "a values" should be "a value" ?

Ack.

> 
>> +   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;
> 
> Ok.
> 
>> -  char *p = tunestr;
>> -  size_t off = 0;
>> +  char *p = valstring;
>> +  bool done = false;
>>  
>> -  while (true)
>> +  while (!done)
>>      {
> 
> Ok.
> 
>>        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++;
> 
> Ok, p points to the end of the tunable name
> 
>>        /* 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;
> 
> Missing value, ok.
> 
>>        /* We did not find a valid name-value pair before encountering the
>>  	 colon.  */
>> -      if (p[len]== ':')
>> +      if (*p == ':')
>>  	{
>> -	  p += len + 1;
>> +	  p++;
>>  	  continue;
>>  	}
> 
> Missing value, ok.
> 
>> -      p += len + 1;
>> +      /* Skip the ':' or '='.  */
>> +      p++;
> 
> Nit: This can't skip the ':' because we know there isn't one (above).
> 
> p now points to the start of the value.  Ok.

Ack, I will fix the comment.

> 
>> -      /* Take the value from the valstring since we need to NULL terminate it.  */
>> -      char *value = &valstring[p - tunestr];
>> -      len = 0;
>> +      const char *value = p;
> 
> "value" points to value, "valstring" still points to name.  Ok.
> 
>> -      while (p[len] != ':' && p[len] != '\0')
>> -	len++;
>> +      while (*p != ':' && *p != '\0')
>> +	p++;
>> +
>> +      if (*p == '\0')
>> +	done = true;
>> +      else
>> +	*p++ = '\0';
> 
> Ok.
> 
>> @@ -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;
>>  	    }
>>  	}
> 
> Ok.
> 
>> -
>> -      /* We reached the end while processing the tunable string.  */
>> -      if (p[len] == '\0')
>> -	break;
>> -
>> -      p += len + 1;
>>      }
> 
> Ok.  The "done" replaces this.
> 
>> -
>> -  /* Terminate tunestr before we leave.  */
>> -  if (__libc_enable_secure)
>> -    tunestr[off] = '\0';
>>  }
> 
> Ok.
> 
>> @@ -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;
>> +
> 
> Ok.
> 
>>    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;
>>  	}
> 
> We don't free these, but that's OK.
> 
>> @@ -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;
>>  	    }
> 
> Ok.
> 
>> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
>> index 695ba7192e..471895af7b 100644
>> --- a/elf/dl-tunables.list
>> +++ b/elf/dl-tunables.list
>> @@ -21,14 +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:
>> -#
>> -# 	     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.
> 
> Ok.
> 
>> @@ -158,7 +150,6 @@ glibc {
>>        type: INT_32
>>        minval: 0
>>        maxval: 255
>> -      security_level: SXID_IGNORE
>>      }
>>    }
> 
> I assume this is based on some other patch which removes the rest, so ok.
> 
>> diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
>> index 2603007b7b..ca02dbba58 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.  */
> 
> Ok.
> 
>> +#include <dl-tunables.h>
> 
> Ok.
> 
>> @@ -40,7 +36,7 @@
>>  #include <support/test-driver.h>
>>  #include <support/capture_subprocess.h>
>>  
>> -const char *teststrings[] =
>> +static const char *teststrings[] =
> 
> Ok.
> 
>> @@ -74,6 +70,23 @@ test_child (int off)
>>      ret = 0;
>>    fflush (stdout);
>>  
>> +  /* Also check if the set tunables are effectively unchanged.  */
>> +  int32_t check = TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL);
>> +  size_t mmap_threshold = TUNABLE_GET_FULL (glibc, malloc, mmap_threshold,
>> +					    size_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=%zu\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;
>> +
> 
> Ok.
> 
>> diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
>> new file mode 100644
>> index 0000000000..57cf532fc4
>> --- /dev/null
>> +++ b/elf/tst-tunables.c
>> @@ -0,0 +1,244 @@
>> +/* 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 <stdint.h>
>> +#include <stdlib.h>
>> +#include <support/capture_subprocess.h>
>> +#include <support/check.h>
> 
> Ok.
> 
>> +static int restart;
>> +#define CMDLINE_OPTIONS \
>> +  { "restart", no_argument, &restart, 1 },
> 
> Ok.
> 
>> +static const struct test_t
>> +{
>> +  const char *env;
>> +  int32_t expected_malloc_check;
>> +  size_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",
> 
> Suggest different values for check, so you know which is used.

Ack.

> 
>> +    2,
>> +    4096,
>> +    0,
>> +  },
>> +  {
>> +    "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
> 
> Likewise.

Ack.

> 
>> +    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,
> 
> Better to put different numbers here, so you know which is parsed, but ok.
> 
>> +    0,
>> +    0,
>> +  },
>> +  {
>> +    "glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
>> +    2,
>> +    4096,
>> +    0,
>> +  }
>> +};
> 
> Ok.
> 
>> +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, size_t, NULL));
>> +  TEST_COMPARE (tests[i].expected_perturb,
>> +		TUNABLE_GET_FULL (glibc, malloc, perturb, int32_t, NULL));
>> +  return 0;
>> +}
> 
> Ok.
> 
>> +static int
>> +do_test (int argc, char *argv[])
>> +{
>> +  /* We must have either:
>> +     - One our fource parameters left if called initially:
> 
> Spelling?
> 
>> +       + 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);
> 
> Ok.
> 
>> +  if (restart)
>> +    return handle_restart (atoi (argv[1]));
> 
> Set by --restart, ok.
> 
>> +  char nteststr[INT_BUFSIZE_BOUND (int)];
> 
> Ok.
> 
>> +  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;
>> +  }
> 
> No overflow prevention, but this is internal, so ok.
> 
>> +  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;
>> +}
> 
> Ok.
> 
>> +#define TEST_FUNCTION_ARGV do_test
>> +#include <support/test-driver.c>
> 
> Ok.
> 
>> 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.
>> -
> 
> Ok.
> 
>> 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"
>> -    }
> 
> Ok.
> 
>> @@ -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
>> -    }
>> -  }
> 
> Ok.
> 
>> @@ -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]);
>>    }
> 
> Ok.
> 
>> 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
>>      }
>>    }
>>  }
> 
> Ok.
>