Wrap unsafe libltdl calls with locking (#848)

* Wrap unsafe libltdl calls with locking

https://github.com/gphoto/libgphoto2/issues/166

* 848: do not install internal header gphoto2-port-mutex.h

* 848: wrap symbol declarations in #ifdef _GPHOTO2_INTERNAL_CODE

* 848: Use proper include guard macro

* 848: Encapsulate the lock/unlock code inside functions

This hides the pthread usage as an implementation detail inside the
gpi_libltdl_lock() and gpi_libltdl_unlock() functions.

* 848: Rename from *-mutex.[ch] to *-locking.[ch]

The *-lock.[ch] name is implementation neutral.

* 848: update libgphoto2_port version number

* 848: internal headers do not require __cplusplus checks

* 848: add copyright comment at the beginning of *.[ch]

* 848: add an unlocked variant of gp_abilities_list_load_dir to avoid lots of unlock/lock codes

* 848: remove unnecessary whitespace addition

Co-authored-by: kadler15 <spurfan15@gmail.com>
Co-authored-by: Hans Ulrich Niedermann <hun@n-dimensional.de>

11 files changed, 144 insertions, 10 deletions

diff --git a/libgphoto2/gphoto2-abilities-list.c b/libgphoto2/gphoto2-abilities-list.c
index b4a9725d1..7db617aed 100644
--- a/libgphoto2/gphoto2-abilities-list.c
+++ b/libgphoto2/gphoto2-abilities-list.c
@@ -36,6 +36,7 @@
 #include <gphoto2/gphoto2-result.h>
 #include <gphoto2/gphoto2-port-log.h>
 #include <gphoto2/gphoto2-library.h>
+#include <gphoto2/gphoto2-port-locking.h>
 
 #include "libgphoto2/i18n.h"
 
@@ -185,9 +186,8 @@ foreach_func (const char *filename, lt_ptr data)
 	return ((fd->result == GP_OK)?0:1);
 }
 
-
-int
-gp_abilities_list_load_dir (CameraAbilitiesList *list, const char *dir,
+static int
+unlocked_gp_abilities_list_load_dir (CameraAbilitiesList *list, const char *dir,
 			    GPContext *context)
 {
 	CameraLibraryIdFunc id;
@@ -295,7 +295,6 @@ gp_abilities_list_load_dir (CameraAbilitiesList *list, const char *dir,
 #if !defined(VALGRIND)
 		lt_dlclose (lh);
 #endif
-
 		new_count = gp_abilities_list_count (list);
 		if (new_count < 0)
 			continue;
@@ -310,16 +309,29 @@ gp_abilities_list_load_dir (CameraAbilitiesList *list, const char *dir,
 		if (gp_context_cancel (context) == GP_CONTEXT_FEEDBACK_CANCEL) {
 			lt_dlexit ();
 			gp_list_free (flist);
-			return (GP_ERROR_CANCEL);
+			return GP_ERROR_CANCEL;
 		}
 	}
 	gp_context_progress_stop (context, p);
 	lt_dlexit ();
 	gp_list_free (flist);
 
-	return (GP_OK);
+	return GP_OK;
 }
 
+int
+gp_abilities_list_load_dir (CameraAbilitiesList *list, const char *dir,
+			    GPContext *context)
+{
+	int ret;
+
+	gpi_libltdl_lock();
+	ret = unlocked_gp_abilities_list_load_dir (list, dir, context);
+	gpi_libltdl_unlock();
+	return ret;
+}
+
+
 
 /**
  * \brief Scans the system for camera drivers.
diff --git a/libgphoto2/gphoto2-camera.c b/libgphoto2/gphoto2-camera.c
index 91828df46..745d7f988 100644
--- a/libgphoto2/gphoto2-camera.c
+++ b/libgphoto2/gphoto2-camera.c
@@ -37,6 +37,7 @@
 #include <gphoto2/gphoto2-result.h>
 #include <gphoto2/gphoto2-library.h>
 #include <gphoto2/gphoto2-port-log.h>
+#include <gphoto2/gphoto2-port-locking.h>
 
 #include "libgphoto2/i18n.h"
 
@@ -279,8 +280,10 @@ gp_camera_exit (Camera *camera, GPContext *context)
 
 	if (camera->pc->lh) {
 #if !defined(VALGRIND)
+		gpi_libltdl_lock();
 		lt_dlclose (camera->pc->lh);
 		lt_dlexit ();
+		gpi_libltdl_unlock();
 #endif
 		camera->pc->lh = NULL;
 	}
@@ -777,21 +780,29 @@ gp_camera_init (Camera *camera, GPContext *context)
 
 	/* Load the library. */
 	GP_LOG_D ("Loading '%s'...", camera->pc->a.library);
+	gpi_libltdl_lock();
 	lt_dlinit ();
 	camera->pc->lh = lt_dlopenext (camera->pc->a.library);
+	gpi_libltdl_unlock();
 	if (!camera->pc->lh) {
+		gpi_libltdl_lock();
 		gp_context_error (context, _("Could not load required "
 			"camera driver '%s' (%s)."), camera->pc->a.library,
 			lt_dlerror ());
 		lt_dlexit ();
+		gpi_libltdl_unlock();
 		return (GP_ERROR_LIBRARY);
 	}
 
 	/* Initialize the camera */
+	gpi_libltdl_lock();
 	init_func = lt_dlsym (camera->pc->lh, "camera_init");
+	gpi_libltdl_unlock();
 	if (!init_func) {
+		gpi_libltdl_lock();
 		lt_dlclose (camera->pc->lh);
 		lt_dlexit ();
+		gpi_libltdl_unlock();
 		camera->pc->lh = NULL;
 		gp_context_error (context, _("Camera driver '%s' is "
 			"missing the 'camera_init' function."),
@@ -802,8 +813,10 @@ gp_camera_init (Camera *camera, GPContext *context)
 	if (strcasecmp (camera->pc->a.model, "Directory Browse")) {
 		result = gp_port_open (camera->port);
 		if (result < 0) {
+			gpi_libltdl_lock();
 			lt_dlclose (camera->pc->lh);
 			lt_dlexit ();
+			gpi_libltdl_unlock();
 			camera->pc->lh = NULL;
 			return (result);
 		}
@@ -812,8 +825,10 @@ gp_camera_init (Camera *camera, GPContext *context)
 	result = init_func (camera, context);
 	if (result < 0) {
 		gp_port_close (camera->port);
+		gpi_libltdl_lock();
 		lt_dlclose (camera->pc->lh);
 		lt_dlexit ();
+		gpi_libltdl_unlock();
 		camera->pc->lh = NULL;
 		memset (camera->functions, 0, sizeof (CameraFunctions));
 		return (result);
diff --git a/libgphoto2_port/Makefile.am b/libgphoto2_port/Makefile.am
index 69720ca2c..8f783057f 100644
--- a/libgphoto2_port/Makefile.am
+++ b/libgphoto2_port/Makefile.am
@@ -104,3 +104,4 @@ nobase_include_HEADERS =	\
 	gphoto2/gphoto2-port-result.h
 
 EXTRA_DIST += gphoto2/gphoto2-port-library.h
+EXTRA_DIST += gphoto2/gphoto2-port-locking.h
diff --git a/libgphoto2_port/NEWS b/libgphoto2_port/NEWS
index 32fca664f..986b330de 100644
--- a/libgphoto2_port/NEWS
+++ b/libgphoto2_port/NEWS
@@ -1,3 +1,6 @@
+libgphoto2_port 0.12.2
+  * internal API/ABI: Added gpi_libltdl_lock() and gpi_libltdl_unlock()
+
 libgphoto2_port 0.12.1
   * API:
     * Added function: `int gp_port_init_localedir(const char *localedir)`
diff --git a/libgphoto2_port/configure.ac b/libgphoto2_port/configure.ac
index ec52b7e62..2b52a093d 100644
--- a/libgphoto2_port/configure.ac
+++ b/libgphoto2_port/configure.ac
@@ -2,7 +2,7 @@ dnl Process this file with autoreconf to produce a configure script.
 AC_PREREQ([2.69])
 
 AC_INIT([libgphoto2 port access library],
-        [0.12.1],
+        [0.12.2],
         [gphoto-devel@lists.sourceforge.net],
         [libgphoto2_port])
 
@@ -59,10 +59,11 @@ dnl A:R:C=7:0:7 libgphoto2_port-0.7.0 libgphoto2-2.3
 dnl A:R:C=7:1:7 libgphoto2_port-0.7.1 libgphoto2-2.3.x
 dnl A:R:C=8:0:8 libgphoto2_port-0.8.0 libgphoto2-2.4.x
 dnl A:R:C=9:1:9 libgphoto2_port-0.10.0 libgphoto2-2.5.x
-dnl A:R:C=1:0:13  libgphoto2-2.5.XXX	(added gp_port_init_localedir ... )
-AC_SUBST([LIBGPHOTO2_PORT_AGE],      [1])
+dnl A:R:C=1:0:13  libgphoto2-2.5.30	(added gp_port_init_localedir ... )
+dnl A:R:C=2:0:14  libgphoto2-2.5.31	(added gpi_libltdl_mutex ... )
+AC_SUBST([LIBGPHOTO2_PORT_AGE],      [2])
 AC_SUBST([LIBGPHOTO2_PORT_REVISION], [0])
-AC_SUBST([LIBGPHOTO2_PORT_CURRENT],  [13])
+AC_SUBST([LIBGPHOTO2_PORT_CURRENT],  [14])
 AC_SUBST([LIBGPHOTO2_PORT_CURRENT_MIN],
          [`expr $LIBGPHOTO2_PORT_CURRENT - $LIBGPHOTO2_PORT_AGE`])
 AC_SUBST([LIBGPHOTO2_PORT_VERSION_INFO],
diff --git a/libgphoto2_port/gphoto2/gphoto2-port-locking.h b/libgphoto2_port/gphoto2/gphoto2-port-locking.h
new file mode 100644
index 000000000..a8b9db76f
--- /dev/null
+++ b/libgphoto2_port/gphoto2/gphoto2-port-locking.h
@@ -0,0 +1,40 @@
+/** \file
+ * \brief internal header to help with locking e.g. MT-unsafe libltdl
+ *
+ * \author Copyright 2017 Kris Adler <spurfan15@gmail.com>
+ * \author Copyright 2022 Marcus Meissner
+ * \author Copyright 2022 Hans Ulrich Niedermann
+ *
+ * \par License
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * \par
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * \par
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA  02110-1301  USA
+ */
+
+#ifndef LIBGPHOTO2_GPHOTO2_PORT_LOCKING_H
+#define LIBGPHOTO2_GPHOTO2_PORT_LOCKING_H
+
+#ifdef _GPHOTO2_INTERNAL_CODE
+
+/** lock libltdl before calling lt_*() (libltdl is not thread safe) */
+extern void gpi_libltdl_lock(void);
+
+/** unlock libltdl after calling lt_*() (libltdl is not thread safe) */
+extern void gpi_libltdl_unlock(void);
+
+#endif /* defined(_GPHOTO2_INTERNAL_CODE) */
+
+#endif /* defined(LIBGPHOTO2_GPHOTO2_PORT_LOCKING_H) */
diff --git a/libgphoto2_port/libgphoto2_port/Makefile.am b/libgphoto2_port/libgphoto2_port/Makefile.am
index 0119253e0..1e211ba34 100644
--- a/libgphoto2_port/libgphoto2_port/Makefile.am
+++ b/libgphoto2_port/libgphoto2_port/Makefile.am
@@ -66,11 +66,13 @@ libgphoto2_port_la_LIBADD       += $(INTLLIBS)
 
 libgphoto2_port_la_SOURCES      += gphoto2-port-info-list.c
 libgphoto2_port_la_SOURCES      += gphoto2-port-info.h
+libgphoto2_port_la_SOURCES      += gphoto2-port-locking.c
 libgphoto2_port_la_SOURCES      += gphoto2-port-log.c
 libgphoto2_port_la_SOURCES      += gphoto2-port-version.c
 libgphoto2_port_la_SOURCES      += gphoto2-port.c
 libgphoto2_port_la_SOURCES      += gphoto2-port-portability.c
 libgphoto2_port_la_SOURCES      += gphoto2-port-result.c
 
+libgphoto2_port_la_DEPENDENCIES += $(top_srcdir)/gphoto2/gphoto2-port-locking.h
 libgphoto2_port_la_DEPENDENCIES += $(top_srcdir)/gphoto2/gphoto2-port-version.h
 libgphoto2_port_la_DEPENDENCIES += $(top_srcdir)/gphoto2/gphoto2-port-library.h
diff --git a/libgphoto2_port/libgphoto2_port/gphoto2-port-info-list.c b/libgphoto2_port/libgphoto2_port/gphoto2-port-info-list.c
index e92004b47..9a3ea40a9 100644
--- a/libgphoto2_port/libgphoto2_port/gphoto2-port-info-list.c
+++ b/libgphoto2_port/libgphoto2_port/gphoto2-port-info-list.c
@@ -44,6 +44,7 @@
 #include <gphoto2/gphoto2-port-result.h>
 #include <gphoto2/gphoto2-port-library.h>
 #include <gphoto2/gphoto2-port-log.h>
+#include <gphoto2/gphoto2-port-locking.h>
 
 #include "libgphoto2_port/gphoto2-port-info.h"
 #include "libgphoto2_port/i18n.h"
@@ -315,10 +316,12 @@ gp_port_info_list_load (GPPortInfoList *list)
 	C_PARAMS (list);
 
 	GP_LOG_D ("Using ltdl to load io-drivers from '%s'...", iolibs);
+	gpi_libltdl_lock();
 	lt_dlinit ();
 	lt_dladdsearchdir (iolibs);
 	result = lt_dlforeachfile (iolibs, foreach_func, list);
 	lt_dlexit ();
+	gpi_libltdl_unlock();
 	if (result < 0)
 		return (result);
 	if (list->iolib_count == 0) {
diff --git a/libgphoto2_port/libgphoto2_port/gphoto2-port-locking.c b/libgphoto2_port/libgphoto2_port/gphoto2-port-locking.c
new file mode 100644
index 000000000..264c364f5
--- /dev/null
+++ b/libgphoto2_port/libgphoto2_port/gphoto2-port-locking.c
@@ -0,0 +1,42 @@
+/** \file
+ * \brief internal header to help with locking e.g. MT-unsafe libltdl
+ *
+ * \author Copyright 2017 kadler15 <spurfan15@gmail.com>
+ * \author Copyright 2022 Marcus Meissner
+ * \author Copyright 2022 Hans Ulrich Niedermann
+ *
+ * \par License
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * \par
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * \par
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA  02110-1301  USA
+ */
+
+#include <pthread.h>
+
+#include <gphoto2/gphoto2-port-locking.h>
+
+static
+pthread_mutex_t gpi_libltdl_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+void gpi_libltdl_lock(void)
+{
+  pthread_mutex_lock(&gpi_libltdl_mutex);
+}
+
+void gpi_libltdl_unlock(void)
+{
+  pthread_mutex_unlock(&gpi_libltdl_mutex);
+}
diff --git a/libgphoto2_port/libgphoto2_port/gphoto2-port.c b/libgphoto2_port/libgphoto2_port/gphoto2-port.c
index b5ed5f073..f481dff83 100644
--- a/libgphoto2_port/libgphoto2_port/gphoto2-port.c
+++ b/libgphoto2_port/libgphoto2_port/gphoto2-port.c
@@ -37,6 +37,7 @@
 #include <gphoto2/gphoto2-port-result.h>
 #include <gphoto2/gphoto2-port-library.h>
 #include <gphoto2/gphoto2-port-log.h>
+#include <gphoto2/gphoto2-port-locking.h>
 
 #include "libgphoto2_port/gphoto2-port-info.h"
 
@@ -159,26 +160,36 @@ gp_port_set_info (GPPort *port, GPPortInfo info)
 	}
 	if (port->pc->lh) {
 #if !defined(VALGRIND)
+		gpi_libltdl_lock();
 		lt_dlclose (port->pc->lh);
 		lt_dlexit ();
+		gpi_libltdl_unlock();
 #endif
 	}
 
+	gpi_libltdl_lock();
 	lt_dlinit ();
 	port->pc->lh = lt_dlopenext (info->library_filename);
+	gpi_libltdl_unlock();
 	if (!port->pc->lh) {
+		gpi_libltdl_lock();
 		GP_LOG_E ("Could not load '%s' ('%s').", info->library_filename, lt_dlerror ());
 		lt_dlexit ();
+		gpi_libltdl_unlock();
 		return (GP_ERROR_LIBRARY);
 	}
 
 	/* Load the operations */
+	gpi_libltdl_lock();
 	ops_func = lt_dlsym (port->pc->lh, "gp_port_library_operations");
+	gpi_libltdl_unlock();
 	if (!ops_func) {
+		gpi_libltdl_lock();
 		GP_LOG_E ("Could not find 'gp_port_library_operations' in '%s' ('%s')",
 			  info->library_filename, lt_dlerror ());
 		lt_dlclose (port->pc->lh);
 		lt_dlexit ();
+		gpi_libltdl_unlock();
 		port->pc->lh = NULL;
 		return (GP_ERROR_LIBRARY);
 	}
@@ -344,8 +355,10 @@ gp_port_free (GPPort *port)
 
 		if (port->pc->lh) {
 #if !defined(VALGRIND)
+			gpi_libltdl_lock();
 			lt_dlclose (port->pc->lh);
 			lt_dlexit ();
+			gpi_libltdl_unlock();
 #endif
 			port->pc->lh = NULL;
 		}
diff --git a/libgphoto2_port/libgphoto2_port/libgphoto2_port.ver b/libgphoto2_port/libgphoto2_port/libgphoto2_port.ver
index e8886fc54..651c7609a 100644
--- a/libgphoto2_port/libgphoto2_port/libgphoto2_port.ver
+++ b/libgphoto2_port/libgphoto2_port/libgphoto2_port.ver
@@ -79,6 +79,8 @@ LIBGPHOTO2_INTERNAL {
 	gpi_string_list_to_flags;
 	gpi_flags_to_string_list;
 	gpi_vsnprintf;
+	gpi_libltdl_lock;
+	gpi_libltdl_unlock;
 
 	gp_port_info_new;
 	gp_port_info_set_name;