fixup! glib-aux: put more effort into seeding GRand fallback for nm_utils_random_bytes()th/thread-local-storage-destroy

1 files changed, 54 insertions, 40 deletions

diff --git a/src/libnm-glib-aux/nm-random-utils.c b/src/libnm-glib-aux/nm-random-utils.c
index d8e716c90e..f622b28411 100644
--- a/src/libnm-glib-aux/nm-random-utils.c
+++ b/src/libnm-glib-aux/nm-random-utils.c
@@ -21,52 +21,49 @@
 
 /*****************************************************************************/
 
-static GRand *
-_rand_create_global(void)
+#define SEED_ARRAY_SIZE (16 + 2 + 4 + 2 + 3)
+
+static guint32
+_pid_hash(pid_t id)
+{
+    if (sizeof(pid_t) > sizeof(guint32))
+        return (((guint64) id) >> 32) ^ ((guint64) id);
+    return id;
+}
+
+static void
+_rand_init_seed(guint32 seed_array[static SEED_ARRAY_SIZE], GRand *rand)
 {
-    GRand *       rand1;
-    guint32       seed_array[16 + 2 + 4 + 2 + 2];
     int           seed_idx;
     const guint8 *p_at_random;
     guint64       now_nsec;
 
-    /* We only call this once, and only in case where getrandom()/urandom
-     * already failed. Let's try hard to get a good seed.
-     *
-     * We also don't want to use glib's singleton GRand instance, because
-     * our code should not consume random numbers from that instance. */
-
-    rand1 = g_rand_new();
-
-    /* Get some seed material by using GRand. g_rand_new() is itself seeded
-     * from /dev/urandom (good, but we are already in a scenario where that
-     * failed) or fallback to the time/pid (not so good). */
+    /* Get some seed material from the provided GRand. */
     for (seed_idx = 0; seed_idx < 16; seed_idx++)
-        seed_array[seed_idx] = g_rand_int(rand1);
+        seed_array[seed_idx] = g_rand_int(rand);
 
     /* Add an address from the heap. */
-    seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand1))) >> 32;
-    seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand1)));
-
-    g_rand_free(rand1);
+    seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand))) >> 32;
+    seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand)));
 
     /* Add the per-process, random number. */
     p_at_random = ((gpointer) getauxval(AT_RANDOM));
     if (p_at_random) {
-        G_STATIC_ASSERT(sizeof(guint32) == 4);
         memcpy(&seed_array[seed_idx], p_at_random, 16);
-        seed_idx += 4;
-    }
+    } else
+        memset(&seed_array[seed_idx], 0, 16);
+    G_STATIC_ASSERT_EXPR(sizeof(guint32) == 4);
+    seed_idx += 4;
 
     /* Add the current timestamp, the pid and ppid. */
     now_nsec               = nm_utils_clock_gettime_nsec(CLOCK_BOOTTIME);
     seed_array[seed_idx++] = ((guint64) now_nsec) >> 32;
     seed_array[seed_idx++] = ((guint64) now_nsec);
-    seed_array[seed_idx++] = getpid();
-    seed_array[seed_idx++] = getppid();
+    seed_array[seed_idx++] = _pid_hash(getpid());
+    seed_array[seed_idx++] = _pid_hash(getppid());
+    seed_array[seed_idx++] = _pid_hash(gettid());
 
-    nm_assert(seed_idx <= G_N_ELEMENTS(seed_array));
-    return g_rand_new_with_seed_array(seed_array, seed_idx);
+    nm_assert(seed_idx == SEED_ARRAY_SIZE);
 }
 
 static GRand *
@@ -74,27 +71,44 @@ _rand_create_thread_local(void)
 {
     G_LOCK_DEFINE_STATIC(global_rand);
     static GRand *global_rand = NULL;
-    guint32       seed_buffer[16];
-    gsize         i;
+    guint32       seed_array[SEED_ARRAY_SIZE];
 
-    /* For our global_rand instance we use a seed derived from getauxval(AT_RANDOM)
-     * or the PID, which don't change per process. So that is not a good source
-     * to initialize multiple GRand instances (for each thread).
+    /* We use thread-local instances of GRand to create a series of
+     * "random" numbers. We use thread-local instances, so that we don't
+     * require additional locking except the first time.
      *
-     * Instead, we use the random numbers generated by global_rand to
-     * seed the thread local instances. */
+     * We trust that once seeded, a GRand gives us a good enough stream of
+     * random numbers. If that wouldn't be the case, then maybe GRand should
+     * be fixed.
+     * Also, we tell our callers that the numbers from GRand are not good.
+     * But that isn't gonna help, because callers have no other way to get
+     * better random numbers, so usually the just ignore the failure and make
+     * the best of it.
+     *
+     * That means, the remaining problem is to seed the instance well.
+     * Note that we are already in a situation where getrandom() failed
+     * to give us good random numbers. So we can not do much to get reliably
+     * good entropy for the seed. */
 
     G_LOCK(global_rand);
 
-    if (G_UNLIKELY(!global_rand))
-        global_rand = _rand_create_global();
-
-    for (i = 0; i < G_N_ELEMENTS(seed_buffer); i++)
-        seed_buffer[i] = g_rand_int(global_rand);
+    if (G_UNLIKELY(!global_rand)) {
+        GRand *rand1;
+
+        /* g_rand_new() reads /dev/urandom, but we already noticed that
+         * /dev/urandom fails to give us good randomness (which is why
+         * we hit this code path). So this may not be as good as we wish,
+         * but let's add it to the mix. */
+        rand1 = g_rand_new();
+        _rand_init_seed(seed_array, rand1);
+        global_rand = g_rand_new_with_seed_array(seed_array, SEED_ARRAY_SIZE);
+        g_rand_free(rand1);
+    }
 
+    _rand_init_seed(seed_array, global_rand);
     G_UNLOCK(global_rand);
 
-    return g_rand_new_with_seed_array(seed_buffer, G_N_ELEMENTS(seed_buffer));
+    return g_rand_new_with_seed_array(seed_array, SEED_ARRAY_SIZE);
 }
 
 /**