Re: [PATCH] elf: Make string tunables startup-only

On 29/07/26 09:56, Yury Khrustalev wrote:
> On Wed, Jun 17, 2026 at 04:35:45PM -0300, Adhemerval Zanella wrote:
>> String tunable values reference the GLIBC_TUNABLES (or alias) environment
>> string, which lives in the environment block the kernel places on the
>> initial stack.  That memory is owned by the application, which may
>> overwrite it (e.g. setproctitle), so the references are only safe while no
>> application code has run.  Until now this was an undocumented convention:
>> every string tunable happened to be consumed by init_cpu_features during
>> early startup.
>>
>> Make the lifetime explicit and enforced without copying the value or
>> allocating any memory.  Add __tunable_seal_strings, which drops every
>> string tunable reference (and marks each string tunable sealed) once early
>> startup is complete, and have __tunable_get_val report a fatal error when
>> a sealed string tunable is read.
>>
>> The seal is applied after the only string tunable consumer
>> (init_cpu_features, run from DL_PLATFORM_INIT before dl_main, or from
>> ARCH_INIT_CPU_FEATURES in __libc_start_main) but before any user code can
>> run.  In particular it precedes the relocation phase, where IFUNC
>> resolvers fire, and the constructors run later from _dl_init; it also
>> precedes RELRO, which freezes the tunable list.
> 
> Sounds good. I think it's a useful improvement and I have no question
> for the implementation. A few comments on tests below.
> 
> Just a thought: Should we seal string tunable as soon as it is read?
> 
>>
>> ...
>>
>> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
>> index 5c65e8b4583..91238349506 100644
>> --- a/elf/dl-tunables.c
>> +++ b/elf/dl-tunables.c
>> @@ -23,6 +23,7 @@
>>  # pragma GCC visibility push(hidden)
>>  #endif
>>  #include <startup.h>
>> +#include <assert.h>
>>  #include <stdint.h>
>>  #include <stdbool.h>
>>  #include <unistd.h>
>> @@ -460,6 +461,12 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback)
>>  	}
>>      case TUNABLE_TYPE_STRING:
>>  	{
>> +	  /* String tunables reference the environment block and are only
>> +	     valid during early startup; once sealed they must not be read. */
>> +	  if (__glibc_unlikely (cur->sealed))
>> +	    _dl_fatal_printf ("Inconsistency detected: trying to read the %s "
>> +			      "string tunable after process initialization\n",
>> +			      cur->name);
> 
> Nit: maybe this error message is too long? No strong opinion about this
> though.

I can replace 'Inconsistency detected' with 'error:', but I don't see how to short
the description further.

> 
>>
>> ...
>>
>> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
>> index 52ac85a75db..fc26750bd12 100644
>> --- a/sysdeps/aarch64/Makefile
>> +++ b/sysdeps/aarch64/Makefile
>> @@ -50,6 +50,9 @@ tests-internal += \
>>    tst-ifunc-arg-4 \
>>    # tests-internal
>>  
>> +CFLAGS-tst-tunables-seal.c += -DTST_SEAL_TUNABLE_NAME=name
>> +tst-tunables-seal-ENV = GLIBC_TUNABLES=glibc.cpu.name=generic
> 
> The glibc.cpu.name tunable is no longer supported on aarch64. We can use
> the glibc.cpu.hwcaps tunable instead.
> 
> Also, could use use the -TUNABLES instead of -ENV (for all added tests)?
> 
>   CFLAGS-tst-tunables-seal.c += -DTST_SEAL_TUNABLE_NAME=hwcaps
>   tst-tunables-seal-TUNABLES += glibc.cpu.hwcaps=-midr

Indeed, this patch was sent before the glibc.cpu.name.  And I will change to
use the -TUNABLES instead.

> 
>>
>> ...
>>
>> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
>> index b16f4a34049..73705fcce18 100644
>> --- a/sysdeps/x86/Makefile
>> +++ b/sysdeps/x86/Makefile
>> @@ -29,6 +29,14 @@ tests += \
>>    tst-get-cpu-features-static \
>>    tst-hwcap-tunables \
>>  # tests
>> +
>> +tests-internal += \
>> +  tst-tunables-seal-static \
>> +# tests-internal
>> +
>> +tests-static += \
>> +  tst-tunables-seal-static \
>> +# tests-static
> 
> Why static test only for x86?

Good question, I think because it was the machine I used for testing. I will
add some static tests for other ABIs as well.

> 
> Thanks,
> Yury
>