diff mbox series

[v3,04/19] elf: Add all malloc tunable to unsecvars

Message ID 20231106202552.3404059-5-adhemerval.zanella@linaro.org
State Accepted
Commit 11f7e3dd8fed66e0b8740af440cd3151e55a466f
Headers show
Series Improve loader environment variable handling | expand

Commit Message

Adhemerval Zanella Netto Nov. 6, 2023, 8:25 p.m. UTC
Some environment variables allow alteration of allocator behavior
across setuid boundaries, where a setuid program may ignore the
tunable, but its non-setuid child can read it and adjust the memory
allocator behavior accordingly.

Most library behavior tunings is limited to the current process and does
not bleed in scope; so it is unclear how pratical this misfeature is.
If behavior change across privilege boundaries is desirable, it would be
better done with a wrapper program around the non-setuid child that sets
these envvars, instead of using the setuid process as the messenger.

The patch as fixes tst-env-setuid, where it fail if any unsecvars is
set.  It also adds a dynamic test, although it requires
--enable-hardcoded-path-in-tests so kernel correctly sets the setuid
bit (using the loader command directly would require to set the
setuid bit on the loader itself, which is not a usual deployment).

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

Checked on x86_64-linux-gnu.
Reviewed-by: DJ Delorie <dj@redhat.com>
---
 elf/Makefile                |   8 +--
 elf/tst-env-setuid-static.c |   1 +
 elf/tst-env-setuid.c        | 128 +++++++++++++++++++++---------------
 sysdeps/generic/unsecvars.h |   7 ++
 4 files changed, 86 insertions(+), 58 deletions(-)
 create mode 100644 elf/tst-env-setuid-static.c
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 5f24a00a92..52981c19d0 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -262,7 +262,7 @@  tests-static-normal := \
   tst-array5-static \
   tst-dl-iter-static \
   tst-dst-static \
-  tst-env-setuid \
+  tst-env-setuid-static \
   tst-getauxval-static \
   tst-linkall-static \
   tst-single_threaded-pthread-static \
@@ -305,6 +305,7 @@  tests := \
   tst-array5 \
   tst-auxv \
   tst-dl-hash \
+  tst-env-setuid \
   tst-leaks1 \
   tst-stringtable \
   tst-tls9 \
@@ -2429,9 +2430,6 @@  $(objpfx)tst-nodelete-dlclose: $(objpfx)tst-nodelete-dlclose-dso.so
 $(objpfx)tst-nodelete-dlclose.out: $(objpfx)tst-nodelete-dlclose-dso.so \
 				   $(objpfx)tst-nodelete-dlclose-plugin.so
 
-tst-env-setuid-ENV = MALLOC_CHECK_=2 MALLOC_MMAP_THRESHOLD_=4096 \
-		     LD_HWCAP_MASK=0x1
-
 $(objpfx)tst-debug1.out: $(objpfx)tst-debug1mod1.so
 
 $(objpfx)tst-debug1mod1.so: $(objpfx)testobj1.so
@@ -2983,3 +2981,5 @@  LDFLAGS-tst-dlclose-lazy-mod1.so = -Wl,-z,lazy,--no-as-needed
 $(objpfx)tst-dlclose-lazy-mod1.so: $(objpfx)tst-dlclose-lazy-mod2.so
 $(objpfx)tst-dlclose-lazy.out: \
   $(objpfx)tst-dlclose-lazy-mod1.so $(objpfx)tst-dlclose-lazy-mod2.so
+
+tst-env-setuid-ARGS = -- $(host-test-program-cmd)
diff --git a/elf/tst-env-setuid-static.c b/elf/tst-env-setuid-static.c
new file mode 100644
index 0000000000..0d88ae88b9
--- /dev/null
+++ b/elf/tst-env-setuid-static.c
@@ -0,0 +1 @@ 
+#include "tst-env-setuid.c"
diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
index 032ab44be2..ba295a6a14 100644
--- a/elf/tst-env-setuid.c
+++ b/elf/tst-env-setuid.c
@@ -15,18 +15,14 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-/* Verify that tunables correctly filter out unsafe environment variables like
-   MALLOC_CHECK_ and MALLOC_MMAP_THRESHOLD_ but also retain
-   MALLOC_MMAP_THRESHOLD_ in an unprivileged child.  */
+/* Verify that correctly filter out unsafe environment variables defined
+   in unsecvars.h.  */
 
-#include <errno.h>
-#include <fcntl.h>
-#include <stdlib.h>
-#include <stdint.h>
+#include <array_length.h>
+#include <gnu/lib-names.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
-#include <sys/stat.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include <support/check.h>
@@ -36,61 +32,72 @@ 
 
 static char SETGID_CHILD[] = "setgid-child";
 
-#ifndef test_child
-static int
-test_child (void)
-{
-  if (getenv ("MALLOC_CHECK_") != NULL)
-    {
-      printf ("MALLOC_CHECK_ is still set\n");
-      return 1;
-    }
-
-  if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
-    {
-      printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
-      return 1;
-    }
+#define FILTERED_VALUE   "some-filtered-value"
+#define UNFILTERED_VALUE "some-unfiltered-value"
 
-  if (getenv ("LD_HWCAP_MASK") != NULL)
-    {
-      printf ("LD_HWCAP_MASK still set\n");
-      return 1;
-    }
+struct envvar_t
+{
+  const char *env;
+  const char *value;
+};
 
-  return 0;
-}
-#endif
+/* That is not an extensible list of all filtered out environment
+   variables.  */
+static const struct envvar_t filtered_envvars[] =
+{
+  { "GLIBC_TUNABLES",          FILTERED_VALUE },
+  { "LD_AUDIT",                FILTERED_VALUE },
+  { "LD_HWCAP_MASK",           FILTERED_VALUE },
+  { "LD_LIBRARY_PATH",         FILTERED_VALUE },
+  { "LD_PRELOAD",              FILTERED_VALUE },
+  { "LD_PROFILE",              FILTERED_VALUE },
+  { "MALLOC_ARENA_MAX",        FILTERED_VALUE },
+  { "MALLOC_PERTURB_",         FILTERED_VALUE },
+  { "MALLOC_TRACE",            FILTERED_VALUE },
+  { "MALLOC_TRIM_THRESHOLD_",  FILTERED_VALUE },
+  { "RES_OPTIONS",             FILTERED_VALUE },
+};
+
+static const struct envvar_t unfiltered_envvars[] =
+{
+  { "LD_BIND_NOW",             "0" },
+  { "LD_BIND_NOT",             "1" },
+  /* Non longer supported option.  */
+  { "LD_ASSUME_KERNEL",        UNFILTERED_VALUE },
+};
 
-#ifndef test_parent
 static int
-test_parent (void)
+test_child (void)
 {
-  if (getenv ("MALLOC_CHECK_") == NULL)
-    {
-      printf ("MALLOC_CHECK_ lost\n");
-      return 1;
-    }
+  int ret = 0;
 
-  if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
+  for (const struct envvar_t *e = filtered_envvars;
+       e != array_end (filtered_envvars);
+       e++)
     {
-      printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
-      return 1;
+      const char *env = getenv (e->env);
+      ret |= env != NULL;
     }
 
-  if (getenv ("LD_HWCAP_MASK") == NULL)
+  for (const struct envvar_t *e = unfiltered_envvars;
+       e != array_end (unfiltered_envvars);
+       e++)
     {
-      printf ("LD_HWCAP_MASK lost\n");
-      return 1;
+      const char *env = getenv (e->env);
+      ret |= !(env != NULL && strcmp (env, e->value) == 0);
     }
 
-  return 0;
+  return ret;
 }
-#endif
 
 static int
 do_test (int argc, char **argv)
 {
+  /* For dynamic loader, the test requires --enable-hardcoded-path-in-tests so
+     the kernel sets the AT_SECURE on process initialization.  */
+  if (argc >= 2 && strstr (argv[1], LD_SO) != 0)
+    FAIL_UNSUPPORTED ("dynamic test requires --enable-hardcoded-path-in-tests");
+
   /* Setgid child process.  */
   if (argc == 2 && strcmp (argv[1], SETGID_CHILD) == 0)
     {
@@ -104,20 +111,33 @@  do_test (int argc, char **argv)
       if (ret != 0)
 	exit (1);
 
-      exit (EXIT_SUCCESS);
+      /* Special return code to make sure that the child executed all the way
+	 through.  */
+      exit (42);
     }
   else
     {
-      if (test_parent () != 0)
-	exit (1);
+      for (const struct envvar_t *e = filtered_envvars;
+	   e != array_end (filtered_envvars);
+	   e++)
+	setenv (e->env, e->value, 1);
+
+      for (const struct envvar_t *e = unfiltered_envvars;
+	   e != array_end (unfiltered_envvars);
+	   e++)
+	setenv (e->env, e->value, 1);
 
       int status = support_capture_subprogram_self_sgid (SETGID_CHILD);
 
       if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
-	return EXIT_UNSUPPORTED;
-
-      if (!WIFEXITED (status))
-	FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status);
+	exit (EXIT_UNSUPPORTED);
+
+      if (WEXITSTATUS (status) != 42)
+	{
+	  printf ("    child failed with status %d\n",
+		  WEXITSTATUS (status));
+	  support_record_failure ();
+	}
 
       return 0;
     }
diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
index 81397fb90b..f7ebed60e5 100644
--- a/sysdeps/generic/unsecvars.h
+++ b/sysdeps/generic/unsecvars.h
@@ -18,7 +18,14 @@ 
   "LD_SHOW_AUXV\0"							      \
   "LOCALDOMAIN\0"							      \
   "LOCPATH\0"								      \
+  "MALLOC_ARENA_MAX\0"							      \
+  "MALLOC_ARENA_TEST\0"							      \
+  "MALLOC_MMAP_MAX_\0"							      \
+  "MALLOC_MMAP_THRESHOLD_\0"						      \
+  "MALLOC_PERTURB_\0"							      \
+  "MALLOC_TOP_PAD_\0"							      \
   "MALLOC_TRACE\0"							      \
+  "MALLOC_TRIM_THRESHOLD_\0"						      \
   "NIS_PATH\0"								      \
   "NLSPATH\0"								      \
   "RESOLV_HOST_CONF\0"							      \