Move setproctitle() automatic initialization to its own library

The automatic initialization cannot be part of the main shared library,
because there is no thread-safe way to change the environ global
variable. This is not a problem if the initializaion happens just at
program load time, but becomes one if the shared library is directly or
indirectly dlopen()ed during the execution of the program, which could
have either kept references to the old environ or could change it in
some other thread. This has been observed for example on systems using
Samba NSS modules.

To avoid any other possible fallout, the constructor is split into a
new static library that needs to be linked explicitly into programs
using setproctitle(). As an additional safety measure the pkg-config
linker flags will mark the program as not allowing to be dlopen()ed
so that we avoid the problem described above.

Reported-by: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=66679

9 files changed, 104 insertions, 11 deletions

diff --git a/configure.ac b/configure.ac
index a390d0c..dbad6f4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,7 @@ AC_CONFIG_FILES([
 	man/Makefile
 	src/Makefile
 	src/libbsd.pc
+	src/libbsd-ctor.pc
 	src/libbsd-overlay.pc
 	test/Makefile
 ])
diff --git a/include/bsd/unistd.h b/include/bsd/unistd.h
index 193dd35..5dfa605 100644
--- a/include/bsd/unistd.h
+++ b/include/bsd/unistd.h
@@ -56,6 +56,10 @@ void *setmode(const char *mode_str);
 
 void closefrom(int lowfd);
 
+/* Compatibility with sendmail implementations. */
+#define initsetproctitle(c, a, e) setproctitle_init((c), (a), (e))
+
+void setproctitle_init(int argc, char *argv[], char *envp[]);
 void setproctitle(const char *fmt, ...);
 
 int getpeereid(int s, uid_t *euid, gid_t *egid);
diff --git a/man/setproctitle.3 b/man/setproctitle.3
index d42498e..dd5df59 100644
--- a/man/setproctitle.3
+++ b/man/setproctitle.3
@@ -33,6 +33,8 @@
 .In sys/types.h
 .In bsd/unistd.h
 .Ft void
+.Fn setproctitle_init "int argc" "char *argv[]" "char *envp[]"
+.Ft void
 .Fn setproctitle "const char *fmt" "..."
 .Sh DESCRIPTION
 The
@@ -41,6 +43,15 @@ library routine sets the process title that appears on the
 .Xr ps 1
 command.
 .Pp
+The
+.Fn setproctitle_init
+library routine only needs to be called (before any call to
+.Fn setproctitle
+and with
+.Fn main
+arguments), if the automatic constructor support has not
+been linked in through the libbsd-ctor pkg-config file.
+.Pp
 The title is set from the executable's name, followed by the
 result of a
 .Xr printf 3
@@ -101,6 +112,11 @@ first appeared in
 .Fx 2.2 .
 Other operating systems have
 similar functions.
+.Pp
+The
+.Fn setproctitle_init
+function is a libbsd extension not present on the BSDs, avoid using it
+in portable code.
 .Sh AUTHORS
 .An -nosplit
 .An Peter Wemm Aq peter@FreeBSD.org
diff --git a/src/Makefile.am b/src/Makefile.am
index 96fee3e..46f8634 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -10,6 +10,7 @@ AM_CPPFLAGS = \
 EXTRA_DIST = \
 	libbsd.map \
 	libbsd.pc.in \
+	libbsd-ctor.pc.in \
 	libbsd-overlay.pc.in \
 	hash/helper.c \
 	$(nil)
@@ -21,10 +22,12 @@ CLEANFILES = \
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = \
 	libbsd.pc \
+	libbsd-ctor.pc \
 	libbsd-overlay.pc \
 	$(nil)
 
 lib_LTLIBRARIES = libbsd.la
+lib_LIBRARIES = libbsd-ctor.a
 
 hash/md5hl.c: $(srcdir)/hash/helper.c
 	$(AM_V_at) $(MKDIR_P) hash
@@ -75,6 +78,10 @@ libbsd_la_SOURCES = \
 	wcslcpy.c \
 	$(nil)
 
+libbsd_ctor_a_SOURCES = \
+	setproctitle_ctor.c \
+	$(nil)
+
 runtimelibdir = $(libdir)
 
 install-exec-hook:
diff --git a/src/libbsd-ctor.pc.in b/src/libbsd-ctor.pc.in
new file mode 100644
index 0000000..117b823
--- /dev/null
+++ b/src/libbsd-ctor.pc.in
@@ -0,0 +1,12 @@
+prefix=@prefix@
+exec_prefix=@exec_prefix@
+libdir=@libdir@
+includedir=@includedir@
+
+Name: libbsd-ctor
+Description: Automatic constructor functions for libbsd
+Version: @VERSION@
+URL: http://libbsd.freedesktop.org/
+Cflags: -I${includedir}
+Libs: -L${libdir} -Wl,-z,nodlopen -Wl,-u,libbsd_init_func -lbsd-ctor
+Requires: libbsd
diff --git a/src/libbsd.map b/src/libbsd.map
index 4067e2c..52df212 100644
--- a/src/libbsd.map
+++ b/src/libbsd.map
@@ -105,3 +105,8 @@ LIBBSD_0.5 {
     wcslcat;
     wcslcpy;
 } LIBBSD_0.4;
+
+LIBBSD_0.6 {
+    /* Exported to cope with the constructor+dlopen+threads mess. */
+    setproctitle_init;
+} LIBBSD_0.5;
diff --git a/src/setproctitle.c b/src/setproctitle.c
index 7e4b165..305b563 100644
--- a/src/setproctitle.c
+++ b/src/setproctitle.c
@@ -153,8 +153,8 @@ spt_copyargs(int argc, char *argv[])
 	return 0;
 }
 
-static void
-spt_init(int argc, char *argv[], char *envp[])
+void
+setproctitle_init(int argc, char *argv[], char *envp[])
 {
 	char *base, *end, *nul, *tmp;
 	int i, envc, error;
@@ -215,14 +215,6 @@ spt_init(int argc, char *argv[], char *envp[])
 	SPT.end  = end;
 }
 
-/*
- * Force spt_init() function into the .init_array section instead of expecting
- * either the compiler to place constructors there or the linker to move them
- * from .ctors to .init_array.
- */
-void (*spt_init_func)(int argc, char *argv[], char *envp[])
-	__attribute__((section(".init_array"))) = spt_init;
-
 #ifndef SPT_MAXTITLE
 #define SPT_MAXTITLE 255
 #endif
diff --git a/src/setproctitle_ctor.c b/src/setproctitle_ctor.c
new file mode 100644
index 0000000..9360774
--- /dev/null
+++ b/src/setproctitle_ctor.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2013 Guillem Jover <guillem@hadrons.org>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+ * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <unistd.h>
+
+/*
+ * The automatic initialization cannot be part of the main shared library,
+ * because there is no thread-safe way to change the environ global
+ * variable. This is not a problem if the initializaion happens just at
+ * program load time, but becomes one if the shared library is directly or
+ * indirectly dlopen()ed during the execution of the program, which could
+ * have either kept references to the old environ or could change it in
+ * some other thread. This has been observed for example on systems using
+ * Samba NSS modules.
+ *
+ * To avoid any other possible fallout, the constructor is split into a
+ * new static library that needs to be linked explicitly into programs
+ * using setproctitle(). As an additional safety measure the pkg-config
+ * linker flags will mark the program as not allowing to be dlopen()ed
+ * so that we make sure to avoid the problem described above.
+ */
+
+/*
+ * Force setproctitle_init() function into the .init_array section instead of
+ * expecting either the compiler to place constructors there or the linker to
+ * move them from .ctors to .init_array.
+ */
+void (*libbsd_init_func)(int argc, char *argv[], char *envp[])
+	__attribute__((section(".init_array"))) = setproctitle_init;
diff --git a/test/Makefile.am b/test/Makefile.am
index 73a1e91..d0e0f0b 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -18,6 +18,10 @@ check_PROGRAMS = \
 
 humanize_LDFLAGS = $(top_builddir)/src/libbsd.la
 fgetln_LDFLAGS = $(top_builddir)/src/libbsd.la
-proctitle_LDFLAGS = $(top_builddir)/src/libbsd.la
+proctitle_LDFLAGS = \
+	-Wl,-u,libbsd_init_func \
+	$(top_builddir)/src/libbsd-ctor.a \
+	$(top_builddir)/src/libbsd.la \
+	$(nil)
 
 TESTS = $(check_PROGRAMS)