[PATCH 4/8] elf: Let environment aliases override overridable system-wide tunables

[Adhemerval Zanella]

The environment-variable alias loop in __tunables_init skipped every tunable
whose "initialized" flag was set, which was originally meant only to give
the canonical GLIBC_TUNABLES form precedence over the legacy MALLOC_*
aliases.

Now that the cache also sets "initialized", a legacy alias could no longer
override an *overridable* cache default, even though the canonical
GLIBC_TUNABLES form still could -- an inconsistent and surprising asymmetry.

Track separately the tunables that were set from GLIBC_TUNABLES during this
call and skip only those in the alias loop

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 elf/Makefile                            |  7 ++++++-
 elf/dl-tunables.c                       | 20 ++++++++++++++++++--
 elf/tst-tunconf1.c                      | 24 ++++++++++++++++++++++++
 elf/tst-tunconf1.root/etc/tunables.conf |  9 +++++++++
 4 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/elf/Makefile b/elf/Makefile
index c0bf3fd0d88..2c34827e241 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -351,7 +351,12 @@ tst-tunconf1-TUNABLES-only = \
    glibc.malloc.tcache_count=5 \
   :glibc.malloc.perturb=41 \
   :glibc.malloc.mmap_threshold=10002 \
-  :glibc.malloc.trim_threshold=10002
+  :glibc.malloc.trim_threshold=10002 \
+  :glibc.malloc.arena_max=300
+tst-tunconf1-ENV = \
+  MALLOC_MMAP_MAX_=200 \
+  MALLOC_TOP_PAD_=200 \
+  MALLOC_ARENA_MAX=400
 
 static-dlopen-environment = \
   LD_LIBRARY_PATH=$(ld-library-path):$(common-objpfx)dlfcn
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index d19c2358c10..2ff87610eb4 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -179,6 +179,14 @@ struct tunable_toset_t
 
 enum { tunables_list_size = array_length (tunable_list) };
 
+/* Records tunables that were set from GLIBC_TUNABLES during this call, so
+   that a legacy environment-variable alias does not override themi (the
+   canonical GLIBC_TUNABLES form takes precedence over the aliases).
+   A tunable that was set only from the system-wide cache is deliberately not
+   recorded here, so an alias may still override an overridable cache default;
+   a nonoverridable one remains protected by tunable_t::locked.  */
+static bool tunable_set_by_env[tunables_list_size];
+
 /* Parse the tunable string VALSTRING and set TUNABLES with the found tunables
    and their respective values.  The VALSTRING is parsed in place, with the
    tunable start and size recorded in TUNABLES.
@@ -288,6 +296,10 @@ parse_tunables (const char *valstring)
       if (!tunable_initialize (tunables[i].t, tunables[i].value,
 			       tunables[i].len))
 	parse_tunable_print_error (&tunables[i]);
+      else
+	/* GLIBC_TUNABLES set this tunable; a legacy alias must not
+	   override it.  */
+	tunable_set_by_env[i] = true;
     }
 }
 
@@ -498,9 +510,13 @@ __tunables_init (char **envp, char **argv)
 
   for (int i = 0; i < tunable_num_env_alias; i++)
     {
-      /* Skip over tunables that have either been set or already initialized.  */
+      /* Skip aliases whose tunable was already set through GLIBC_TUNABLES,
+	 which takes precedence over the alias.  A value coming only from the
+	 system-wide cache does not block the alias here: an overridable cache
+	 default may still be overridden, while a nonoverridable one is
+	 protected by tunable_t::locked.  */
       if (tunables_env_alias[i].t == NULL
-	  || tunables_env_alias[i].t->initialized)
+	  || tunable_set_by_env[tunable_env_alias_list[i]])
 	continue;
 
       if (!tunable_initialize (tunables_env_alias[i].t,
diff --git a/elf/tst-tunconf1.c b/elf/tst-tunconf1.c
index 74f596d913b..2d245de84f1 100644
--- a/elf/tst-tunconf1.c
+++ b/elf/tst-tunconf1.c
@@ -16,6 +16,7 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <stdint.h>
 #include <stdio.h>
 #include <support/check.h>
 
@@ -50,6 +51,29 @@ do_test (void)
 	 (long)trim_threshold);
   TEST_COMPARE ((long)trim_threshold, 10001);
 
+  /* Interaction with legacy environment-variable aliases (MALLOC_*).  */
+  int32_t mmap_max = TUNABLE_GET_FULL (glibc, malloc, mmap_max, int32_t, NULL);
+  size_t top_pad = TUNABLE_GET_FULL (glibc, malloc, top_pad, size_t, NULL);
+  size_t arena_max = TUNABLE_GET_FULL (glibc, malloc, arena_max, size_t, NULL);
+
+  /* Overridable cache default (100); the MALLOC_MMAP_MAX_ alias overrides
+     it, just like GLIBC_TUNABLES would.  */
+  printf("mmap_max is %d (should be 200, from MALLOC_MMAP_MAX_ alias)\n",
+	 mmap_max);
+  TEST_COMPARE (mmap_max, 200);
+
+  /* Nonoverridable cache default (100); the MALLOC_TOP_PAD_ alias must not
+     override it.  */
+  printf("top_pad is %ld (should be 100, from /etc nonoverridable)\n",
+	 (long)top_pad);
+  TEST_COMPARE ((long)top_pad, 100);
+
+  /* Set both by GLIBC_TUNABLES (300) and by the MALLOC_ARENA_MAX alias
+     (400); the canonical GLIBC_TUNABLES form wins.  */
+  printf("arena_max is %ld (should be 300, from GLIBC_TUNABLES)\n",
+	 (long)arena_max);
+  TEST_COMPARE ((long)arena_max, 300);
+
   return 0;
 }
 
diff --git a/elf/tst-tunconf1.root/etc/tunables.conf b/elf/tst-tunconf1.root/etc/tunables.conf
index f708a8fce4a..b590635e135 100644
--- a/elf/tst-tunconf1.root/etc/tunables.conf
+++ b/elf/tst-tunconf1.root/etc/tunables.conf
@@ -13,6 +13,15 @@ $glibc.malloc.tcache_count=3
 -glibc.malloc.mmap_threshold=10000
 overridable glibc.malloc.trim_threshold=10000
 
+# Interaction with legacy environment-variable aliases (MALLOC_*), checked
+# in the test case:
+#  - mmap_max: overridable cache default, must be overridable by the
+#    MALLOC_MMAP_MAX_ alias just like by GLIBC_TUNABLES.
+#  - top_pad: nonoverridable cache default, must NOT be overridable by the
+#    MALLOC_TOP_PAD_ alias.
+glibc.malloc.mmap_max=100
+-glibc.malloc.top_pad=100
+
 [proc:/bin/ls]
 glibc.malloc.tcache_max=7
 
-- 
2.43.0