[8/8] elf: Scrub and reseed the AT_RANDOM bytes after deriving the guards (BZ 34197)

  Once the pointer and stack guards have been derived from AT_RANDOM, scrub
the bytes and refill them with new random data unrelated to the guards.
On Linux, it uses getrandom syscall (as for tcache_key_initialize), and
fallback to zero the memory if the syscall is not avaiable.

This keeps AT_RANDOM useful to applications while ensuring those bytes no
longer reveal the guards.

The work is done by _dl_reseed_random, called once the guards are in place
and before any ELF constructor can observe AT_RANDOM: in security_init for
the dynamic loader and in __libc_start_main for statically linked programs.

Checked on aarch64-linux-gnu, x86_64-linux-gnu, and i686-linux-gnu.
---
 csu/libc-start.c                           |  4 ++
 elf/Makefile                               |  5 ++
 elf/rtld.c                                 |  6 +-
 elf/tst-atrandom-scrub-static.c            |  1 +
 elf/tst-atrandom-scrub.c                   | 74 ++++++++++++++++++++++
 sysdeps/generic/dl-reseed-random.h         | 34 ++++++++++
 sysdeps/unix/sysv/linux/dl-reseed-random.h | 43 +++++++++++++
 7 files changed, 163 insertions(+), 4 deletions(-)
 create mode 100644 elf/tst-atrandom-scrub-static.c
 create mode 100644 elf/tst-atrandom-scrub.c
 create mode 100644 sysdeps/generic/dl-reseed-random.h
 create mode 100644 sysdeps/unix/sysv/linux/dl-reseed-random.h
  

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> Once the pointer and stack guards have been derived from AT_RANDOM, scrub
> the bytes and refill them with new random data unrelated to the guards.
> On Linux, it uses getrandom syscall (as for tcache_key_initialize), and
> fallback to zero the memory if the syscall is not avaiable.
>
> This keeps AT_RANDOM useful to applications while ensuring those bytes no
> longer reveal the guards.
>
> The work is done by _dl_reseed_random, called once the guards are in place
> and before any ELF constructor can observe AT_RANDOM: in security_init for
> the dynamic loader and in __libc_start_main for statically linked programs.

> diff --git a/csu/libc-start.c b/csu/libc-start.c
> +  _dl_reseed_random (&_dl_random);

Ok.

> diff --git a/elf/rtld.c b/elf/rtld.c
> -  /* We do not need the _dl_random value anymore.  The less
> -     information we leave behind, the better, so clear the
> -     variable.  */

I see no reason to remove this comment; it's still valid.  If anything,
it should be expanded to include the new logic.  The other caller has no
comment, though...

> -  _dl_random = NULL;
> +  _dl_reseed_random (&_dl_random);

can _dl_reseed_random ever be called twice in the same program?  This
second one happens when we load audit modules, but you don't test that
case, and I wonder if calling it twice might result in NULL guards, or
mismatched ones...

> diff --git a/elf/tst-atrandom-scrub.c b/elf/tst-atrandom-scrub.c

> +/* The loader (security_init) and the static startup code (__libc_start_main)
> +   derive the stack and pointer guards from the AT_RANDOM bytes, scrub those
> +   bytes, and refill them with fresh entropy unrelated to the guards.  The
> +   AT_RANDOM entry is kept, so getauxval (AT_RANDOM) keeps returning 16 random
> +   bytes, but they no longer reveal the guards.  Check that neither guard can
> +   be reconstructed from AT_RANDOM and that no auxiliary vector entry holds a
> +   guard value.  */
> +
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/auxv.h>
> +
> +#include <stackguard-macros.h>
> +#include <tls.h>
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> +  uintptr_t stack_guard = STACK_CHK_GUARD;
> +  uintptr_t pointer_guard = POINTER_CHK_GUARD;
> +
> +  unsigned char *random = (unsigned char *) getauxval (AT_RANDOM);
> +  if (random == NULL)
> +    FAIL_UNSUPPORTED ("the kernel did not provide AT_RANDOM");
> +
> +  printf ("debug: stack guard   = %0*jx\n",
> +          (int) (2 * sizeof (uintptr_t)), (uintmax_t) stack_guard);
> +  printf ("debug: pointer guard = %0*jx\n",
> +          (int) (2 * sizeof (uintptr_t)), (uintmax_t) pointer_guard);
> +  printf ("debug: AT_RANDOM     = ");
> +  for (int i = 0; i < 16; i++)
> +    printf ("%02x", random[i]);
> +  printf ("\n");

Ok.

> +  /* Reconstruct the guards from the (reseeded) AT_RANDOM bytes the way the
> +     loader does and check that they no longer match the live guards.  */
> +  uintptr_t recovered_stack;
> +  memcpy (&recovered_stack, random, sizeof (recovered_stack));
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> +  recovered_stack &= ~(uintptr_t) 0xff;
> +#else
> +  recovered_stack &= ~((uintptr_t) 0xff << (8 * (sizeof (recovered_stack) - 1)));
> +#endif
> +  TEST_VERIFY (recovered_stack != stack_guard);
> +
> +  uintptr_t recovered_pointer;
> +  memcpy (&recovered_pointer, random + sizeof (uintptr_t),
> +          sizeof (recovered_pointer));
> +  TEST_VERIFY (recovered_pointer != pointer_guard);

Ok.

Should be verify that none of (stack_guard, pointer_guard, AT_RANDOM)
are all zeros?  Even assuming that the kernel can always give us more
random bits, we should ensure that they're being filled properly.

> diff --git a/sysdeps/generic/dl-reseed-random.h b/sysdeps/generic/dl-reseed-random.h
> +#include <string.h>
> +
> +static inline void __attribute__ ((always_inline))
> +_dl_reseed_random (void **dl_random)
> +{
> +  if (*dl_random == NULL)
> +    return;
> +  memset (*dl_random, '\0', 16);
> +  __asm__ __volatile__ ("" : : "r" (*dl_random) : "memory");
> +  *dl_random = NULL;
> +}
> +

Ok.

> diff --git a/sysdeps/unix/sysv/linux/dl-reseed-random.h b/sysdeps/unix/sysv/linux/dl-reseed-random.h
> +#include <string.h>
> +#include <not-cancel.h>
> +#include <sys/random.h>
> +
> +/* The stack and pointer guards have been derived from the 16 AT_RANDOM
> +   bytes pointed to by DL_RANDOM.  Scrub them first, so the guards cannot be
> +   recovered even if the refill below fails, then refill them with fresh
> +   entropy unrelated to the guards so that getauxval (AT_RANDOM) keeps
> +   returning random bytes.  */
> +static inline void __attribute__ ((always_inline))
> +_dl_reseed_random (void **dl_random)
> +{
> +  if (*dl_random == NULL)
> +    return;
> +  memset (*dl_random, '\0', 16);
> +  __asm__ __volatile__ ("" : : "r" (*dl_random) : "memory");
> +
> +  __getrandom_nocancel_nostatus_direct (*dl_random, 16, GRND_NONBLOCK);
> +  _dl_random = NULL;

Why access _dl_random directly here, and not &dl_random like the generic case?

IMHO This looks too much like a typo.
  

On 10/06/26 19:05, DJ Delorie wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> Once the pointer and stack guards have been derived from AT_RANDOM, scrub
>> the bytes and refill them with new random data unrelated to the guards.
>> On Linux, it uses getrandom syscall (as for tcache_key_initialize), and
>> fallback to zero the memory if the syscall is not avaiable.
>>
>> This keeps AT_RANDOM useful to applications while ensuring those bytes no
>> longer reveal the guards.
>>
>> The work is done by _dl_reseed_random, called once the guards are in place
>> and before any ELF constructor can observe AT_RANDOM: in security_init for
>> the dynamic loader and in __libc_start_main for statically linked programs.
> 
>> diff --git a/csu/libc-start.c b/csu/libc-start.c
>> +  _dl_reseed_random (&_dl_random);
> 
> Ok.
> 
>> diff --git a/elf/rtld.c b/elf/rtld.c
>> -  /* We do not need the _dl_random value anymore.  The less
>> -     information we leave behind, the better, so clear the
>> -     variable.  */
> 
> I see no reason to remove this comment; it's still valid.  If anything,
> it should be expanded to include the new logic.  The other caller has no
> comment, though...

Ack, I will keep and add a similar one on the other caller.

> 
>> -  _dl_random = NULL;
>> +  _dl_reseed_random (&_dl_random);
> 
> can _dl_reseed_random ever be called twice in the same program?  This
> second one happens when we load audit modules, but you don't test that
> case, and I wonder if calling it twice might result in NULL guards, or
> mismatched ones...

The security_init cannot be called twice: in dl_main, need_security_init is
set 'false' after the early audit-path call. The later call around line 2020
is gated on need_security_init, and the __libc_start_main's call is only for
!SHARED.

But I think even if a double call were ever introduced, the function should be
safe. The if (*dl_random == NULL) return; early-return makes the second call a 
no-op.

> 
>> diff --git a/elf/tst-atrandom-scrub.c b/elf/tst-atrandom-scrub.c
> 
>> +/* The loader (security_init) and the static startup code (__libc_start_main)
>> +   derive the stack and pointer guards from the AT_RANDOM bytes, scrub those
>> +   bytes, and refill them with fresh entropy unrelated to the guards.  The
>> +   AT_RANDOM entry is kept, so getauxval (AT_RANDOM) keeps returning 16 random
>> +   bytes, but they no longer reveal the guards.  Check that neither guard can
>> +   be reconstructed from AT_RANDOM and that no auxiliary vector entry holds a
>> +   guard value.  */
>> +
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <sys/auxv.h>
>> +
>> +#include <stackguard-macros.h>
>> +#include <tls.h>
>> +#include <support/check.h>
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  uintptr_t stack_guard = STACK_CHK_GUARD;
>> +  uintptr_t pointer_guard = POINTER_CHK_GUARD;
>> +
>> +  unsigned char *random = (unsigned char *) getauxval (AT_RANDOM);
>> +  if (random == NULL)
>> +    FAIL_UNSUPPORTED ("the kernel did not provide AT_RANDOM");
>> +
>> +  printf ("debug: stack guard   = %0*jx\n",
>> +          (int) (2 * sizeof (uintptr_t)), (uintmax_t) stack_guard);
>> +  printf ("debug: pointer guard = %0*jx\n",
>> +          (int) (2 * sizeof (uintptr_t)), (uintmax_t) pointer_guard);
>> +  printf ("debug: AT_RANDOM     = ");
>> +  for (int i = 0; i < 16; i++)
>> +    printf ("%02x", random[i]);
>> +  printf ("\n");
> 
> Ok.
> 
>> +  /* Reconstruct the guards from the (reseeded) AT_RANDOM bytes the way the
>> +     loader does and check that they no longer match the live guards.  */
>> +  uintptr_t recovered_stack;
>> +  memcpy (&recovered_stack, random, sizeof (recovered_stack));
>> +#if __BYTE_ORDER == __LITTLE_ENDIAN
>> +  recovered_stack &= ~(uintptr_t) 0xff;
>> +#else
>> +  recovered_stack &= ~((uintptr_t) 0xff << (8 * (sizeof (recovered_stack) - 1)));
>> +#endif
>> +  TEST_VERIFY (recovered_stack != stack_guard);
>> +
>> +  uintptr_t recovered_pointer;
>> +  memcpy (&recovered_pointer, random + sizeof (uintptr_t),
>> +          sizeof (recovered_pointer));
>> +  TEST_VERIFY (recovered_pointer != pointer_guard);
> 
> Ok.
> 
> Should be verify that none of (stack_guard, pointer_guard, AT_RANDOM)
> are all zeros?  Even assuming that the kernel can always give us more
> random bits, we should ensure that they're being filled properly.
> 
>> diff --git a/sysdeps/generic/dl-reseed-random.h b/sysdeps/generic/dl-reseed-random.h
>> +#include <string.h>
>> +
>> +static inline void __attribute__ ((always_inline))
>> +_dl_reseed_random (void **dl_random)
>> +{
>> +  if (*dl_random == NULL)
>> +    return;
>> +  memset (*dl_random, '\0', 16);
>> +  __asm__ __volatile__ ("" : : "r" (*dl_random) : "memory");
>> +  *dl_random = NULL;
>> +}
>> +
> 
> Ok.
> 
>> diff --git a/sysdeps/unix/sysv/linux/dl-reseed-random.h b/sysdeps/unix/sysv/linux/dl-reseed-random.h
>> +#include <string.h>
>> +#include <not-cancel.h>
>> +#include <sys/random.h>
>> +
>> +/* The stack and pointer guards have been derived from the 16 AT_RANDOM
>> +   bytes pointed to by DL_RANDOM.  Scrub them first, so the guards cannot be
>> +   recovered even if the refill below fails, then refill them with fresh
>> +   entropy unrelated to the guards so that getauxval (AT_RANDOM) keeps
>> +   returning random bytes.  */
>> +static inline void __attribute__ ((always_inline))
>> +_dl_reseed_random (void **dl_random)
>> +{
>> +  if (*dl_random == NULL)
>> +    return;
>> +  memset (*dl_random, '\0', 16);
>> +  __asm__ __volatile__ ("" : : "r" (*dl_random) : "memory");
>> +
>> +  __getrandom_nocancel_nostatus_direct (*dl_random, 16, GRND_NONBLOCK);
>> +  _dl_random = NULL;
> 
> Why access _dl_random directly here, and not &dl_random like the generic case?
> 
> IMHO This looks too much like a typo.
> 

It is a typo, it should be "*dl_random = NULL'.
  

Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
>>> -  _dl_random = NULL;
>>> +  _dl_reseed_random (&_dl_random);
>> 
>> can _dl_reseed_random ever be called twice in the same program?  This
>> second one happens when we load audit modules, but you don't test that
>> case, and I wonder if calling it twice might result in NULL guards, or
>> mismatched ones...
>
> The security_init cannot be called twice: in dl_main, need_security_init is
> set 'false' after the early audit-path call. The later call around line 2020
> is gated on need_security_init, and the __libc_start_main's call is only for
> !SHARED.
>
> But I think even if a double call were ever introduced, the function should be
> safe. The if (*dl_random == NULL) return; early-return makes the second call a 
> no-op.

I was more worried about the call in csu/libc-start.c and the call in
elf/rtld.c both affecting the same process.
  

On 11/06/26 17:25, DJ Delorie wrote:
> 
> Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
>>>> -  _dl_random = NULL;
>>>> +  _dl_reseed_random (&_dl_random);
>>>
>>> can _dl_reseed_random ever be called twice in the same program?  This
>>> second one happens when we load audit modules, but you don't test that
>>> case, and I wonder if calling it twice might result in NULL guards, or
>>> mismatched ones...
>>
>> The security_init cannot be called twice: in dl_main, need_security_init is
>> set 'false' after the early audit-path call. The later call around line 2020
>> is gated on need_security_init, and the __libc_start_main's call is only for
>> !SHARED.
>>
>> But I think even if a double call were ever introduced, the function should be
>> safe. The if (*dl_random == NULL) return; early-return makes the second call a 
>> no-op.
> 
> I was more worried about the call in csu/libc-start.c and the call in
> elf/rtld.c both affecting the same process.
> 

Right, the csu/libc-start.c is only for static objects.  The static-dlopen edge
case also does trigger the elf/rtld.c initialization, and that's why we have
__rtld_static_init to handle the requires missing pieces.