Re: [PATCH v2 1/1] Add system-wide tunables: implement overridability

On 02/07/26 12:19, DJ Delorie wrote:
> 
> Andreas Schwab <[email protected]> writes:
>> That proves to be pretty fragile.
> 
> How about this?
> 
>     Add system-wide tunables: implement overridability
>     
>     Implement the overridability/nonoverridability flags
>     for tunables.
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 789c504da9..7b00a0b931 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -347,7 +347,11 @@ tests-static += \
>    tst-tls9-static \
>    # tests-static
>  
> -tst-tunconf1-TUNABLES-only = glibc.malloc.tcache_count=5
> +tst-tunconf1-TUNABLES-only = \
> +   glibc.malloc.tcache_count=5 \
> +  :glibc.malloc.perturb=41 \
> +  :glibc.malloc.mmap_threshold=10002 \
> +  :glibc.malloc.trim_threshold=10002

No need to add ':' for each tunable, the TUNABLES rule already handles it.

The rest looks ok.

>  
>  static-dlopen-environment = \
>    LD_LIBRARY_PATH=$(ld-library-path):$(common-objpfx)dlfcn
> diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
> index f421c01c26..3506eee512 100644
> --- a/elf/dl-tunable-types.h
> +++ b/elf/dl-tunable-types.h
> @@ -65,6 +65,8 @@ struct _tunable
>    tunable_val_t val;			/* The value.  */
>    bool initialized;			/* Flag to indicate that the tunable is
>  					   initialized.  */
> +  bool locked;				/* If set, modifications are not
> +					   allowed.  */
>    /* 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 197d940d38..d19c2358c1 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -72,6 +72,9 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
>  {
>    tunable_num_t val, min, max;
>  
> +  if (cur->locked)
> +    return;
> +
>    switch (cur->type.type_code)
>      {
>      case TUNABLE_TYPE_STRING:
> @@ -409,6 +412,12 @@ __tunables_init (char **envp, char **argv)
>  	    goto skip_due_to_filter;
>  	  }
>  
> +	  /* If the tunable is set here, any previously set
> +	     overridability flag is discarded.  We need to reset the
> +	     overridability flag here so we can change the tunable,
> +	     and may set it later if this tunable also locks it.  */
> +	  tunable_list[tid].locked = false;
> +
>  	  /* See if the parsed type matches the desired type.  */
>  	  if (tunable_list[tid].type.type_code == TUNABLE_TYPE_STRING)
>  	    {
> @@ -435,6 +444,12 @@ __tunables_init (char **envp, char **argv)
>  		}
>  	    }
>  
> +	  /* The overriability flag only applies to tunables
> +	     which aren't filtered out.  */
> +	  if ((tec->flags & TUNCONF_FLAG_OVERRIDABLE)
> +	      == TUNCONF_OVERRIDE_DENY)
> +	    tunable_list[tid].locked = true;
> +
>  	skip_due_to_filter:;
>  	}
>      }
> diff --git a/elf/tst-tunconf1.c b/elf/tst-tunconf1.c
> index c95a7cb8ba..74f596d913 100644
> --- a/elf/tst-tunconf1.c
> +++ b/elf/tst-tunconf1.c
> @@ -26,10 +26,30 @@ do_test (void)
>  {
>    size_t tcache_count = TUNABLE_GET_FULL (glibc, malloc, tcache_count, size_t, NULL);
>    size_t tcache_max = TUNABLE_GET_FULL (glibc, malloc, tcache_max, size_t, NULL);
> +  size_t perturb = TUNABLE_GET_FULL (glibc, malloc, perturb, size_t, NULL);
> +  size_t mmap_threshold = TUNABLE_GET_FULL (glibc, malloc, mmap_threshold, size_t, NULL);
> +  size_t trim_threshold = TUNABLE_GET_FULL (glibc, malloc, trim_threshold, size_t, NULL);
> +
>    printf("tcache count is %ld (should be 5, from env)\n", (long)tcache_count);
>    TEST_COMPARE ((long)tcache_count, 5);
>    printf("tcache max is %ld (should be 4, from /etc)\n", (long)tcache_max);
>    TEST_COMPARE ((long)tcache_max, 4);
> +
> +  /* This is set by the environment but blocked by the config.  */
> +  printf("perturb is %ld (should be 42, from /etc)\n",
> +	 (long)perturb);
> +  TEST_COMPARE ((long)perturb, 42);
> +
> +  /* This is blocked by the general config, enabled by filter, set in env.  */
> +  printf("mmap_threshold is %ld (should be 10002, from env)\n",
> +	 (long)mmap_threshold);
> +  TEST_COMPARE ((long)mmap_threshold, 10002);
> +
> +  /* This is allowed by the general config, blocked by filter, set in env.  */
> +  printf("trim_threshold is %ld (should be 10001, from filter)\n",
> +	 (long)trim_threshold);
> +  TEST_COMPARE ((long)trim_threshold, 10001);
> +
>    return 0;
>  }
>  
> diff --git a/elf/tst-tunconf1.root/etc/tunables.conf b/elf/tst-tunconf1.root/etc/tunables.conf
> index f373a67902..f708a8fce4 100644
> --- a/elf/tst-tunconf1.root/etc/tunables.conf
> +++ b/elf/tst-tunconf1.root/etc/tunables.conf
> @@ -8,7 +8,15 @@ $glibc.test_unsecure=1
>  # These are checked inside the test case
>  glibc.malloc.tcache_max=6
>  $glibc.malloc.tcache_count=3
> +
> +-glibc.malloc.perturb=42
> +-glibc.malloc.mmap_threshold=10000
> +overridable glibc.malloc.trim_threshold=10000
> +
>  [proc:/bin/ls]
>  glibc.malloc.tcache_max=7
> +
>  [proc:tst-tunconf1]
>  glibc.malloc.tcache_max=4
> ++glibc.malloc.mmap_threshold=10001
> +nonoverridable glibc.malloc.trim_threshold=10001
> diff --git a/elf/tunconf.c b/elf/tunconf.c
> index a5ca755abd..c27b50b1de 100644
> --- a/elf/tunconf.c
> +++ b/elf/tunconf.c
> @@ -194,34 +194,40 @@ add_tunable (char *line, const char *filename, int lineno)
>    /* Parse modifiers.  */
>    while (*line)
>      {
> -      if (strncmp (line, "overridable ", 13) == 0)
> +      int prefix_len;
> +
> +#define TUN_PREFIX(s)				\
> +      prefix_len = sizeof(s) - 1,			\
> +	strncmp (line, s " ", prefix_len + 1) == 0
> +
> +      if (TUN_PREFIX("overridable"))
>  	{
>  	  top = TOP_ALLOW;
>  	  /* The line++ below skips the space.  */
> -	  line += 12;
> +	  line += prefix_len;
>  	}
> -      else if (strncmp (line, "nonoverridable ", 16) == 0)
> +      else if (TUN_PREFIX ("nonoverridable"))
>  	{
>  	  top = TOP_DENY;
> -	  line += 15;
> +	  line += prefix_len;
>  	}
> -      else if (strncmp (line, "onlysecure ", 11) == 0)
> +      else if (TUN_PREFIX ("onlysecure"))
>  	{
>  	  exclude_nonsecure = 1;
>  	  exclude_secure = 0;
> -	  line += 10;
> +	  line += prefix_len;
>  	}
> -      else if (strncmp (line, "nonsecure ", 10) == 0)
> +      else if (TUN_PREFIX ("nonsecure"))
>  	{
>  	  exclude_secure = 1;
>  	  exclude_nonsecure = 0;
> -	  line += 9;
> +	  line += prefix_len;
>  	}
> -      else if (strncmp (line, "anysecure ", 10) == 0)
> +      else if (TUN_PREFIX ("anysecure"))
>  	{
>  	  exclude_secure = 0;
>  	  exclude_nonsecure = 0;
> -	  line += 9;
> +	  line += prefix_len;
>  	}
>        else switch (*line)
>  	{
> diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
> index 5d34075c16..7c832303db 100644
> --- a/scripts/gen-tunables.awk
> +++ b/scripts/gen-tunables.awk
> @@ -169,7 +169,7 @@ END {
>      n = indices[2];
>      m = indices[3];
>      printf ("  {TUNABLE_NAME_S(%s, %s, %s)", t, n, m)
> -    printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, {%s}, false, %s},\n",
> +    printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, {%s}, false, false, %s},\n",
>  	    types[t,n,m], minvals[t,n,m], maxvals[t,n,m], default_val[t,n,m],
>  	    default_val[t,n,m], env_alias[t,n,m]);
>    }
>