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

[Adhemerval Zanella Netto]

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'.