glib-aux: merge branch 'th/thread-local-storage-destroy'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/914

Makefile.am

@@ -803,6 +803,7 @@ src_libnm_client_aux_extern_tests_test_libnm_client_aux_LDADD = \
 	src/libnm-core-aux-extern/libnm-core-aux-extern.la \
 	src/libnm-core-aux-intern/libnm-core-aux-intern.la \
 	src/libnm-glib-aux/libnm-glib-aux.la \
+	src/libnm-log-null/libnm-log-null.la \
 	src/libnm-std-aux/libnm-std-aux.la \
 	src/c-siphash/libc-siphash.la \
 	src/libnm-client-impl/libnm.la \
@@ -4584,6 +4585,7 @@ src_nm_dispatcher_tests_test_dispatcher_envp_LDFLAGS = \
 src_nm_dispatcher_tests_test_dispatcher_envp_LDADD = \
 	src/nm-dispatcher/libnm-dispatcher-core.la \
 	src/libnm-glib-aux/libnm-glib-aux.la \
+	src/libnm-log-null/libnm-log-null.la \
 	src/libnm-std-aux/libnm-std-aux.la \
 	src/c-siphash/libc-siphash.la \
 	src/libnm-client-impl/libnm.la \
@@ -4778,6 +4780,7 @@ src_libnmc_setting_tests_test_libnmc_setting_LDADD = \
 	src/libnm-core-aux-intern/libnm-core-aux-intern.la \
 	src/libnm-base/libnm-base.la \
 	src/libnm-glib-aux/libnm-glib-aux.la \
+	src/libnm-log-null/libnm-log-null.la \
 	src/libnm-std-aux/libnm-std-aux.la \
 	src/c-siphash/libc-siphash.la \
 	src/libnm-client-impl/libnm.la \

src/core/ppp/meson.build

@@ -14,6 +14,7 @@ nm_pppd_plugin = shared_module(
   ],
   link_with: [
     libnm_core_impl,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,

src/libnm-core-impl/tests/meson.build

@@ -32,8 +32,8 @@ foreach test_unit: test_units
       libnm_core_impl,
       libnm_crypto,
       libnm_base,
-      libnm_log_null,
       libnm_systemd_shared,
+      libnm_log_null,
       libnm_glib_aux,
       libnm_std_aux,
       libc_siphash,

src/libnm-glib-aux/nm-errno.c

@@ -7,8 +7,6 @@
 
 #include "nm-errno.h"
 
-#include <pthread.h>
-
 /*****************************************************************************/
 
 static NM_UTILS_LOOKUP_STR_DEFINE(
@@ -162,19 +160,9 @@ nm_strerror_native(int errsv)
 
     buf = buf_static;
     if (G_UNLIKELY(!buf)) {
-        int           errno_saved = errno;
-        pthread_key_t key;
-
         buf        = g_malloc(NM_STRERROR_BUFSIZE);
         buf_static = buf;
-
-        if (pthread_key_create(&key, g_free) != 0 || pthread_setspecific(key, buf) != 0) {
-            /* Failure. We will leak the buffer when the thread exits.
-             *
-             * Nothing we can do about it really. For Debug builds we fail with an assertion. */
-            nm_assert_not_reached();
-        }
-        errno = errno_saved;
+        nm_utils_thread_local_register_destroy(buf, g_free);
     }
 
     return nm_strerror_native_r(errsv, buf, NM_STRERROR_BUFSIZE);

src/libnm-glib-aux/nm-random-utils.c

@@ -8,6 +8,7 @@
 #include "nm-random-utils.h"
 
 #include <fcntl.h>
+#include <sys/auxv.h>
 
 #if USE_SYS_RANDOM_H
     #include <sys/random.h>
@@ -16,9 +17,100 @@
 #endif
 
 #include "nm-shared-utils.h"
+#include "nm-time-utils.h"
 
 /*****************************************************************************/
 
+#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)
+{
+    int           seed_idx;
+    const guint8 *p_at_random;
+    guint64       now_nsec;
+
+    /* Get some seed material from the provided GRand. */
+    for (seed_idx = 0; seed_idx < 16; seed_idx++)
+        seed_array[seed_idx] = g_rand_int(rand);
+
+    /* Add an address from the heap. */
+    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) {
+        memcpy(&seed_array[seed_idx], p_at_random, 16);
+    } 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++] = _pid_hash(getpid());
+    seed_array[seed_idx++] = _pid_hash(getppid());
+    seed_array[seed_idx++] = _pid_hash(gettid());
+
+    nm_assert(seed_idx == SEED_ARRAY_SIZE);
+}
+
+static GRand *
+_rand_create_thread_local(void)
+{
+    G_LOCK_DEFINE_STATIC(global_rand);
+    static GRand *global_rand = NULL;
+    guint32       seed_array[SEED_ARRAY_SIZE];
+
+    /* 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.
+     *
+     * 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)) {
+        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_array, SEED_ARRAY_SIZE);
+}
+
 /**
  * nm_utils_random_bytes:
  * @p: the buffer to fill
@@ -111,7 +203,8 @@ fd_open:
     }
 
     if (!urandom_success) {
-        static _nm_thread_local GRand *rand = NULL;
+        static _nm_thread_local GRand *rand_tls = NULL;
+        GRand *                        rand;
         gsize                          i;
         int                            j;
 
@@ -122,8 +215,12 @@ fd_open:
          */
         has_high_quality = FALSE;
 
-        if (G_UNLIKELY(!rand))
-            rand = g_rand_new();
+        rand = rand_tls;
+        if (G_UNLIKELY(!rand)) {
+            rand     = _rand_create_thread_local();
+            rand_tls = rand;
+            nm_utils_thread_local_register_destroy(rand, (GDestroyNotify) g_rand_free);
+        }
 
         nm_assert(n > 0);
         i = 0;

src/libnm-glib-aux/nm-shared-utils.c

@@ -14,7 +14,9 @@
 #include <sys/syscall.h>
 #include <net/if.h>
 #include <net/ethernet.h>
+#include <pthread.h>
 
+#include "c-list/src/c-list.h"
 #include "nm-errno.h"
 #include "nm-str-buf.h"
 
@@ -6385,3 +6387,80 @@ nm_utils_validate_hostname(const char *hostname)
 
     return (p - hostname <= HOST_NAME_MAX);
 }
+
+/*****************************************************************************/
+
+typedef struct {
+    CList          lst;
+    gpointer       tls_data;
+    GDestroyNotify destroy_notify;
+} TlsRegData;
+
+static pthread_key_t _tls_reg_key;
+
+static void
+_tls_reg_destroy(gpointer data)
+{
+    CList *     lst_head = data;
+    TlsRegData *entry;
+
+    if (!lst_head)
+        return;
+
+    /* For no strong reason are we destroying the elements in reverse
+     * order than they were added. It seems a bit more sensible (but shouldn't
+     * matter nor should you rely on that). */
+    while ((entry = c_list_last_entry(lst_head, TlsRegData, lst))) {
+        c_list_unlink_stale(&entry->lst);
+        entry->destroy_notify(entry->tls_data);
+        nm_g_slice_free(entry);
+    }
+
+    nm_g_slice_free(lst_head);
+}
+
+static void
+_tls_reg_make_key(void)
+{
+    if (pthread_key_create(&_tls_reg_key, _tls_reg_destroy) != 0)
+        g_return_if_reached();
+}
+
+/**
+ * nm_utils_thread_local_register_destroy:
+ * @tls_data: the thread local storage data that should be destroyed when the thread
+ *   exits. This pointer will be "owned" by the current thread. There is no way
+ *   to un-register the destruction.
+ * @destroy_notify: the free function that will be called when the thread exits.
+ *
+ * If _nm_tread_local storage is heap allocated it requires freeing the pointer
+ * when the thread exits. Use this function to register the pointer to be
+ * released.
+ *
+ * This function does not change errno.
+ */
+void
+nm_utils_thread_local_register_destroy(gpointer tls_data, GDestroyNotify destroy_notify)
+{
+    NM_AUTO_PROTECT_ERRNO(errsv);
+    static pthread_once_t key_once = PTHREAD_ONCE_INIT;
+    CList *               lst_head;
+    TlsRegData *          entry;
+
+    nm_assert(destroy_notify);
+
+    if (pthread_once(&key_once, _tls_reg_make_key) != 0)
+        g_return_if_reached();
+
+    if ((lst_head = pthread_getspecific(_tls_reg_key)) == NULL) {
+        lst_head = g_slice_new(CList);
+        c_list_init(lst_head);
+        if (pthread_setspecific(_tls_reg_key, lst_head) != 0)
+            g_return_if_reached();
+    }
+
+    entry                 = g_slice_new(TlsRegData);
+    entry->tls_data       = tls_data;
+    entry->destroy_notify = destroy_notify;
+    c_list_link_tail(lst_head, &entry->lst);
+}

src/libnm-glib-aux/nm-shared-utils.h

@@ -2994,4 +2994,8 @@ char *nm_utils_get_process_exit_status_desc(int status);
 
 gboolean nm_utils_validate_hostname(const char *hostname);
 
+/*****************************************************************************/
+
+void nm_utils_thread_local_register_destroy(gpointer tls_data, GDestroyNotify destroy_notify);
+
 #endif /* __NM_SHARED_UTILS_H__ */

src/libnm-platform/nmp-netns.c

@@ -11,7 +11,6 @@
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <pthread.h>
 
 #include "libnm-log-core/nm-logging.h"
 
@@ -151,20 +150,13 @@ _netns_stack_get_impl(void)
 {
     gs_unref_object NMPNetns *netns = NULL;
     gs_free_error GError *error     = NULL;
-    pthread_key_t         key;
     GArray *              s;
 
     s = g_array_new(FALSE, FALSE, sizeof(NetnsInfo));
     g_array_set_clear_func(s, _netns_stack_clear_cb);
     _netns_stack = s;
 
-    /* register a destructor function to cleanup the array. If we fail
-     * to do so, we will leak NMPNetns instances (and their file descriptor) when the
-     * thread exits. */
-    if (pthread_key_create(&key, (void (*)(void *)) g_array_unref) != 0)
-        _LOGE(NULL, "failure to initialize thread-local storage");
-    else if (pthread_setspecific(key, s) != 0)
-        _LOGE(NULL, "failure to set thread-local storage");
+    nm_utils_thread_local_register_destroy(s, (GDestroyNotify) g_array_unref);
 
     /* at the bottom of the stack we must try to create a netns instance
      * that we never pop. It's the base to which we need to return. */

src/libnmc-setting/tests/meson.build

@@ -13,6 +13,7 @@ exe = executable(
     libnm_core_aux_extern,
     libnm_core_aux_intern,
     libnm_base,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,

src/nm-cloud-setup/tests/meson.build

@@ -11,6 +11,7 @@ exe = executable(
   link_with: [
     libnm_cloud_setup_core,
     libnmc_base,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,

src/nm-dispatcher/meson.build

@@ -39,6 +39,7 @@ executable(
   link_with: [
     libnm_core_aux_extern,
     libnm_dispatcher_core,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,

src/nm-dispatcher/tests/meson.build

@@ -13,6 +13,7 @@ exe = executable(
   c_args: introspection_extra_cflags,
   link_with: [
     libnm_dispatcher_core,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,

src/nm-initrd-generator/meson.build

@@ -34,8 +34,8 @@ executable(
     libnm_platform,
     libnm_base,
     libnm_systemd_shared,
-    libnm_log_core,
     libnm_udev_aux,
+    libnm_log_core,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,

src/nm-online/meson.build

@@ -9,6 +9,7 @@ executable(
   ],
   link_with: [
     libnm_client_aux_extern,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,

src/nmcli/meson.build

@@ -35,6 +35,7 @@ executable(
     libnm_core_aux_extern,
     libnm_core_aux_intern,
     libnm_base,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,
@@ -61,6 +62,7 @@ generate_docs_nm_settings_nmcli = executable(
     libnm_core_aux_extern,
     libnm_core_aux_intern,
     libnm_base,
+    libnm_log_null,
     libnm_glib_aux,
     libnm_std_aux,
     libc_siphash,