From 483843c282c62ab8c81113578fd8163bcec238e1 Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@endlessos.org>
Date: Thu, 27 May 2021 13:46:36 +0100
Subject: [PATCH] gmodule: Add locking around dlerror() for some libc
 implementations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Specifically, for uclibc at the moment. Other implementations may need
locking, but I haven’t checked any aside from uclibc-ng and glibc.

POSIX.1-2001 specifies that `dlerror()` is not thread-safe, but the
glibc (and likely other libc) implementation is.

Issue #399 was originally reported about eglibc, but that project has
since died and been merged back into glibc. So that’s one less thing to
worry about.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #399
---
 gmodule/gmodule-dl.c | 48 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/gmodule/gmodule-dl.c b/gmodule/gmodule-dl.c
index 7fb30bd84..0d75e9d31 100644
--- a/gmodule/gmodule-dl.c
+++ b/gmodule/gmodule-dl.c
@@ -28,6 +28,7 @@
 #include "config.h"
 
 #include <dlfcn.h>
+#include <glib.h>
 
 /* Perl includes <nlist.h> and <link.h> instead of <dlfcn.h> on some systems? */
 
@@ -73,7 +74,42 @@
 #endif	/* RTLD_GLOBAL */
 
 
-/* --- functions --- */
+/* According to POSIX.1-2001, dlerror() is not necessarily thread-safe
+ * (see https://pubs.opengroup.org/onlinepubs/009695399/), and so must be
+ * called within the same locked section as the dlopen()/dlsym() call which
+ * may have caused an error.
+ *
+ * However, some libc implementations, such as glibc, implement dlerror() using
+ * thread-local storage, so are thread-safe. As of early 2021:
+ *  - glibc is thread-safe: https://github.com/bminor/glibc/blob/master/dlfcn/libc_dlerror_result.c
+ *  - uclibc-ng is not thread-safe: https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/ldso/libdl/libdl.c?id=132decd2a043d0ccf799f42bf89f3ae0c11e95d5#n1075
+ *  - Other libc implementations have not been checked, and no problems have
+ *    been reported with them in 10 years, so default to assuming that they
+ *    don’t need additional thread-safety from GLib
+ */
+#if defined(__UCLIBC__)
+G_LOCK_DEFINE_STATIC (errors);
+#else
+#define DLERROR_IS_THREADSAFE 1
+#endif
+
+static void
+lock_dlerror (void)
+{
+#ifndef DLERROR_IS_THREADSAFE
+  G_LOCK (errors);
+#endif
+}
+
+static void
+unlock_dlerror (void)
+{
+#ifndef DLERROR_IS_THREADSAFE
+  G_UNLOCK (errors);
+#endif
+}
+
+/* This should be called with lock_dlerror() held */
 static const gchar *
 fetch_dlerror (gboolean replace_null)
 {
@@ -95,10 +131,12 @@ _g_module_open (const gchar *file_name,
 {
   gpointer handle;
   
+  lock_dlerror ();
   handle = dlopen (file_name,
 		   (bind_local ? 0 : RTLD_GLOBAL) | (bind_lazy ? RTLD_LAZY : RTLD_NOW));
   if (!handle)
     g_module_set_error (fetch_dlerror (TRUE));
+  unlock_dlerror ();
   
   return handle;
 }
@@ -119,6 +157,7 @@ _g_module_self (void)
    * always returns 'undefined symbol'. Only if RTLD_DEFAULT or 
    * NULL is given, dlsym returns an appropriate pointer.
    */
+  lock_dlerror ();
 #if defined(__BIONIC__)
   handle = RTLD_DEFAULT;
 #else
@@ -126,6 +165,7 @@ _g_module_self (void)
 #endif
   if (!handle)
     g_module_set_error (fetch_dlerror (TRUE));
+  unlock_dlerror ();
   
   return handle;
 }
@@ -137,8 +177,10 @@ _g_module_close (gpointer handle)
   if (handle != RTLD_DEFAULT)
 #endif
     {
+      lock_dlerror ();
       if (dlclose (handle) != 0)
-	g_module_set_error (fetch_dlerror (TRUE));
+        g_module_set_error (fetch_dlerror (TRUE));
+      unlock_dlerror ();
     }
 }
 
@@ -149,11 +191,13 @@ _g_module_symbol (gpointer     handle,
   gpointer p;
   const gchar *msg;
 
+  lock_dlerror ();
   fetch_dlerror (FALSE);
   p = dlsym (handle, symbol_name);
   msg = fetch_dlerror (FALSE);
   if (msg)
     g_module_set_error (msg);
+  unlock_dlerror ();
   
   return p;
 }