[v2,02/19] elf: Add GLIBC_TUNABLES to unsecvars

Message ID 20231017130526.2216827-3-adhemerval.zanella@linaro.org
State Superseded
Delegated to: Siddhesh Poyarekar
Headers
Series Improve loader environment variable handling |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed

Commit Message

Adhemerval Zanella Netto Oct. 17, 2023, 1:05 p.m. UTC
  setuid/setgid process now ignores any glibc tunables, and filters out
all environment variables that might changes its behavior. This patch
also adds GLIBC_TUNABLES, so any spawned process by setuid/setgid
processes should set tunable explicitly.

Checked on x86_64-linux-gnu.

Reviewed-by: Florian Weimer <fweimer@redhat.com>
---
 elf/tst-env-setuid-tunables.c | 32 ++++----------------------------
 sysdeps/generic/unsecvars.h   |  1 +
 2 files changed, 5 insertions(+), 28 deletions(-)
  

Comments

Siddhesh Poyarekar Oct. 18, 2023, 12:52 p.m. UTC | #1
On 2023-10-17 09:05, Adhemerval Zanella wrote:
> setuid/setgid process now ignores any glibc tunables, and filters out
> all environment variables that might changes its behavior. This patch
> also adds GLIBC_TUNABLES, so any spawned process by setuid/setgid
> processes should set tunable explicitly.
> 
> Checked on x86_64-linux-gnu.
> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>

Also.

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

> ---
>   elf/tst-env-setuid-tunables.c | 32 ++++----------------------------
>   sysdeps/generic/unsecvars.h   |  1 +
>   2 files changed, 5 insertions(+), 28 deletions(-)
> 
> diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
> index f0b92c97e7..2603007b7b 100644
> --- a/elf/tst-env-setuid-tunables.c
> +++ b/elf/tst-env-setuid-tunables.c
> @@ -60,45 +60,21 @@ const char *teststrings[] =
>     "glibc.not_valid.check=2",
>   };
>   
> -const char *resultstrings[] =
> -{
> -  "glibc.malloc.mmap_threshold=4096",
> -  "glibc.malloc.mmap_threshold=4096",
> -  "glibc.malloc.mmap_threshold=4096",
> -  "glibc.malloc.perturb=0x800",
> -  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
> -  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
> -  "glibc.malloc.mmap_threshold=4096",
> -  "glibc.malloc.mmap_threshold=4096",
> -  "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
> -  "",
> -  "",
> -  "",
> -  "",
> -  "",
> -  "",
> -  "",
> -};
> -
>   static int
>   test_child (int off)
>   {
>     const char *val = getenv ("GLIBC_TUNABLES");
> +  int ret = 1;
>   
>     printf ("    [%d] GLIBC_TUNABLES is %s\n", off, val);
>     fflush (stdout);
> -  if (val != NULL && strcmp (val, resultstrings[off]) == 0)
> -    return 0;
> -
>     if (val != NULL)
> -    printf ("    [%d] Unexpected GLIBC_TUNABLES VALUE %s, expected %s\n",
> -	    off, val, resultstrings[off]);
> +    printf ("    [%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
>     else
> -    printf ("    [%d] GLIBC_TUNABLES environment variable absent\n", off);
> -
> +    ret = 0;
>     fflush (stdout);
>   
> -  return 1;
> +  return ret;
>   }
>   
>   static int
> diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
> index 8278c50a84..81397fb90b 100644
> --- a/sysdeps/generic/unsecvars.h
> +++ b/sysdeps/generic/unsecvars.h
> @@ -4,6 +4,7 @@
>   #define UNSECURE_ENVVARS \
>     "GCONV_PATH\0"							      \
>     "GETCONF_DIR\0"							      \
> +  "GLIBC_TUNABLES\0"							      \
>     "HOSTALIASES\0"							      \
>     "LD_AUDIT\0"								      \
>     "LD_DEBUG\0"								      \
  

Patch

diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
index f0b92c97e7..2603007b7b 100644
--- a/elf/tst-env-setuid-tunables.c
+++ b/elf/tst-env-setuid-tunables.c
@@ -60,45 +60,21 @@  const char *teststrings[] =
   "glibc.not_valid.check=2",
 };
 
-const char *resultstrings[] =
-{
-  "glibc.malloc.mmap_threshold=4096",
-  "glibc.malloc.mmap_threshold=4096",
-  "glibc.malloc.mmap_threshold=4096",
-  "glibc.malloc.perturb=0x800",
-  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
-  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
-  "glibc.malloc.mmap_threshold=4096",
-  "glibc.malloc.mmap_threshold=4096",
-  "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
-  "",
-  "",
-  "",
-  "",
-  "",
-  "",
-  "",
-};
-
 static int
 test_child (int off)
 {
   const char *val = getenv ("GLIBC_TUNABLES");
+  int ret = 1;
 
   printf ("    [%d] GLIBC_TUNABLES is %s\n", off, val);
   fflush (stdout);
-  if (val != NULL && strcmp (val, resultstrings[off]) == 0)
-    return 0;
-
   if (val != NULL)
-    printf ("    [%d] Unexpected GLIBC_TUNABLES VALUE %s, expected %s\n",
-	    off, val, resultstrings[off]);
+    printf ("    [%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
   else
-    printf ("    [%d] GLIBC_TUNABLES environment variable absent\n", off);
-
+    ret = 0;
   fflush (stdout);
 
-  return 1;
+  return ret;
 }
 
 static int
diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
index 8278c50a84..81397fb90b 100644
--- a/sysdeps/generic/unsecvars.h
+++ b/sysdeps/generic/unsecvars.h
@@ -4,6 +4,7 @@ 
 #define UNSECURE_ENVVARS \
   "GCONV_PATH\0"							      \
   "GETCONF_DIR\0"							      \
+  "GLIBC_TUNABLES\0"							      \
   "HOSTALIASES\0"							      \
   "LD_AUDIT\0"								      \
   "LD_DEBUG\0"								      \