public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] More tunable fixes
@ 2024-05-06 16:18 Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 1/4] elf: Only process multiple tunable once (BZ 31686) Adhemerval Zanella
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2024-05-06 16:18 UTC (permalink / raw)
  To: libc-alpha; +Cc: Joe Simmons-Talbott, Siddhesh Poyarekar

The 680c597e9c3 commit made loader reject ill-formatted strings by
first tracking all set tunables and then applying them. However, it does
not take into consideration if the same tunable is set multiple times,
where parse_tunables_string appends the found tunable without checking
if it was already in the list. It leads to a stack-based buffer overflow
if the tunable is specified more than the total number of
tunables (BZ 31686).

While fixing this issue, I noted that the new glibc.rtld.enable_secure
check could be optimized a bit to avoid the string comparison on the
tunable loop.

I also found an issue where it does have the handle case where the
environment alias is handled before the GLIBC_TUNABLES, which will
change the tunable even if glibc.rtld.enable_secure it set to 0.  Fixing
it allows us to optimize the environment alias parsing a bit, since only
tunable with aliases need to be checked (instead of the whole list).

Changes from v2:
* Fixed typos and improve comments.

Changes from v1:
* Do not change tunables internal position.

Adhemerval Zanella (4):
  elf: Only process multiple tunable once (BZ 31686)
  elf: Remove glibc.rtld.enable_secure check from parse_tunables_string
  support: Add envp argument to support_capture_subprogram
  elf: Make glibc.rtld.enable_secure ignore alias environment variables

 elf/dl-tunables.c                          | 114 +++++++++++------
 elf/tst-audit18.c                          |   2 +-
 elf/tst-audit19b.c                         |   2 +-
 elf/tst-audit22.c                          |   2 +-
 elf/tst-audit23.c                          |   2 +-
 elf/tst-audit25a.c                         |   4 +-
 elf/tst-audit25b.c                         |   4 +-
 elf/tst-glibc-hwcaps-2-cache.c             |   2 +-
 elf/tst-rtld-run-static.c                  |   4 +-
 elf/tst-tunables-enable_secure.c           | 136 ++++++++++++++++++---
 elf/tst-tunables.c                         |  63 +++++++++-
 scripts/gen-tunables.awk                   |  16 ++-
 support/capture_subprocess.h               |   9 +-
 support/subprocess.h                       |   7 +-
 support/support_capture_subprocess.c       |   5 +-
 support/support_subprocess.c               |   5 +-
 support/tst-support_capture_subprocess.c   |   2 +-
 sysdeps/aarch64/multiarch/memset_generic.S |   4 +
 sysdeps/sparc/sparc64/rtld-memset.c        |   3 +
 sysdeps/x86/tst-hwcap-tunables.c           |   2 +-
 20 files changed, 310 insertions(+), 78 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/4] elf: Only process multiple tunable once (BZ 31686)
  2024-05-06 16:18 [PATCH v3 0/4] More tunable fixes Adhemerval Zanella
@ 2024-05-06 16:18 ` Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 2/4] elf: Remove glibc.rtld.enable_secure check from parse_tunables_string Adhemerval Zanella
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2024-05-06 16:18 UTC (permalink / raw)
  To: libc-alpha
  Cc: Joe Simmons-Talbott, Siddhesh Poyarekar, Yuto Maeda, Yutaro Shimizu

The 680c597e9c3 commit made loader reject ill-formatted strings by
first tracking all set tunables and then applying them. However, it does
not take into consideration if the same tunable is set multiple times,
where parse_tunables_string appends the found tunable without checking
if it was already in the list. It leads to a stack-based buffer overflow
if the tunable is specified more than the total number of tunables.  For
instance:

  GLIBC_TUNABLES=glibc.malloc.check=2:... (repeat over the number of
  total support for different tunable).

Instead, use the index of the tunable list to get the expected tunable
entry.  Since now the initial list is zero-initialized, the compiler
might emit an extra memset and this requires some minor adjustment
on some ports.

Checked on x86_64-linux-gnu and aarch64-linux-gnu.

Reported-by: Yuto Maeda <maeda@cyberdefense.jp>
Reported-by: Yutaro Shimizu <shimizu@cyberdefense.jp>
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 elf/dl-tunables.c                          | 30 ++++++-----
 elf/tst-tunables.c                         | 61 +++++++++++++++++++++-
 sysdeps/aarch64/multiarch/memset_generic.S |  4 ++
 sysdeps/sparc/sparc64/rtld-memset.c        |  3 ++
 4 files changed, 84 insertions(+), 14 deletions(-)

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index d3ccd2ecd4..1db80e0f92 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -32,6 +32,7 @@
 #include <ldsodefs.h>
 #include <array_length.h>
 #include <dl-minimal-malloc.h>
+#include <dl-symbol-redir-ifunc.h>
 
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
@@ -221,8 +222,7 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
 
 	  if (tunable_is_name (cur->name, name))
 	    {
-	      tunables[ntunables++] =
-		(struct tunable_toset_t) { cur, value, p - value };
+	      tunables[i] = (struct tunable_toset_t) { cur, value, p - value };
 
 	      /* Ignore tunables if enable_secure is set */
 	      if (tunable_is_name ("glibc.rtld.enable_secure", name))
@@ -245,23 +245,27 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
 static void
 parse_tunables (const char *valstring)
 {
-  struct tunable_toset_t tunables[tunables_list_size];
-  int ntunables = parse_tunables_string (valstring, tunables);
-  if (ntunables == -1)
+  struct tunable_toset_t tunables[tunables_list_size] = { 0 };
+  if (parse_tunables_string (valstring, tunables) == -1)
     {
       _dl_error_printf (
         "WARNING: ld.so: invalid GLIBC_TUNABLES `%s': ignored.\n", valstring);
       return;
     }
 
-  for (int i = 0; i < ntunables; i++)
-    if (!tunable_initialize (tunables[i].t, tunables[i].value,
-			     tunables[i].len))
-      _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' "
-		       "for option `%s': ignored.\n",
-		       (int) tunables[i].len,
-		       tunables[i].value,
-		       tunables[i].t->name);
+  for (int i = 0; i < tunables_list_size; i++)
+    {
+      if (tunables[i].t == NULL)
+	continue;
+
+      if (!tunable_initialize (tunables[i].t, tunables[i].value,
+			       tunables[i].len))
+	_dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' "
+			  "for option `%s': ignored.\n",
+			  (int) tunables[i].len,
+			  tunables[i].value,
+			  tunables[i].t->name);
+    }
 }
 
 /* Initialize the tunables list from the environment.  For now we only use the
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
index 095b5c81d9..dff34ed748 100644
--- a/elf/tst-tunables.c
+++ b/elf/tst-tunables.c
@@ -17,6 +17,10 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <array_length.h>
+/* The test uses the tunable_list size, which is only exported for
+   ld.so.  This will result in a copy of tunable_list, which is ununsed by
+   the test itself.  */
+#define TUNABLES_INTERNAL 1
 #include <dl-tunables.h>
 #include <getopt.h>
 #include <intprops.h>
@@ -24,12 +28,13 @@
 #include <stdlib.h>
 #include <support/capture_subprocess.h>
 #include <support/check.h>
+#include <support/support.h>
 
 static int restart;
 #define CMDLINE_OPTIONS \
   { "restart", no_argument, &restart, 1 },
 
-static const struct test_t
+static struct test_t
 {
   const char *name;
   const char *value;
@@ -284,6 +289,29 @@ static const struct test_t
     0,
     0,
   },
+  /* Also check for repeated tunables with a count larger than the total number
+     of tunables.  */
+  {
+    "GLIBC_TUNABLES",
+    NULL,
+    2,
+    0,
+    0,
+  },
+  {
+    "GLIBC_TUNABLES",
+    NULL,
+    1,
+    0,
+    0,
+  },
+  {
+    "GLIBC_TUNABLES",
+    NULL,
+    0,
+    0,
+    0,
+  },
 };
 
 static int
@@ -327,6 +355,37 @@ do_test (int argc, char *argv[])
     spargv[i] = NULL;
   }
 
+  /* Create a tunable line with the duplicate values with a total number
+     larger than the different number of tunables.  */
+  {
+    enum { tunables_list_size = array_length (tunable_list) };
+    const char *value = "";
+    for (int i = 0; i < tunables_list_size; i++)
+      value = xasprintf ("%sglibc.malloc.check=2%c",
+			 value,
+			 i == (tunables_list_size - 1) ? '\0' : ':');
+    tests[33].value = value;
+  }
+  /* Same as before, but the last tunable values is differen than the
+     rest.  */
+  {
+    enum { tunables_list_size = array_length (tunable_list) };
+    const char *value = "";
+    for (int i = 0; i < tunables_list_size - 1; i++)
+      value = xasprintf ("%sglibc.malloc.check=2:", value);
+    value = xasprintf ("%sglibc.malloc.check=1", value);
+    tests[34].value = value;
+  }
+  /* Same as before, but with an invalid last entry.  */
+  {
+    enum { tunables_list_size = array_length (tunable_list) };
+    const char *value = "";
+    for (int i = 0; i < tunables_list_size - 1; i++)
+      value = xasprintf ("%sglibc.malloc.check=2:", value);
+    value = xasprintf ("%sglibc.malloc.check=1=1", value);
+    tests[35].value = value;
+  }
+
   for (int i = 0; i < array_length (tests); i++)
     {
       snprintf (nteststr, sizeof nteststr, "%d", i);
diff --git a/sysdeps/aarch64/multiarch/memset_generic.S b/sysdeps/aarch64/multiarch/memset_generic.S
index 81748bdbce..e125a5ed85 100644
--- a/sysdeps/aarch64/multiarch/memset_generic.S
+++ b/sysdeps/aarch64/multiarch/memset_generic.S
@@ -33,3 +33,7 @@
 #endif
 
 #include <../memset.S>
+
+#if IS_IN (rtld)
+strong_alias (memset, __memset_generic)
+#endif
diff --git a/sysdeps/sparc/sparc64/rtld-memset.c b/sysdeps/sparc/sparc64/rtld-memset.c
index 55f3835790..a19202a620 100644
--- a/sysdeps/sparc/sparc64/rtld-memset.c
+++ b/sysdeps/sparc/sparc64/rtld-memset.c
@@ -1 +1,4 @@
 #include <string/memset.c>
+#if IS_IN(rtld)
+strong_alias (memset, __memset_ultra1)
+#endif
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/4] elf: Remove glibc.rtld.enable_secure check from parse_tunables_string
  2024-05-06 16:18 [PATCH v3 0/4] More tunable fixes Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 1/4] elf: Only process multiple tunable once (BZ 31686) Adhemerval Zanella
@ 2024-05-06 16:18 ` Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 3/4] support: Add envp argument to support_capture_subprogram Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables Adhemerval Zanella
  3 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2024-05-06 16:18 UTC (permalink / raw)
  To: libc-alpha; +Cc: Joe Simmons-Talbott, Siddhesh Poyarekar

And move it to parse_tunables.  It avoids a string comparison for
each tunable.

Checked on aarch64-linux-gnu and x86_64-linux-gnu.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 elf/dl-tunables.c | 58 +++++++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 1db80e0f92..63cf8c7ab5 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -119,6 +119,17 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
   cur->initialized = true;
 }
 
+static bool
+tunable_parse_num (const char *strval, size_t len, tunable_num_t *val)
+{
+  char *endptr = NULL;
+  uint64_t numval = _dl_strtoul (strval, &endptr);
+  if (endptr != strval + len)
+    return false;
+  *val = (tunable_num_t) numval;
+  return true;
+}
+
 /* Validate range of the input value and initialize the tunable CUR if it looks
    good.  */
 static bool
@@ -128,11 +139,8 @@ tunable_initialize (tunable_t *cur, const char *strval, size_t len)
 
   if (cur->type.type_code != TUNABLE_TYPE_STRING)
     {
-      char *endptr = NULL;
-      uint64_t numval = _dl_strtoul (strval, &endptr);
-      if (endptr != strval + len)
+      if (!tunable_parse_num (strval, len, &val.numval))
 	return false;
-      val.numval = (tunable_num_t) numval;
     }
   else
     val.strval = (struct tunable_str_t) { strval, len };
@@ -223,17 +231,6 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
 	  if (tunable_is_name (cur->name, name))
 	    {
 	      tunables[i] = (struct tunable_toset_t) { cur, value, p - value };
-
-	      /* Ignore tunables if enable_secure is set */
-	      if (tunable_is_name ("glibc.rtld.enable_secure", name))
-		{
-                  tunable_num_t val = (tunable_num_t) _dl_strtoul (value, NULL);
-		  if (val == 1)
-		    {
-		      __libc_enable_secure = 1;
-		      return 0;
-		    }
-		}
 	      break;
 	    }
 	}
@@ -242,6 +239,16 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
   return ntunables;
 }
 
+static void
+parse_tunable_print_error (const struct tunable_toset_t *toset)
+{
+  _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' "
+		    "for option `%s': ignored.\n",
+		    (int) toset->len,
+		    toset->value,
+		    toset->t->name);
+}
+
 static void
 parse_tunables (const char *valstring)
 {
@@ -253,6 +260,21 @@ parse_tunables (const char *valstring)
       return;
     }
 
+  /* Ignore tunables if enable_secure is set */
+  struct tunable_toset_t *tsec =
+    &tunables[TUNABLE_ENUM_NAME(glibc, rtld, enable_secure)];
+  if (tsec->t != NULL)
+    {
+      tunable_num_t val;
+      if (!tunable_parse_num (tsec->value, tsec->len, &val))
+        parse_tunable_print_error (tsec);
+      else if (val == 1)
+	{
+	  __libc_enable_secure = 1;
+	  return;
+	}
+    }
+
   for (int i = 0; i < tunables_list_size; i++)
     {
       if (tunables[i].t == NULL)
@@ -260,11 +282,7 @@ parse_tunables (const char *valstring)
 
       if (!tunable_initialize (tunables[i].t, tunables[i].value,
 			       tunables[i].len))
-	_dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' "
-			  "for option `%s': ignored.\n",
-			  (int) tunables[i].len,
-			  tunables[i].value,
-			  tunables[i].t->name);
+	parse_tunable_print_error (&tunables[i]);
     }
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 3/4] support: Add envp argument to support_capture_subprogram
  2024-05-06 16:18 [PATCH v3 0/4] More tunable fixes Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 1/4] elf: Only process multiple tunable once (BZ 31686) Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 2/4] elf: Remove glibc.rtld.enable_secure check from parse_tunables_string Adhemerval Zanella
@ 2024-05-06 16:18 ` Adhemerval Zanella
  2024-05-06 16:18 ` [PATCH v3 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables Adhemerval Zanella
  3 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2024-05-06 16:18 UTC (permalink / raw)
  To: libc-alpha; +Cc: Joe Simmons-Talbott, Siddhesh Poyarekar

So tests can specify a list of environment variables.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 elf/tst-audit18.c                        | 2 +-
 elf/tst-audit19b.c                       | 2 +-
 elf/tst-audit22.c                        | 2 +-
 elf/tst-audit23.c                        | 2 +-
 elf/tst-audit25a.c                       | 4 ++--
 elf/tst-audit25b.c                       | 4 ++--
 elf/tst-glibc-hwcaps-2-cache.c           | 2 +-
 elf/tst-rtld-run-static.c                | 4 ++--
 elf/tst-tunables-enable_secure.c         | 2 +-
 elf/tst-tunables.c                       | 2 +-
 support/capture_subprocess.h             | 9 +++++----
 support/subprocess.h                     | 7 ++++---
 support/support_capture_subprocess.c     | 5 +++--
 support/support_subprocess.c             | 5 +++--
 support/tst-support_capture_subprocess.c | 2 +-
 sysdeps/x86/tst-hwcap-tunables.c         | 2 +-
 16 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/elf/tst-audit18.c b/elf/tst-audit18.c
index 841251dd70..cec93e269c 100644
--- a/elf/tst-audit18.c
+++ b/elf/tst-audit18.c
@@ -79,7 +79,7 @@ do_test (int argc, char *argv[])
 
   setenv ("LD_AUDIT", "tst-auditmod18.so", 0);
   struct support_capture_subprocess result
-    = support_capture_subprogram (spargv[0], spargv);
+    = support_capture_subprogram (spargv[0], spargv, NULL);
   support_capture_subprocess_check (&result, "tst-audit18", 0, sc_allow_stderr);
 
   struct
diff --git a/elf/tst-audit19b.c b/elf/tst-audit19b.c
index 70bfe4eadf..88d99a416b 100644
--- a/elf/tst-audit19b.c
+++ b/elf/tst-audit19b.c
@@ -69,7 +69,7 @@ do_test (int argc, char *argv[])
 
   setenv ("LD_AUDIT", "tst-auditmod18b.so", 0);
   struct support_capture_subprocess result
-    = support_capture_subprogram (spargv[0], spargv);
+    = support_capture_subprogram (spargv[0], spargv, NULL);
   support_capture_subprocess_check (&result, "tst-audit18b", 0, sc_allow_stderr);
 
   bool find_symbind = false;
diff --git a/elf/tst-audit22.c b/elf/tst-audit22.c
index 4e97be3be0..6aa18af948 100644
--- a/elf/tst-audit22.c
+++ b/elf/tst-audit22.c
@@ -83,7 +83,7 @@ do_test (int argc, char *argv[])
 
   setenv ("LD_AUDIT", "tst-auditmod22.so", 0);
   struct support_capture_subprocess result
-    = support_capture_subprogram (spargv[0], spargv);
+    = support_capture_subprogram (spargv[0], spargv, NULL);
   support_capture_subprocess_check (&result, "tst-audit22", 0, sc_allow_stderr);
 
   /* The respawned process should always print the vDSO address (otherwise it
diff --git a/elf/tst-audit23.c b/elf/tst-audit23.c
index 32e7c8b2a3..d2640fe8b2 100644
--- a/elf/tst-audit23.c
+++ b/elf/tst-audit23.c
@@ -82,7 +82,7 @@ do_test (int argc, char *argv[])
 
   setenv ("LD_AUDIT", "tst-auditmod23.so", 0);
   struct support_capture_subprocess result
-    = support_capture_subprogram (spargv[0], spargv);
+    = support_capture_subprogram (spargv[0], spargv, NULL);
   support_capture_subprocess_check (&result, "tst-audit22", 0, sc_allow_stderr);
 
   /* The expected la_objopen/la_objclose:
diff --git a/elf/tst-audit25a.c b/elf/tst-audit25a.c
index b209ee820f..cdd4f2ce2b 100644
--- a/elf/tst-audit25a.c
+++ b/elf/tst-audit25a.c
@@ -77,7 +77,7 @@ do_test (int argc, char *argv[])
 
   {
     struct support_capture_subprocess result
-      = support_capture_subprogram (spargv[0], spargv);
+      = support_capture_subprogram (spargv[0], spargv, NULL);
     support_capture_subprocess_check (&result, "tst-audit25a", 0,
 				      sc_allow_stderr);
 
@@ -102,7 +102,7 @@ do_test (int argc, char *argv[])
   {
     setenv ("LD_BIND_NOW", "1", 0);
     struct support_capture_subprocess result
-      = support_capture_subprogram (spargv[0], spargv);
+      = support_capture_subprogram (spargv[0], spargv, NULL);
     support_capture_subprocess_check (&result, "tst-audit25a", 0,
 				      sc_allow_stderr);
 
diff --git a/elf/tst-audit25b.c b/elf/tst-audit25b.c
index 9b8665d517..939f4d6188 100644
--- a/elf/tst-audit25b.c
+++ b/elf/tst-audit25b.c
@@ -76,7 +76,7 @@ do_test (int argc, char *argv[])
 
   {
     struct support_capture_subprocess result
-      = support_capture_subprogram (spargv[0], spargv);
+      = support_capture_subprogram (spargv[0], spargv, NULL);
     support_capture_subprocess_check (&result, "tst-audit25a", 0,
 				      sc_allow_stderr);
 
@@ -102,7 +102,7 @@ do_test (int argc, char *argv[])
   {
     setenv ("LD_BIND_NOW", "1", 0);
     struct support_capture_subprocess result
-      = support_capture_subprogram (spargv[0], spargv);
+      = support_capture_subprogram (spargv[0], spargv, NULL);
     support_capture_subprocess_check (&result, "tst-audit25a", 0,
 				      sc_allow_stderr);
 
diff --git a/elf/tst-glibc-hwcaps-2-cache.c b/elf/tst-glibc-hwcaps-2-cache.c
index 81ab44ff78..af91476cca 100644
--- a/elf/tst-glibc-hwcaps-2-cache.c
+++ b/elf/tst-glibc-hwcaps-2-cache.c
@@ -32,7 +32,7 @@ main (int argc, char **argv)
   /* Run ldconfig to populate the cache.  */
   char *command = xasprintf ("%s/ldconfig", support_install_rootsbindir);
   struct support_capture_subprocess result =
-    support_capture_subprogram (command,  &((char *) { NULL }));
+    support_capture_subprogram (command,  &((char *) { NULL }), NULL);
   support_capture_subprocess_check (&result, "ldconfig", 0, sc_allow_none);
   free (command);
 
diff --git a/elf/tst-rtld-run-static.c b/elf/tst-rtld-run-static.c
index b2650e85ff..f05c00eb7b 100644
--- a/elf/tst-rtld-run-static.c
+++ b/elf/tst-rtld-run-static.c
@@ -30,7 +30,7 @@ do_test (void)
   {
     char *argv[] = { (char *) "ld.so", ldconfig_path, (char *) "--help", NULL };
     struct support_capture_subprocess cap
-      = support_capture_subprogram (support_objdir_elf_ldso, argv);
+      = support_capture_subprogram (support_objdir_elf_ldso, argv, NULL);
     support_capture_subprocess_check (&cap, "no --argv0", 0, sc_allow_stdout);
     puts ("info: output without --argv0:");
     puts (cap.out.buffer);
@@ -46,7 +46,7 @@ do_test (void)
         ldconfig_path, (char *) "--help", NULL
       };
     struct support_capture_subprocess cap
-      = support_capture_subprogram (support_objdir_elf_ldso, argv);
+      = support_capture_subprogram (support_objdir_elf_ldso, argv, NULL);
     support_capture_subprocess_check (&cap, "with --argv0", 0, sc_allow_stdout);
     puts ("info: output with --argv0:");
     puts (cap.out.buffer);
diff --git a/elf/tst-tunables-enable_secure.c b/elf/tst-tunables-enable_secure.c
index e31e1f7faa..f5db1c84e9 100644
--- a/elf/tst-tunables-enable_secure.c
+++ b/elf/tst-tunables-enable_secure.c
@@ -113,7 +113,7 @@ do_test (int argc, char *argv[])
       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_subprogram (spargv[0], spargv, NULL);
       support_capture_subprocess_check (&result, "tst-tunables-enable_secure",
 		                        0, sc_allow_stderr);
       support_capture_subprocess_free (&result);
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
index dff34ed748..4884dd09f0 100644
--- a/elf/tst-tunables.c
+++ b/elf/tst-tunables.c
@@ -396,7 +396,7 @@ do_test (int argc, char *argv[])
 	      tests[i].value);
       setenv (tests[i].name, tests[i].value, 1);
       struct support_capture_subprocess result
-	= support_capture_subprogram (spargv[0], spargv);
+	= support_capture_subprogram (spargv[0], spargv, NULL);
       support_capture_subprocess_check (&result, "tst-tunables", 0,
 					sc_allow_stderr);
       support_capture_subprocess_free (&result);
diff --git a/support/capture_subprocess.h b/support/capture_subprocess.h
index 1ecbdfe4fc..93b7245d2a 100644
--- a/support/capture_subprocess.h
+++ b/support/capture_subprocess.h
@@ -35,11 +35,12 @@ struct support_capture_subprocess
 struct support_capture_subprocess support_capture_subprocess
   (void (*callback) (void *), void *closure);
 
-/* Issue FILE with ARGV arguments by using posix_spawn and capture standard
-   output, standard error, and the exit status.  The out.buffer and err.buffer
-   are handle as support_capture_subprocess.  */
+/* Issue FILE with ARGV arguments and ENVP environments by using posix_spawn
+   and capture standard output, standard error, and the exit status.  If
+   ENVP is NULL the current environment variable is used.  The out.buffer and
+   err.buffer are handle by support_capture_subprocess.  */
 struct support_capture_subprocess support_capture_subprogram
-  (const char *file, char *const argv[]);
+  (const char *file, char *const argv[], char *const envp[]);
 
 /* Copy the running program into a setgid binary and run it with CHILD_ID
    argument.  If execution is successful, return the exit status of the child
diff --git a/support/subprocess.h b/support/subprocess.h
index 8fbb895353..8274a2b22b 100644
--- a/support/subprocess.h
+++ b/support/subprocess.h
@@ -33,10 +33,11 @@ struct support_subprocess
 struct support_subprocess support_subprocess
   (void (*callback) (void *), void *closure);
 
-/* Issue FILE with ARGV arguments by using posix_spawn and return is PID, a
-   pipe redirected to STDOUT, and a pipe redirected to STDERR.  */
+/* Issue FILE with ARGV arguments and ENVP environments by using posix_spawn
+   and return is PID, a pipe redirected to STDOUT, and a pipe redirected to
+   STDERR.  If ENVP is NULL the current environment variable is used.  */
 struct support_subprocess support_subprogram
-  (const char *file, char *const argv[]);
+  (const char *file, char *const argv[], char *const envp[]);
 
 /* Invoke program FILE with ARGV arguments by using posix_spawn and wait for it
    to complete.  Return program exit status.  */
diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c
index ffced8a89f..53847194cb 100644
--- a/support/support_capture_subprocess.c
+++ b/support/support_capture_subprocess.c
@@ -93,13 +93,14 @@ support_capture_subprocess (void (*callback) (void *), void *closure)
 }
 
 struct support_capture_subprocess
-support_capture_subprogram (const char *file, char *const argv[])
+support_capture_subprogram (const char *file, char *const argv[],
+			    char *const envp[])
 {
   struct support_capture_subprocess result;
   xopen_memstream (&result.out);
   xopen_memstream (&result.err);
 
-  struct support_subprocess proc = support_subprogram (file, argv);
+  struct support_subprocess proc = support_subprogram (file, argv, envp);
 
   support_capture_poll (&result, &proc);
   return result;
diff --git a/support/support_subprocess.c b/support/support_subprocess.c
index a2fef394d4..b692a7f8b1 100644
--- a/support/support_subprocess.c
+++ b/support/support_subprocess.c
@@ -69,7 +69,7 @@ support_subprocess (void (*callback) (void *), void *closure)
 }
 
 struct support_subprocess
-support_subprogram (const char *file, char *const argv[])
+support_subprogram (const char *file, char *const argv[], char *const envp[])
 {
   struct support_subprocess result = support_subprocess_init ();
 
@@ -84,7 +84,8 @@ support_subprogram (const char *file, char *const argv[])
   xposix_spawn_file_actions_addclose (&fa, result.stdout_pipe[1]);
   xposix_spawn_file_actions_addclose (&fa, result.stderr_pipe[1]);
 
-  result.pid = xposix_spawn (file, &fa, NULL, argv, environ);
+  result.pid = xposix_spawn (file, &fa, NULL, argv,
+			     envp == NULL ? environ : envp);
 
   xclose (result.stdout_pipe[1]);
   xclose (result.stderr_pipe[1]);
diff --git a/support/tst-support_capture_subprocess.c b/support/tst-support_capture_subprocess.c
index 8145548982..756fb75d19 100644
--- a/support/tst-support_capture_subprocess.c
+++ b/support/tst-support_capture_subprocess.c
@@ -238,7 +238,7 @@ do_subprogram (const struct test *test)
   args[argc]   = NULL;
   TEST_VERIFY (argc < argv_size);
 
-  return support_capture_subprogram (args[0], args);
+  return support_capture_subprogram (args[0], args, NULL);
 }
 
 enum test_type
diff --git a/sysdeps/x86/tst-hwcap-tunables.c b/sysdeps/x86/tst-hwcap-tunables.c
index f6a65b88de..8589a9fd66 100644
--- a/sysdeps/x86/tst-hwcap-tunables.c
+++ b/sysdeps/x86/tst-hwcap-tunables.c
@@ -133,7 +133,7 @@ do_test (int argc, char *argv[])
       setenv ("GLIBC_TUNABLES", tunable, 1);
 
       struct support_capture_subprocess result
-	= support_capture_subprogram (spargv[0], spargv);
+	= support_capture_subprogram (spargv[0], spargv, NULL);
       support_capture_subprocess_check (&result, "tst-tunables", 0,
 					sc_allow_stderr);
       support_capture_subprocess_free (&result);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables
  2024-05-06 16:18 [PATCH v3 0/4] More tunable fixes Adhemerval Zanella
                   ` (2 preceding siblings ...)
  2024-05-06 16:18 ` [PATCH v3 3/4] support: Add envp argument to support_capture_subprogram Adhemerval Zanella
@ 2024-05-06 16:18 ` Adhemerval Zanella
  2024-05-07 14:33   ` Siddhesh Poyarekar
  3 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella @ 2024-05-06 16:18 UTC (permalink / raw)
  To: libc-alpha; +Cc: Joe Simmons-Talbott, Siddhesh Poyarekar

Tunable with environment variables aliases are also ignored if
glibc.rtld.enable_secure is enabled.  The tunable parsing is also
optimized a bit, where the loop that checks each environment variable
only checks for the tunables with aliases instead of all tables.

Checked on aarch64-linux-gnu and x86_64-linux-gnu.
---
 elf/dl-tunables.c                |  36 ++++++--
 elf/tst-tunables-enable_secure.c | 136 +++++++++++++++++++++++++++----
 scripts/gen-tunables.awk         |  16 +++-
 3 files changed, 164 insertions(+), 24 deletions(-)

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 63cf8c7ab5..147cc4cf23 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -300,6 +300,9 @@ __tunables_init (char **envp)
   if (__libc_enable_secure)
     return;
 
+  enum { tunable_num_env_alias = array_length (tunable_env_alias_list) };
+  struct tunable_toset_t tunables_env_alias[tunable_num_env_alias] = { 0 };
+
   while ((envp = get_next_env (envp, &envname, &envval, &prev_envp)) != NULL)
     {
       /* The environment variable is allocated on the stack by the kernel, so
@@ -311,29 +314,44 @@ __tunables_init (char **envp)
 	  continue;
 	}
 
-      for (int i = 0; i < tunables_list_size; i++)
+      for (int i = 0; i < tunable_num_env_alias; i++)
 	{
-	  tunable_t *cur = &tunable_list[i];
+	  tunable_t *cur = &tunable_list[tunable_env_alias_list[i]];
+	  const char *name = cur->env_alias;
 
-	  /* Skip over tunables that have either been set already or should be
-	     skipped.  */
-	  if (cur->initialized || cur->env_alias[0] == '\0')
+	  if (name[0] == '\0')
 	    continue;
 
-	  const char *name = cur->env_alias;
-
-	  /* We have a match.  Initialize and move on to the next line.  */
 	  if (tunable_is_name (name, envname))
 	    {
 	      size_t envvallen = 0;
 	      /* The environment variable is always null-terminated.  */
 	      for (const char *p = envval; *p != '\0'; p++, envvallen++);
 
-	      tunable_initialize (cur, envval, envvallen);
+	      tunables_env_alias[i] =
+		(struct tunable_toset_t) { cur, envval, envvallen };
 	      break;
 	    }
 	}
     }
+
+  /* Check if glibc.rtld.enable_secure was set and skip over the environment
+     variables aliases.  */
+  if (__libc_enable_secure)
+    return;
+
+  for (int i = 0; i < tunable_num_env_alias; i++)
+    {
+      /* Skip over tunables that have either been set or already initialized.  */
+      if (tunables_env_alias[i].t == NULL
+	  || tunables_env_alias[i].t->initialized)
+	continue;
+
+      if (!tunable_initialize (tunables_env_alias[i].t,
+			       tunables_env_alias[i].value,
+			       tunables_env_alias[i].len))
+	parse_tunable_print_error (&tunables_env_alias[i]);
+    }
 }
 
 void
diff --git a/elf/tst-tunables-enable_secure.c b/elf/tst-tunables-enable_secure.c
index f5db1c84e9..99a065e8db 100644
--- a/elf/tst-tunables-enable_secure.c
+++ b/elf/tst-tunables-enable_secure.c
@@ -17,6 +17,10 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <array_length.h>
+/* The test uses the tunable_env_alias_list size, which is only exported for
+   ld.so.  This will result in a copy of tunable_list and
+   tunable_env_alias_list, which is ununsed by the test itself.  */
+#define TUNABLES_INTERNAL 1
 #include <dl-tunables.h>
 #include <getopt.h>
 #include <intprops.h>
@@ -34,6 +38,8 @@ static int restart;
 static const struct test_t
 {
   const char *env;
+  const char *extraenv;
+  bool check_multiple;
   int32_t expected_malloc_check;
   int32_t expected_enable_secure;
 } tests[] =
@@ -41,39 +47,124 @@ static const struct test_t
   /* Expected tunable format.  */
   /* Tunables should be ignored if enable_secure is set. */
   {
-    "glibc.malloc.check=2:glibc.rtld.enable_secure=1",
+    "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1",
+    NULL,
+    false,
     0,
     1,
   },
   /* Tunables should be ignored if enable_secure is set. */
   {
-    "glibc.rtld.enable_secure=1:glibc.malloc.check=2",
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
+    NULL,
+    false,
     0,
     1,
   },
   /* Tunables should be set if enable_secure is unset. */
   {
-    "glibc.rtld.enable_secure=0:glibc.malloc.check=2",
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2",
+    NULL,
+    false,
     2,
     0,
   },
+  /* Tunables should be ignored if enable_secure is set. */
+  {
+    "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1",
+    "MALLOC_CHECK_=2",
+    false,
+    0,
+    1,
+  },
+  /* Same as before, but with enviroment alias prior GLIBC_TUNABLES.  */
+  {
+    "MALLOC_CHECK_=2",
+    "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1",
+    false,
+    0,
+    1,
+  },
+  /* Tunables should be ignored if enable_secure is set. */
+  {
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
+    "MALLOC_CHECK_=2",
+    false,
+    0,
+    1,
+  },
+  {
+    "MALLOC_CHECK_=2",
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
+    false,
+    0,
+    1,
+  },
+  /* Tunables should be set if enable_secure is unset. */
+  {
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2",
+    /* Tunable have precedence over the environment variable.  */
+    "MALLOC_CHECK_=1",
+    false,
+    2,
+    0,
+  },
+  {
+    "MALLOC_CHECK_=1",
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2",
+    /* Tunable have precedence over the environment variable.  */
+    false,
+    2,
+    0,
+  },
+  /* Tunables should be set if enable_secure is unset. */
+  {
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0",
+    /* Tunable have precedence over the environment variable.  */
+    "MALLOC_CHECK_=1",
+    false,
+    1,
+    0,
+  },
+  /* Tunables should be set if enable_secure is unset. */
+  {
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0",
+    /* Tunable have precedence over the environment variable.  */
+    "MALLOC_CHECK_=1",
+    false,
+    1,
+    0,
+  },
+  /* Check with tunables environment variable alias set multiple times.  */
+  {
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
+    "MALLOC_CHECK_=2",
+    true,
+    0,
+    1,
+  },
+  /* Tunables should be set if enable_secure is unset. */
+  {
+    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0",
+    /* Tunable have precedence over the environment variable.  */
+    "MALLOC_CHECK_=1",
+    true,
+    1,
+    0,
+  },
 };
 
 static int
 handle_restart (int i)
 {
   if (tests[i].expected_enable_secure == 1)
-    {
-      TEST_COMPARE (1, __libc_enable_secure);
-    }
+    TEST_COMPARE (1, __libc_enable_secure);
   else
-    {
-      TEST_COMPARE (tests[i].expected_malloc_check,
-		    TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL));
-      TEST_COMPARE (tests[i].expected_enable_secure,
-		    TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t,
-		    NULL));
-    }
+    TEST_COMPARE (tests[i].expected_enable_secure,
+		  TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t,
+				     NULL));
+  TEST_COMPARE (tests[i].expected_malloc_check,
+		TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL));
   return 0;
 }
 
@@ -106,14 +197,31 @@ do_test (int argc, char *argv[])
     spargv[i] = NULL;
   }
 
+  enum { tunable_num_env_alias = array_length (tunable_env_alias_list) };
+
   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);
+
+      char *envp[2 + tunable_num_env_alias + 1] =
+      {
+	(char *) tests[i].env,
+	(char *) tests[i].extraenv,
+	NULL,
+      };
+      if (tests[i].check_multiple)
+	{
+	  int j;
+	  for (j=0; j < tunable_num_env_alias; j++)
+	    envp[j + 2] = (char *) tests[i].extraenv;
+	  envp[j + 2] = NULL;
+	}
+
       struct support_capture_subprocess result
-	= support_capture_subprogram (spargv[0], spargv, NULL);
+	= support_capture_subprogram (spargv[0], spargv, envp);
       support_capture_subprocess_check (&result, "tst-tunables-enable_secure",
 		                        0, sc_allow_stderr);
       support_capture_subprocess_free (&result);
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index 9f5336381e..fc3b41376f 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -156,7 +156,7 @@ END {
   print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1)
   print "# include \"dl-tunable-types.h\""
   # Finally, the tunable list.
-  print "static tunable_t tunable_list[] attribute_relro = {"
+  print "static tunable_t tunable_list[] attribute_relro __attribute_used__ = {"
   for (tnm in types) {
     split (tnm, indices, SUBSEP);
     t = indices[1];
@@ -168,5 +168,19 @@ END {
 	    default_val[t,n,m], env_alias[t,n,m]);
   }
   print "};"
+
+  # Map of tunable with environment variables aliases used during parsing.  */
+  print "\nstatic const tunable_id_t tunable_env_alias_list[] ="
+  printf "{\n"
+  for (tnm in types) {
+    split (tnm, indices, SUBSEP);
+    t = indices[1];
+    n = indices[2];
+    m = indices[3];
+    if (env_alias[t,n,m] != "{0}") {
+      printf ("  TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m);
+    }
+  }
+  printf "};\n"
   print "#endif"
 }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables
  2024-05-06 16:18 ` [PATCH v3 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables Adhemerval Zanella
@ 2024-05-07 14:33   ` Siddhesh Poyarekar
  0 siblings, 0 replies; 6+ messages in thread
From: Siddhesh Poyarekar @ 2024-05-07 14:33 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha; +Cc: Joe Simmons-Talbott

On 2024-05-06 12:18, Adhemerval Zanella wrote:
> Tunable with environment variables aliases are also ignored if
> glibc.rtld.enable_secure is enabled.  The tunable parsing is also
> optimized a bit, where the loop that checks each environment variable
> only checks for the tunables with aliases instead of all tables.
> 
> Checked on aarch64-linux-gnu and x86_64-linux-gnu.
> ---

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>


>   elf/dl-tunables.c                |  36 ++++++--
>   elf/tst-tunables-enable_secure.c | 136 +++++++++++++++++++++++++++----
>   scripts/gen-tunables.awk         |  16 +++-
>   3 files changed, 164 insertions(+), 24 deletions(-)
> 
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 63cf8c7ab5..147cc4cf23 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -300,6 +300,9 @@ __tunables_init (char **envp)
>     if (__libc_enable_secure)
>       return;
>   
> +  enum { tunable_num_env_alias = array_length (tunable_env_alias_list) };
> +  struct tunable_toset_t tunables_env_alias[tunable_num_env_alias] = { 0 };
> +
>     while ((envp = get_next_env (envp, &envname, &envval, &prev_envp)) != NULL)
>       {
>         /* The environment variable is allocated on the stack by the kernel, so
> @@ -311,29 +314,44 @@ __tunables_init (char **envp)
>   	  continue;
>   	}
>   
> -      for (int i = 0; i < tunables_list_size; i++)
> +      for (int i = 0; i < tunable_num_env_alias; i++)
>   	{
> -	  tunable_t *cur = &tunable_list[i];
> +	  tunable_t *cur = &tunable_list[tunable_env_alias_list[i]];
> +	  const char *name = cur->env_alias;
>   
> -	  /* Skip over tunables that have either been set already or should be
> -	     skipped.  */
> -	  if (cur->initialized || cur->env_alias[0] == '\0')
> +	  if (name[0] == '\0')
>   	    continue;
>   
> -	  const char *name = cur->env_alias;
> -
> -	  /* We have a match.  Initialize and move on to the next line.  */
>   	  if (tunable_is_name (name, envname))
>   	    {
>   	      size_t envvallen = 0;
>   	      /* The environment variable is always null-terminated.  */
>   	      for (const char *p = envval; *p != '\0'; p++, envvallen++);
>   
> -	      tunable_initialize (cur, envval, envvallen);
> +	      tunables_env_alias[i] =
> +		(struct tunable_toset_t) { cur, envval, envvallen };
>   	      break;
>   	    }
>   	}
>       }
> +
> +  /* Check if glibc.rtld.enable_secure was set and skip over the environment
> +     variables aliases.  */
> +  if (__libc_enable_secure)
> +    return;
> +
> +  for (int i = 0; i < tunable_num_env_alias; i++)
> +    {
> +      /* Skip over tunables that have either been set or already initialized.  */
> +      if (tunables_env_alias[i].t == NULL
> +	  || tunables_env_alias[i].t->initialized)
> +	continue;
> +
> +      if (!tunable_initialize (tunables_env_alias[i].t,
> +			       tunables_env_alias[i].value,
> +			       tunables_env_alias[i].len))
> +	parse_tunable_print_error (&tunables_env_alias[i]);
> +    }
>   }
>   
>   void
> diff --git a/elf/tst-tunables-enable_secure.c b/elf/tst-tunables-enable_secure.c
> index f5db1c84e9..99a065e8db 100644
> --- a/elf/tst-tunables-enable_secure.c
> +++ b/elf/tst-tunables-enable_secure.c
> @@ -17,6 +17,10 @@
>      <https://www.gnu.org/licenses/>.  */
>   
>   #include <array_length.h>
> +/* The test uses the tunable_env_alias_list size, which is only exported for
> +   ld.so.  This will result in a copy of tunable_list and
> +   tunable_env_alias_list, which is ununsed by the test itself.  */
> +#define TUNABLES_INTERNAL 1
>   #include <dl-tunables.h>
>   #include <getopt.h>
>   #include <intprops.h>
> @@ -34,6 +38,8 @@ static int restart;
>   static const struct test_t
>   {
>     const char *env;
> +  const char *extraenv;
> +  bool check_multiple;
>     int32_t expected_malloc_check;
>     int32_t expected_enable_secure;
>   } tests[] =
> @@ -41,39 +47,124 @@ static const struct test_t
>     /* Expected tunable format.  */
>     /* Tunables should be ignored if enable_secure is set. */
>     {
> -    "glibc.malloc.check=2:glibc.rtld.enable_secure=1",
> +    "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1",
> +    NULL,
> +    false,
>       0,
>       1,
>     },
>     /* Tunables should be ignored if enable_secure is set. */
>     {
> -    "glibc.rtld.enable_secure=1:glibc.malloc.check=2",
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
> +    NULL,
> +    false,
>       0,
>       1,
>     },
>     /* Tunables should be set if enable_secure is unset. */
>     {
> -    "glibc.rtld.enable_secure=0:glibc.malloc.check=2",
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2",
> +    NULL,
> +    false,
>       2,
>       0,
>     },
> +  /* Tunables should be ignored if enable_secure is set. */
> +  {
> +    "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1",
> +    "MALLOC_CHECK_=2",
> +    false,
> +    0,
> +    1,
> +  },
> +  /* Same as before, but with enviroment alias prior GLIBC_TUNABLES.  */
> +  {
> +    "MALLOC_CHECK_=2",
> +    "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1",
> +    false,
> +    0,
> +    1,
> +  },
> +  /* Tunables should be ignored if enable_secure is set. */
> +  {
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
> +    "MALLOC_CHECK_=2",
> +    false,
> +    0,
> +    1,
> +  },
> +  {
> +    "MALLOC_CHECK_=2",
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
> +    false,
> +    0,
> +    1,
> +  },
> +  /* Tunables should be set if enable_secure is unset. */
> +  {
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2",
> +    /* Tunable have precedence over the environment variable.  */
> +    "MALLOC_CHECK_=1",
> +    false,
> +    2,
> +    0,
> +  },
> +  {
> +    "MALLOC_CHECK_=1",
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2",
> +    /* Tunable have precedence over the environment variable.  */
> +    false,
> +    2,
> +    0,
> +  },
> +  /* Tunables should be set if enable_secure is unset. */
> +  {
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0",
> +    /* Tunable have precedence over the environment variable.  */
> +    "MALLOC_CHECK_=1",
> +    false,
> +    1,
> +    0,
> +  },
> +  /* Tunables should be set if enable_secure is unset. */
> +  {
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0",
> +    /* Tunable have precedence over the environment variable.  */
> +    "MALLOC_CHECK_=1",
> +    false,
> +    1,
> +    0,
> +  },
> +  /* Check with tunables environment variable alias set multiple times.  */
> +  {
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2",
> +    "MALLOC_CHECK_=2",
> +    true,
> +    0,
> +    1,
> +  },
> +  /* Tunables should be set if enable_secure is unset. */
> +  {
> +    "GLIBC_TUNABLES=glibc.rtld.enable_secure=0",
> +    /* Tunable have precedence over the environment variable.  */
> +    "MALLOC_CHECK_=1",
> +    true,
> +    1,
> +    0,
> +  },
>   };
>   
>   static int
>   handle_restart (int i)
>   {
>     if (tests[i].expected_enable_secure == 1)
> -    {
> -      TEST_COMPARE (1, __libc_enable_secure);
> -    }
> +    TEST_COMPARE (1, __libc_enable_secure);
>     else
> -    {
> -      TEST_COMPARE (tests[i].expected_malloc_check,
> -		    TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL));
> -      TEST_COMPARE (tests[i].expected_enable_secure,
> -		    TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t,
> -		    NULL));
> -    }
> +    TEST_COMPARE (tests[i].expected_enable_secure,
> +		  TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t,
> +				     NULL));
> +  TEST_COMPARE (tests[i].expected_malloc_check,
> +		TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL));
>     return 0;
>   }
>   
> @@ -106,14 +197,31 @@ do_test (int argc, char *argv[])
>       spargv[i] = NULL;
>     }
>   
> +  enum { tunable_num_env_alias = array_length (tunable_env_alias_list) };
> +
>     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);
> +
> +      char *envp[2 + tunable_num_env_alias + 1] =
> +      {
> +	(char *) tests[i].env,
> +	(char *) tests[i].extraenv,
> +	NULL,
> +      };
> +      if (tests[i].check_multiple)
> +	{
> +	  int j;
> +	  for (j=0; j < tunable_num_env_alias; j++)
> +	    envp[j + 2] = (char *) tests[i].extraenv;
> +	  envp[j + 2] = NULL;
> +	}
> +
>         struct support_capture_subprocess result
> -	= support_capture_subprogram (spargv[0], spargv, NULL);
> +	= support_capture_subprogram (spargv[0], spargv, envp);
>         support_capture_subprocess_check (&result, "tst-tunables-enable_secure",
>   		                        0, sc_allow_stderr);
>         support_capture_subprocess_free (&result);
> diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
> index 9f5336381e..fc3b41376f 100644
> --- a/scripts/gen-tunables.awk
> +++ b/scripts/gen-tunables.awk
> @@ -156,7 +156,7 @@ END {
>     print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1)
>     print "# include \"dl-tunable-types.h\""
>     # Finally, the tunable list.
> -  print "static tunable_t tunable_list[] attribute_relro = {"
> +  print "static tunable_t tunable_list[] attribute_relro __attribute_used__ = {"
>     for (tnm in types) {
>       split (tnm, indices, SUBSEP);
>       t = indices[1];
> @@ -168,5 +168,19 @@ END {
>   	    default_val[t,n,m], env_alias[t,n,m]);
>     }
>     print "};"
> +
> +  # Map of tunable with environment variables aliases used during parsing.  */
> +  print "\nstatic const tunable_id_t tunable_env_alias_list[] ="
> +  printf "{\n"
> +  for (tnm in types) {
> +    split (tnm, indices, SUBSEP);
> +    t = indices[1];
> +    n = indices[2];
> +    m = indices[3];
> +    if (env_alias[t,n,m] != "{0}") {
> +      printf ("  TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m);
> +    }
> +  }
> +  printf "};\n"
>     print "#endif"
>   }

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-07 14:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-06 16:18 [PATCH v3 0/4] More tunable fixes Adhemerval Zanella
2024-05-06 16:18 ` [PATCH v3 1/4] elf: Only process multiple tunable once (BZ 31686) Adhemerval Zanella
2024-05-06 16:18 ` [PATCH v3 2/4] elf: Remove glibc.rtld.enable_secure check from parse_tunables_string Adhemerval Zanella
2024-05-06 16:18 ` [PATCH v3 3/4] support: Add envp argument to support_capture_subprogram Adhemerval Zanella
2024-05-06 16:18 ` [PATCH v3 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables Adhemerval Zanella
2024-05-07 14:33   ` Siddhesh Poyarekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).