[SCM] Socket Wrapper Repository - branch master updated
Andreas Schneider
asn at samba.org
Mon Dec 4 11:24:21 UTC 2017
The branch, master has been updated
via 9a23836 Bump version to 1.1.9
via 3f80411 tests: Add a thread deadlock reproducer
via 3547fdf swrap: Avoid symbol binding deadlocks during fork
via 570930c swrap: Bind all symbols during prepare
via 3d0f65f tests: Fix sa_socklen for sockaddr_in
via 11bb41a swrap: Whitespace fixes
from 230a8a0 Bump version to 1.1.8
https://git.samba.org/?p=socket_wrapper.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 9a23836b9789ae6405734e075fb440c3c06ad4ee
Author: Andreas Schneider <asn at samba.org>
Date: Mon Dec 4 10:19:38 2017 +0100
Bump version to 1.1.9
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 3f804113608f0c2f04a8212fbacf569b67dd0a19
Author: Andreas Schneider <asn at samba.org>
Date: Tue Nov 28 15:41:05 2017 +0100
tests: Add a thread deadlock reproducer
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 3547fdfc094f4588375cb7b1716461f8f328b643
Author: Andreas Schneider <asn at samba.org>
Date: Mon Nov 27 11:00:54 2017 +0100
swrap: Avoid symbol binding deadlocks during fork
If there is a signal handler defined, e.g. for SIGCHILD and this signal
handler is called during pthread_atfork() has just locked all mutexes
and we want to check if a symbol is bound, we end up in a deadlock.
(gdb) bt
0 __lll_lock_wait () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
1 0x00007fe48837aef5 in __GI___pthread_mutex_lock (mutex=0x7fe4889a71c0 <libc_symbol_binding_mutex>) at ../nptl/pthread_mutex_lock.c:80
2 0x00007fe48879b2f1 in libc_write (fd=14, buf=0x7ffdad6436e8, count=8) at /home/asn/workspace/projects/socket_wrapper/src/socket_wrapper.c:1070
3 0x00007fe4887a21cb in swrap_write (s=14, buf=0x7ffdad6436e8, len=8) at /home/asn/workspace/projects/socket_wrapper/src/socket_wrapper.c:5137
4 0x00007fe4887a2359 in write (s=14, buf=0x7ffdad6436e8, len=8) at /home/asn/workspace/projects/socket_wrapper/src/socket_wrapper.c:5171
5 0x00007fe48713e5f2 in tevent_common_wakeup_fd (fd=14) at ../lib/tevent/tevent.c:959
6 0x00007fe48713e61f in tevent_common_wakeup (ev=ev at entry=0x55d04922e280) at ../lib/tevent/tevent.c:975
7 0x00007fe487141d38 in tevent_common_signal_handler (signum=<optimized out>) at ../lib/tevent/tevent_signal.c:109
8 <signal handler called>
9 0x00007fe4844de081 in __libc_fork () at ../sysdeps/nptl/fork.c:135
10 0x000055d047e8ddb3 in smbd_accept_connection (ev=0x55d04922e280, fde=<optimized out>, flags=<optimized out>, private_data=<optimized out>)
at ../source3/smbd/server.c:992
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 570930cff3511054eeeea1a3e5e8155946b501ef
Author: Andreas Schneider <asn at samba.org>
Date: Fri Dec 1 16:05:07 2017 +0100
swrap: Bind all symbols during prepare
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 3d0f65fcb183a0fb0e6ad122d929189ce5c0a2aa
Author: Andreas Schneider <asn at samba.org>
Date: Tue Nov 28 10:15:07 2017 +0100
tests: Fix sa_socklen for sockaddr_in
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 11bb41a674a54cb0507b61270993902c0a5ff582
Author: Andreas Schneider <asn at samba.org>
Date: Tue Nov 14 09:03:24 2017 +0100
swrap: Whitespace fixes
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
-----------------------------------------------------------------------
Summary of changes:
CMakeLists.txt | 4 +-
ChangeLog | 3 +
src/socket_wrapper.c | 119 ++++++++++++++++++++++++++-------
tests/CMakeLists.txt | 8 ++-
tests/test_echo_tcp_bind.c | 4 +-
tests/test_fork_thread_deadlock.c | 98 +++++++++++++++++++++++++++
tests/{torture.h => thread_deadlock.c} | 78 +++++++++------------
7 files changed, 237 insertions(+), 77 deletions(-)
create mode 100644 tests/test_fork_thread_deadlock.c
copy tests/{torture.h => thread_deadlock.c} (56%)
Changeset truncated at 500 lines:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index f332cac..9de79b4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -8,7 +8,7 @@ set(APPLICATION_NAME ${PROJECT_NAME})
set(APPLICATION_VERSION_MAJOR "1")
set(APPLICATION_VERSION_MINOR "1")
-set(APPLICATION_VERSION_PATCH "8")
+set(APPLICATION_VERSION_PATCH "9")
set(APPLICATION_VERSION "${APPLICATION_VERSION_MAJOR}.${APPLICATION_VERSION_MINOR}.${APPLICATION_VERSION_PATCH}")
@@ -19,7 +19,7 @@ set(APPLICATION_VERSION "${APPLICATION_VERSION_MAJOR}.${APPLICATION_VERSION_MINO
# Increment AGE. Set REVISION to 0
# If the source code was changed, but there were no interface changes:
# Increment REVISION.
-set(LIBRARY_VERSION "0.1.8")
+set(LIBRARY_VERSION "0.1.9")
set(LIBRARY_SOVERSION "0")
# where to look first for cmake modules, before ${CMAKE_ROOT}/Modules/ is checked
diff --git a/ChangeLog b/ChangeLog
index cfbcbe7..50911b6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,9 @@
ChangeLog
==========
+version 1.1.9 (released 2017-12-04)
+ * Fixed thread - signal deadlock issue
+
version 1.1.8 (released 2017-10-13)
* Added support for openat()
* Added support for open64() and fopen64()
diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index ccbe67f..539d27d 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -671,37 +671,46 @@ static void *_swrap_bind_symbol(enum swrap_lib lib, const char *fn_name)
}
#define swrap_bind_symbol_libc(sym_name) \
- SWRAP_LOCK(libc_symbol_binding); \
if (swrap.libc.symbols._libc_##sym_name.obj == NULL) { \
- swrap.libc.symbols._libc_##sym_name.obj = \
- _swrap_bind_symbol(SWRAP_LIBC, #sym_name); \
- } \
- SWRAP_UNLOCK(libc_symbol_binding)
+ SWRAP_LOCK(libc_symbol_binding); \
+ if (swrap.libc.symbols._libc_##sym_name.obj == NULL) { \
+ swrap.libc.symbols._libc_##sym_name.obj = \
+ _swrap_bind_symbol(SWRAP_LIBC, #sym_name); \
+ } \
+ SWRAP_UNLOCK(libc_symbol_binding); \
+ }
#define swrap_bind_symbol_libsocket(sym_name) \
- SWRAP_LOCK(libc_symbol_binding); \
if (swrap.libc.symbols._libc_##sym_name.obj == NULL) { \
- swrap.libc.symbols._libc_##sym_name.obj = \
- _swrap_bind_symbol(SWRAP_LIBSOCKET, #sym_name); \
- } \
- SWRAP_UNLOCK(libc_symbol_binding)
+ SWRAP_LOCK(libc_symbol_binding); \
+ if (swrap.libc.symbols._libc_##sym_name.obj == NULL) { \
+ swrap.libc.symbols._libc_##sym_name.obj = \
+ _swrap_bind_symbol(SWRAP_LIBSOCKET, #sym_name); \
+ } \
+ SWRAP_UNLOCK(libc_symbol_binding); \
+ }
#define swrap_bind_symbol_libnsl(sym_name) \
- SWRAP_LOCK(libc_symbol_binding); \
if (swrap.libc.symbols._libc_##sym_name.obj == NULL) { \
- swrap.libc.symbols._libc_##sym_name.obj = \
- _swrap_bind_symbol(SWRAP_LIBNSL, #sym_name); \
- } \
- SWRAP_UNLOCK(libc_symbol_binding)
+ SWRAP_LOCK(libc_symbol_binding); \
+ if (swrap.libc.symbols._libc_##sym_name.obj == NULL) { \
+ swrap.libc.symbols._libc_##sym_name.obj = \
+ _swrap_bind_symbol(SWRAP_LIBNSL, #sym_name); \
+ } \
+ SWRAP_UNLOCK(libc_symbol_binding); \
+ }
-/*
- * IMPORTANT
+/****************************************************************************
+ * IMPORTANT
+ ****************************************************************************
*
- * Functions especially from libc need to be loaded individually, you can't load
- * all at once or gdb will segfault at startup. The same applies to valgrind and
- * has probably something todo with with the linker.
- * So we need load each function at the point it is called the first time.
- */
+ * Functions especially from libc need to be loaded individually, you can't
+ * load all at once or gdb will segfault at startup. The same applies to
+ * valgrind and has probably something todo with with the linker. So we need
+ * load each function at the point it is called the first time.
+ *
+ ****************************************************************************/
+
#ifdef HAVE_ACCEPT4
static int libc_accept4(int sockfd,
struct sockaddr *addr,
@@ -1077,6 +1086,59 @@ static ssize_t libc_writev(int fd, const struct iovec *iov, int iovcnt)
return swrap.libc.symbols._libc_writev.f(fd, iov, iovcnt);
}
+/* DO NOT call this function during library initialization! */
+static void swrap_bind_symbol_all(void)
+{
+#ifdef HAVE_ACCEPT4
+ swrap_bind_symbol_libsocket(accept4);
+#else
+ swrap_bind_symbol_libsocket(accept);
+#endif
+ swrap_bind_symbol_libsocket(bind);
+ swrap_bind_symbol_libc(close);
+ swrap_bind_symbol_libsocket(connect);
+ swrap_bind_symbol_libc(dup);
+ swrap_bind_symbol_libc(dup2);
+ swrap_bind_symbol_libc(fcntl);
+ swrap_bind_symbol_libc(fopen);
+#ifdef HAVE_FOPEN64
+ swrap_bind_symbol_libc(fopen64);
+#endif
+#ifdef HAVE_EVENTFD
+ swrap_bind_symbol_libc(eventfd);
+#endif
+ swrap_bind_symbol_libsocket(getpeername);
+ swrap_bind_symbol_libsocket(getsockname);
+ swrap_bind_symbol_libsocket(getsockopt);
+ swrap_bind_symbol_libc(ioctl);
+ swrap_bind_symbol_libsocket(listen);
+ swrap_bind_symbol_libc(open);
+#ifdef HAVE_OPEN64
+ swrap_bind_symbol_libc(open64);
+#endif
+ swrap_bind_symbol_libc(openat);
+ swrap_bind_symbol_libsocket(pipe);
+ swrap_bind_symbol_libc(read);
+ swrap_bind_symbol_libsocket(readv);
+ swrap_bind_symbol_libsocket(recv);
+ swrap_bind_symbol_libsocket(recvfrom);
+ swrap_bind_symbol_libsocket(recvmsg);
+ swrap_bind_symbol_libsocket(send);
+ swrap_bind_symbol_libsocket(sendmsg);
+ swrap_bind_symbol_libsocket(sendto);
+ swrap_bind_symbol_libsocket(setsockopt);
+#ifdef HAVE_SIGNALFD
+ swrap_bind_symbol_libsocket(signalfd);
+#endif
+ swrap_bind_symbol_libsocket(socket);
+ swrap_bind_symbol_libsocket(socketpair);
+#ifdef HAVE_TIMERFD_CREATE
+ swrap_bind_symbol_libc(timerfd_create);
+#endif
+ swrap_bind_symbol_libc(write);
+ swrap_bind_symbol_libsocket(writev);
+}
+
/*********************************************************
* SWRAP HELPER FUNCTIONS
*********************************************************/
@@ -1523,7 +1585,7 @@ static int convert_in_un_alloc(struct socket_info *si, const struct sockaddr *in
if (addr == 0) {
/* 0.0.0.0 */
- is_bcast = 0;
+ is_bcast = 0;
type = d_type;
iface = socket_wrapper_default_iface();
} else if (a_type && addr == 0xFFFFFFFF) {
@@ -2344,7 +2406,7 @@ static int swrap_pcap_get_fd(const char *fname)
if (fd != -1) {
struct swrap_file_hdr file_hdr;
file_hdr.magic = 0xA1B2C3D4;
- file_hdr.version_major = 0x0002;
+ file_hdr.version_major = 0x0002;
file_hdr.version_minor = 0x0004;
file_hdr.timezone = 0x00000000;
file_hdr.sigfigs = 0x00000000;
@@ -5835,6 +5897,15 @@ int pledge(const char *promises, const char *paths[])
static void swrap_thread_prepare(void)
{
+ /*
+ * This function should only be called here!!
+ *
+ * We bind all symobls to avoid deadlocks of the fork is
+ * interrupted by a signal handler using a symbol of this
+ * library.
+ */
+ swrap_bind_symbol_all();
+
SWRAP_LOCK_ALL;
}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 32a457e..b86cc84 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -37,7 +37,8 @@ set(SWRAP_TESTS
test_echo_udp_sendmsg_recvmsg
test_swrap_unit
test_max_sockets
- test_close_failure)
+ test_close_failure
+ test_fork_thread_deadlock)
if (HAVE_STRUCT_MSGHDR_MSG_CONTROL)
set(SWRAP_TESTS ${SWRAP_TESTS} test_sendmsg_recvmsg_fd)
@@ -60,3 +61,8 @@ foreach(_SWRAP_TEST ${SWRAP_TESTS})
ENVIRONMENT LD_PRELOAD=${SOCKET_WRAPPER_LOCATION})
endif()
endforeach()
+
+# test_fork_pthread
+add_library(thread_deadlock SHARED thread_deadlock.c)
+target_link_libraries(thread_deadlock ${CMAKE_THREAD_LIBS_INIT})
+target_link_libraries(test_fork_thread_deadlock thread_deadlock)
diff --git a/tests/test_echo_tcp_bind.c b/tests/test_echo_tcp_bind.c
index cde7e3f..796c362 100644
--- a/tests/test_echo_tcp_bind.c
+++ b/tests/test_echo_tcp_bind.c
@@ -143,7 +143,7 @@ static void test_bind_ipv4(void **state)
* Finally, success binding a new IPv4 address.
*/
addr_in = (struct torture_address) {
- .sa_socklen = sizeof(struct sockaddr_un),
+ .sa_socklen = sizeof(struct sockaddr_in),
.sa.in = (struct sockaddr_in) {
.sin_family = AF_INET,
},
@@ -156,7 +156,7 @@ static void test_bind_ipv4(void **state)
assert_return_code(rc, errno);
addr_in = (struct torture_address) {
- .sa_socklen = sizeof(struct sockaddr_un),
+ .sa_socklen = sizeof(struct sockaddr_in),
.sa.in = (struct sockaddr_in) {
.sin_family = AF_INET,
.sin_port = htons(torture_server_port()),
diff --git a/tests/test_fork_thread_deadlock.c b/tests/test_fork_thread_deadlock.c
new file mode 100644
index 0000000..6cd1ca0
--- /dev/null
+++ b/tests/test_fork_thread_deadlock.c
@@ -0,0 +1,98 @@
+#include "config.h"
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <errno.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+
+/*
+ * This reproduces and issue if we get a signal after the pthread_atfork()
+ * prepare function of socket wrapper has been called.
+ *
+ * The order how pthread_atfork() handlers are set up is:
+ * -> application
+ * -> preloaded libraries
+ * -> libraries
+ *
+ * We have a library called thread_deadlock.
+ *
+ * This library registers a thread_deadlock_prepare() function via
+ * pthread_atfork().
+ *
+ * So pthread_atfork() registers the prepare function in the follow order:
+ * -> swrap_thread_prepare()
+ * -> thread_deadlock_prepare()
+ *
+ * In this test we fork and the swrap_thread_prepare() locks the mutex for
+ * symbol binding.
+ * Then thread_deadlock_prepare() is called which sends a signal to the parent
+ * process of this test. The signal triggers the signal handler below.
+ *
+ * When we call write() in the signal handler, we will try to bind the libc symbol
+ * and want to lock the symbol binding mutex. As it is already locked we run into
+ * a deadlock.
+ */
+
+static void test_swrap_signal_handler(int signum)
+{
+ fprintf(stderr, "PID: %d, SIGNUM: %d\n", getpid(), signum);
+ write(1, "DEADLOCK?\n", 10);
+}
+
+static void test_swrap_fork_pthread(void **state)
+{
+ pid_t pid;
+ struct sigaction act = {
+ .sa_handler = test_swrap_signal_handler,
+ .sa_flags = 0,
+ };
+
+ (void)state; /* unused */
+
+ sigemptyset(&act.sa_mask);
+ sigaction(SIGUSR1, &act, NULL);
+
+ pid = fork();
+ assert_return_code(pid, errno);
+
+ /* child */
+ if (pid == 0) {
+ exit(0);
+ }
+
+ /* parent */
+ if (pid > 0) {
+ pid_t child_pid;
+ int wstatus = -1;
+
+ child_pid = waitpid(-1, &wstatus, 0);
+ assert_return_code(child_pid, errno);
+
+ assert_true(WIFEXITED(wstatus));
+
+ assert_int_equal(WEXITSTATUS(wstatus), 0);
+ }
+}
+
+int main(void)
+{
+ int rc;
+
+ const struct CMUnitTest swrap_tests[] = {
+ cmocka_unit_test(test_swrap_fork_pthread),
+ };
+
+ rc = cmocka_run_group_tests(swrap_tests, NULL, NULL);
+
+ return rc;
+}
diff --git a/tests/torture.h b/tests/thread_deadlock.c
similarity index 56%
copy from tests/torture.h
copy to tests/thread_deadlock.c
index 921195d..b72367f 100644
--- a/tests/torture.h
+++ b/tests/thread_deadlock.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) Andreas Schneider 2013 <asn at samba.org>
+ * Copyright (C) 2017 Andreas Schneider <asn at samba.org>
*
* All rights reserved.
*
@@ -29,60 +29,42 @@
* 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.
+ *
*/
-#ifndef _TORTURE_H
-#define _TORTURE_H
-
#include "config.h"
-#include <sys/socket.h>
-#include <sys/un.h>
-#include <netinet/in.h>
-
-#include <stdarg.h>
-#include <stddef.h>
-#include <setjmp.h>
-#include <cmocka.h>
-
-#include <stdint.h>
-#include <string.h>
-
-struct torture_address {
- socklen_t sa_socklen;
- union {
- struct sockaddr s;
- struct sockaddr_in in;
-#ifdef HAVE_IPV6
- struct sockaddr_in6 in6;
-#endif
- struct sockaddr_un un;
- struct sockaddr_storage ss;
- } sa;
-};
-
-struct torture_state {
- char *socket_dir;
- char *pcap_file;
- char *srv_pidfile;
-};
+#include <stdio.h>
+#include <signal.h>
+#include <pthread.h>
-#ifndef ZERO_STRUCT
-#define ZERO_STRUCT(x) memset((char *)&(x), 0, sizeof(x))
-#endif
+#ifdef HAVE_CONSTRUCTOR_ATTRIBUTE
+#define CONSTRUCTOR_ATTRIBUTE __attribute__ ((constructor))
+#else
+#define CONSTRUCTOR_ATTRIBUTE
+#endif /* HAVE_CONSTRUCTOR_ATTRIBUTE */
-const char *torture_server_address(int domain);
-int torture_server_port(void);
+void thread_deadlock_constructor(void) CONSTRUCTOR_ATTRIBUTE;
-void torture_setup_socket_dir(void **state);
-void torture_setup_echo_srv_udp_ipv4(void **state);
-void torture_setup_echo_srv_udp_ipv6(void **state);
+static void thread_deadlock_prepare(void)
+{
+ pthread_kill(pthread_self(), SIGUSR1);
+ return;
+}
-void torture_setup_echo_srv_tcp_ipv4(void **state);
-void torture_setup_echo_srv_tcp_ipv6(void **state);
+static void thread_deadlock_parent(void)
+{
+ return;
+}
-void torture_teardown_socket_dir(void **state);
-void torture_teardown_echo_srv(void **state);
+static void thread_deadlock_child(void)
+{
+ return;
+}
-void torture_generate_random_buffer(uint8_t *out, int len);
-#endif /* _TORTURE_H */
+void thread_deadlock_constructor(void)
+{
+ pthread_atfork(&thread_deadlock_prepare,
+ &thread_deadlock_parent,
+ &thread_deadlock_child);
+}
--
Socket Wrapper Repository
More information about the samba-cvs
mailing list