[PATCH] Add thread safety to socket_wrapper

Andreas Schneider asn at samba.org
Thu Feb 22 10:28:30 UTC 2018


Hi,

the attached patchset adds thread safety support to socket_wrapper. It is the 
ground work for fd passing to test multichannel and smbdirect in selftest.

It would be great to get additional review. If I don't hear anything till next 
week Wednesday I'm going to push it.


Thanks,


	Andreas


-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From cf75487814478761f42a24e7040dadfee6b0dd29 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 13 Jul 2017 02:40:11 +0200
Subject: [PATCH 01/23] swrap: Make early-libc-out more obvious by removing
 else

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 02fe970..507f2b2 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -4016,7 +4016,9 @@ static int swrap_setsockopt(int s, int level, int optname,
 				       optname,
 				       optval,
 				       optlen);
-	} else if (level == IPPROTO_TCP) {
+	}
+
+	if (level == IPPROTO_TCP) {
 		switch (optname) {
 #ifdef TCP_NODELAY
 		case TCP_NODELAY: {
-- 
2.16.1


>From e4597590e5d12ace3912f04e662193e4faa1b6ea Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 12 Jul 2017 17:41:07 +0200
Subject: [PATCH 02/23] swrap: Introduce macro SOCKET_INFO_GET

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 507f2b2..2a4e3c4 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -185,6 +185,8 @@ enum swrap_dbglvl_e {
 # define SWRAP_UNLOCK_ALL \
 	SWRAP_UNLOCK(libc_symbol_binding); \
 
+#define SOCKET_INFO_GET(i) \
+	(struct socket_info *)(&sockets[i])
 
 #define SWRAP_DLIST_ADD(list,item) do { \
 	if (!(list)) { \
@@ -1748,7 +1750,7 @@ static struct socket_info *find_socket_info(int fd)
 		return NULL;
 	}
 
-	return &sockets[idx];
+	return SOCKET_INFO_GET(idx);
 }
 
 #if 0 /* FIXME */
@@ -1777,7 +1779,7 @@ static bool check_addr_port_in_use(const struct sockaddr *sa, socklen_t len)
 	}
 
 	for (f = socket_fds; f; f = f->next) {
-		struct socket_info *s = &sockets[f->si_index];
+		struct socket_info *s = SOCKET_INFO_GET(f->si_index);
 
 		if (s == last_s) {
 			continue;
@@ -1859,7 +1861,7 @@ static void swrap_remove_stale(int fd)
 	SWRAP_DLIST_REMOVE(socket_fds, fi);
 	free(fi);
 
-	si = &sockets[si_index];
+	si = SOCKET_INFO_GET(si_index);
 	si->refcount--;
 
 	if (si->refcount > 0) {
@@ -2872,7 +2874,7 @@ static int swrap_socket(int family, int type, int protocol)
 		return -1;
 	}
 
-	si = &sockets[idx];
+	si = SOCKET_INFO_GET(idx);
 
 	si->family = family;
 
@@ -3084,7 +3086,7 @@ static int swrap_accept(int s,
 		return -1;
 	}
 
-	child_si = &sockets[idx];
+	child_si = SOCKET_INFO_GET(idx);
 
 	child_fi = (struct socket_info_fd *)calloc(1, sizeof(struct socket_info_fd));
 	if (child_fi == NULL) {
@@ -5657,7 +5659,7 @@ static int swrap_close(int fd)
 
 	ret = libc_close(fd);
 
-	si = &sockets[si_index];
+	si = SOCKET_INFO_GET(si_index);
 	si->refcount--;
 
 	if (si->refcount > 0) {
@@ -5703,7 +5705,7 @@ static int swrap_dup(int fd)
 		return libc_dup(fd);
 	}
 
-	si = &sockets[src_fi->si_index];
+	si = SOCKET_INFO_GET(src_fi->si_index);
 
 	fi = (struct socket_info_fd *)calloc(1, sizeof(struct socket_info_fd));
 	if (fi == NULL) {
@@ -5748,7 +5750,7 @@ static int swrap_dup2(int fd, int newfd)
 		return libc_dup2(fd, newfd);
 	}
 
-	si = &sockets[src_fi->si_index];
+	si = SOCKET_INFO_GET(src_fi->si_index);
 
 	if (fd == newfd) {
 		/*
@@ -5810,7 +5812,7 @@ static int swrap_vfcntl(int fd, int cmd, va_list va)
 		return libc_vfcntl(fd, cmd, va);
 	}
 
-	si = &sockets[src_fi->si_index];
+	si = SOCKET_INFO_GET(src_fi->si_index);
 
 	switch (cmd) {
 	case F_DUPFD:
-- 
2.16.1


>From da4a990aff5d50cd651584ade5a684fda082b2b6 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 12 Jul 2017 18:19:39 +0200
Subject: [PATCH 03/23] swrap: Introduce macros to treat refcount

- SOCKET_INFO_GET_REFCOUNT
- SOCKET_INFO_INC_REFCOUNT
- SOCKET_INFO_DEC_REFCOUNT

to avoid directly dereferencing refcount.

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 2a4e3c4..d297fcc 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -185,6 +185,17 @@ enum swrap_dbglvl_e {
 # define SWRAP_UNLOCK_ALL \
 	SWRAP_UNLOCK(libc_symbol_binding); \
 
+#define SOCKET_INFO_GET_REFCOUNT(si) \
+	(si)->refcount
+
+#define SOCKET_INFO_INC_REFCOUNT(si) do { \
+	 SOCKET_INFO_GET_REFCOUNT(si) += 1; \
+} while(0)
+
+#define SOCKET_INFO_DEC_REFCOUNT(si) do { \
+	SOCKET_INFO_GET_REFCOUNT(si) -= 1; \
+} while(0)
+
 #define SOCKET_INFO_GET(i) \
 	(struct socket_info *)(&sockets[i])
 
@@ -1862,9 +1873,9 @@ static void swrap_remove_stale(int fd)
 	free(fi);
 
 	si = SOCKET_INFO_GET(si_index);
-	si->refcount--;
+	SOCKET_INFO_DEC_REFCOUNT(si);
 
-	if (si->refcount > 0) {
+	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
 		return;
 	}
 
@@ -2917,7 +2928,11 @@ static int swrap_socket(int family, int type, int protocol)
 		return -1;
 	}
 
-	si->refcount = 1;
+	/*
+	 * note: as side-effect, socket_wrapper_first_free_index
+	 * zeroed the si, so we are starting from refcount 0
+	 */
+	SOCKET_INFO_INC_REFCOUNT(si);
 	first_free = si->next_free;
 	si->next_free = 0;
 
@@ -3147,7 +3162,7 @@ static int swrap_accept(int s,
 	};
 	memcpy(&child_si->myname.sa.ss, &in_my_addr.sa.ss, in_my_addr.sa_socklen);
 
-	child_si->refcount = 1;
+	SOCKET_INFO_INC_REFCOUNT(child_si);
 	first_free = child_si->next_free;
 	child_si->next_free = 0;
 
@@ -5660,9 +5675,9 @@ static int swrap_close(int fd)
 	ret = libc_close(fd);
 
 	si = SOCKET_INFO_GET(si_index);
-	si->refcount--;
+	SOCKET_INFO_DEC_REFCOUNT(si);
 
-	if (si->refcount > 0) {
+	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
 		/* there are still references left */
 		return ret;
 	}
@@ -5721,7 +5736,7 @@ static int swrap_dup(int fd)
 		return -1;
 	}
 
-	si->refcount++;
+	SOCKET_INFO_INC_REFCOUNT(si);
 	fi->si_index = src_fi->si_index;
 
 	/* Make sure we don't have an entry for the fd */
@@ -5782,7 +5797,7 @@ static int swrap_dup2(int fd, int newfd)
 		return -1;
 	}
 
-	si->refcount++;
+	SOCKET_INFO_INC_REFCOUNT(si);
 	fi->si_index = src_fi->si_index;
 
 	/* Make sure we don't have an entry for the fd */
@@ -5830,7 +5845,7 @@ static int swrap_vfcntl(int fd, int cmd, va_list va)
 			return -1;
 		}
 
-		si->refcount++;
+		SOCKET_INFO_INC_REFCOUNT(si);
 		fi->si_index = src_fi->si_index;
 
 		/* Make sure we don't have an entry for the fd */
-- 
2.16.1


>From 008c088bc5ff6ad19ea8726a0916fcfaf019ada8 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 13 Jul 2017 01:01:57 +0200
Subject: [PATCH 04/23] swrap: Introduce macros to treat next_free

- SOCKET_INFO_GET_NEXT_FREE
- SOCKET_INFO_SET_NEXT_FREE

to avoid accessing socket_info.next_free directly

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index d297fcc..8b2689b 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -196,6 +196,13 @@ enum swrap_dbglvl_e {
 	SOCKET_INFO_GET_REFCOUNT(si) -= 1; \
 } while(0)
 
+#define SOCKET_INFO_GET_NEXT_FREE(si) \
+	(si)->next_free
+
+#define SOCKET_INFO_SET_NEXT_FREE(si, next_free) do { \
+	SOCKET_INFO_GET_NEXT_FREE(si) = (next_free); \
+} while (0)
+
 #define SOCKET_INFO_GET(i) \
 	(struct socket_info *)(&sockets[i])
 
@@ -1309,10 +1316,10 @@ static void socket_wrapper_init_sockets(void)
 	first_free = 0;
 
 	for (i = 0; i < max_sockets; i++) {
-		sockets[i].next_free = i+1;
+		SOCKET_INFO_SET_NEXT_FREE(&sockets[i], i+1);
 	}
 
-	sockets[max_sockets-1].next_free = -1;
+	SOCKET_INFO_SET_NEXT_FREE(&sockets[max_sockets-1], -1);
 }
 
 bool socket_wrapper_enabled(void)
@@ -1355,9 +1362,9 @@ static int socket_wrapper_first_free_index(void)
 		return -1;
 	}
 
-	next_free = sockets[first_free].next_free;
+	next_free = SOCKET_INFO_GET_NEXT_FREE(&sockets[first_free]);
 	ZERO_STRUCT(sockets[first_free]);
-	sockets[first_free].next_free = next_free;
+	SOCKET_INFO_SET_NEXT_FREE(&sockets[first_free], next_free);
 
 	return first_free;
 }
@@ -1883,7 +1890,7 @@ static void swrap_remove_stale(int fd)
 		unlink(si->un_addr.sun_path);
 	}
 
-	si->next_free = first_free;
+	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
 	first_free = si_index;
 }
 
@@ -2933,8 +2940,8 @@ static int swrap_socket(int family, int type, int protocol)
 	 * zeroed the si, so we are starting from refcount 0
 	 */
 	SOCKET_INFO_INC_REFCOUNT(si);
-	first_free = si->next_free;
-	si->next_free = 0;
+	first_free = SOCKET_INFO_GET_NEXT_FREE(si);
+	SOCKET_INFO_SET_NEXT_FREE(si, 0);
 
 	fi->fd = fd;
 	fi->si_index = idx;
@@ -3163,8 +3170,8 @@ static int swrap_accept(int s,
 	memcpy(&child_si->myname.sa.ss, &in_my_addr.sa.ss, in_my_addr.sa_socklen);
 
 	SOCKET_INFO_INC_REFCOUNT(child_si);
-	first_free = child_si->next_free;
-	child_si->next_free = 0;
+	first_free = SOCKET_INFO_GET_NEXT_FREE(child_si);
+	SOCKET_INFO_SET_NEXT_FREE(child_si, 0);
 
 	child_fi->si_index = idx;
 
@@ -5695,7 +5702,7 @@ static int swrap_close(int fd)
 		unlink(si->un_addr.sun_path);
 	}
 
-	si->next_free = first_free;
+	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
 	first_free = si_index;
 
 	return ret;
-- 
2.16.1


>From a0ccbeb44ead36970470a0b0e4846557a88ae10e Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 18 Jul 2017 12:50:15 +0200
Subject: [PATCH 05/23] swrap: set errno to ENFILE if there is no more free
 socket_info

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c     | 3 +--
 tests/test_max_sockets.c | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 8b2689b..9c2dbf5 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -1359,6 +1359,7 @@ static int socket_wrapper_first_free_index(void)
 	int next_free;
 
 	if (first_free == -1) {
+		errno = ENFILE;
 		return -1;
 	}
 
@@ -2888,7 +2889,6 @@ static int swrap_socket(int family, int type, int protocol)
 
 	idx = socket_wrapper_first_free_index();
 	if (idx == -1) {
-		errno = ENOMEM;
 		return -1;
 	}
 
@@ -3104,7 +3104,6 @@ static int swrap_accept(int s,
 
 	idx = socket_wrapper_first_free_index();
 	if (idx == -1) {
-		errno = ENOMEM;
 		return -1;
 	}
 
diff --git a/tests/test_max_sockets.c b/tests/test_max_sockets.c
index 0bc694b..00aa79b 100644
--- a/tests/test_max_sockets.c
+++ b/tests/test_max_sockets.c
@@ -63,7 +63,7 @@ static void test_max_sockets(void **state)
 	/* no free space for sockets left */
 	rc = _socket(&s[MAX_SOCKETS]);
 	assert_int_equal(rc, -1);
-	assert_int_equal(errno, ENOMEM);
+	assert_int_equal(errno, ENFILE);
 
 	/* closing a socket frees up space */
 	close(s[0]);
@@ -73,7 +73,7 @@ static void test_max_sockets(void **state)
 	/* but just one */
 	rc = _socket(&s[MAX_SOCKETS]);
 	assert_int_equal(rc, -1);
-	assert_int_equal(errno, ENOMEM);
+	assert_int_equal(errno, ENFILE);
 
 	for (i = 0; i < MAX_SOCKETS; i++) {
 		close(s[i]);
-- 
2.16.1


>From f4b4710dca5c8601d7326d65546f0a153f604712 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 18 Jul 2017 12:53:03 +0200
Subject: [PATCH 06/23] swrap: Use SOCKET_INFO_GET inside
 socket_wrapper_first_free_index

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 9c2dbf5..0c1fcc0 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -1356,6 +1356,7 @@ static unsigned int socket_wrapper_default_iface(void)
  */
 static int socket_wrapper_first_free_index(void)
 {
+	struct socket_info *si = NULL;
 	int next_free;
 
 	if (first_free == -1) {
@@ -1363,9 +1364,10 @@ static int socket_wrapper_first_free_index(void)
 		return -1;
 	}
 
-	next_free = SOCKET_INFO_GET_NEXT_FREE(&sockets[first_free]);
-	ZERO_STRUCT(sockets[first_free]);
-	SOCKET_INFO_SET_NEXT_FREE(&sockets[first_free], next_free);
+	si = SOCKET_INFO_GET(first_free);
+	next_free = SOCKET_INFO_GET_NEXT_FREE(si);
+	ZERO_STRUCTP(si);
+	SOCKET_INFO_SET_NEXT_FREE(si, next_free);
 
 	return first_free;
 }
-- 
2.16.1


>From a8f70b8582de15662413dc21b42b45176b30d385 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 2 Aug 2017 13:56:09 +0200
Subject: [PATCH 07/23] swrap: Reorder code inside swrap_socket

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 0c1fcc0..238153c 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -2807,8 +2807,10 @@ int signalfd(int fd, const sigset_t *mask, int flags)
 
 static int swrap_socket(int family, int type, int protocol)
 {
-	struct socket_info *si;
-	struct socket_info_fd *fi;
+	struct socket_info *si = NULL;
+	struct socket_info _si = { 0 };
+	struct socket_info *free_si = NULL;
+	struct socket_info_fd *fi = NULL;
 	int fd;
 	int idx;
 	int real_type = type;
@@ -2889,13 +2891,7 @@ static int swrap_socket(int family, int type, int protocol)
 	/* Check if we have a stale fd and remove it */
 	swrap_remove_stale(fd);
 
-	idx = socket_wrapper_first_free_index();
-	if (idx == -1) {
-		return -1;
-	}
-
-	si = SOCKET_INFO_GET(idx);
-
+	si = &_si;
 	si->family = family;
 
 	/* however, the rest of the socket_wrapper code expects just
@@ -2937,13 +2933,19 @@ static int swrap_socket(int family, int type, int protocol)
 		return -1;
 	}
 
-	/*
-	 * note: as side-effect, socket_wrapper_first_free_index
-	 * zeroed the si, so we are starting from refcount 0
-	 */
-	SOCKET_INFO_INC_REFCOUNT(si);
-	first_free = SOCKET_INFO_GET_NEXT_FREE(si);
-	SOCKET_INFO_SET_NEXT_FREE(si, 0);
+	idx = socket_wrapper_first_free_index();
+	if (idx == -1) {
+		return -1;
+	}
+
+	free_si = SOCKET_INFO_GET(idx);
+
+	first_free = SOCKET_INFO_GET_NEXT_FREE(free_si);
+
+	*free_si = _si;
+
+	SOCKET_INFO_INC_REFCOUNT(free_si);
+	SOCKET_INFO_SET_NEXT_FREE(free_si, 0);
 
 	fi->fd = fd;
 	fi->si_index = idx;
-- 
2.16.1


>From 80b7154d98fc1e2041a3988d7a3938f2b93e7ff1 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sat, 15 Jul 2017 14:40:07 +0530
Subject: [PATCH 08/23] swrap: Internal reorganization of core socket_info
 structures

The change basically splits socket_info structure into
two structures, namely,
    - socket_info: to store the core information
      corresponding to a socket entry.
    - socket_info_container: wrapping structure to hold
      the socket_info data as well as metadata(currently
      refcount and next_free).

Pair-Programmed-With: Anoop C S <anoopcs at redhat.com>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 238153c..be6f32a 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -185,8 +185,11 @@ enum swrap_dbglvl_e {
 # define SWRAP_UNLOCK_ALL \
 	SWRAP_UNLOCK(libc_symbol_binding); \
 
+#define SOCKET_INFO_CONTAINER(si) \
+	(struct socket_info_container *)(si)
+
 #define SOCKET_INFO_GET_REFCOUNT(si) \
-	(si)->refcount
+	(SOCKET_INFO_CONTAINER(si))->refcount
 
 #define SOCKET_INFO_INC_REFCOUNT(si) do { \
 	 SOCKET_INFO_GET_REFCOUNT(si) += 1; \
@@ -197,14 +200,14 @@ enum swrap_dbglvl_e {
 } while(0)
 
 #define SOCKET_INFO_GET_NEXT_FREE(si) \
-	(si)->next_free
+	(SOCKET_INFO_CONTAINER(si))->next_free
 
 #define SOCKET_INFO_SET_NEXT_FREE(si, next_free) do { \
 	SOCKET_INFO_GET_NEXT_FREE(si) = (next_free); \
 } while (0)
 
 #define SOCKET_INFO_GET(i) \
-	(struct socket_info *)(&sockets[i])
+	(struct socket_info *)(&(sockets[i].info))
 
 #define SWRAP_DLIST_ADD(list,item) do { \
 	if (!(list)) { \
@@ -322,10 +325,6 @@ int first_free;
 
 struct socket_info
 {
-	unsigned int refcount;
-
-	int next_free;
-
 	int family;
 	int type;
 	int protocol;
@@ -350,7 +349,14 @@ struct socket_info
 	} io;
 };
 
-static struct socket_info *sockets;
+struct socket_info_container
+{
+	struct socket_info info;
+	unsigned int refcount;
+	int next_free;
+};
+
+static struct socket_info_container *sockets;
 static size_t max_sockets = 0;
 
 /*
@@ -1304,8 +1310,8 @@ static void socket_wrapper_init_sockets(void)
 
 	max_sockets = socket_wrapper_max_sockets();
 
-	sockets = (struct socket_info *)calloc(max_sockets,
-					       sizeof(struct socket_info));
+	sockets = (struct socket_info_container *)calloc(max_sockets,
+					sizeof(struct socket_info_container));
 
 	if (sockets == NULL) {
 		SWRAP_LOG(SWRAP_LOG_ERROR,
@@ -1319,6 +1325,7 @@ static void socket_wrapper_init_sockets(void)
 		SOCKET_INFO_SET_NEXT_FREE(&sockets[i], i+1);
 	}
 
+	/* mark the end of the free list */
 	SOCKET_INFO_SET_NEXT_FREE(&sockets[max_sockets-1], -1);
 }
 
-- 
2.16.1


>From 26fad4ff44819d4e4cc06c22b1b9eefce99b4542 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Tue, 29 Aug 2017 12:40:35 +0530
Subject: [PATCH 09/23] swrap: Add new routines to handle socket creation

A new function named swrap_create_socket is introduced which
cleanly performs all stuff related to creation of new socket
file descriptors and updation of relevant metadata including
the free-list and reference counter.

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index be6f32a..81d3d52 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -1379,6 +1379,56 @@ static int socket_wrapper_first_free_index(void)
 	return first_free;
 }
 
+static int swrap_add_socket_info(struct socket_info *si_input)
+{
+	struct socket_info *si = NULL;
+	int si_index;
+
+	if (si_input == NULL) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	if (first_free == -1) {
+		errno = ENFILE;
+		return -1;
+	}
+
+	si_index = first_free;
+	si = SOCKET_INFO_GET(si_index);
+
+	first_free = SOCKET_INFO_GET_NEXT_FREE(si);
+	*si = *si_input;
+	SOCKET_INFO_INC_REFCOUNT(si);
+
+	return si_index;
+}
+
+static int swrap_create_socket(struct socket_info *si, int fd)
+{
+	struct socket_info_fd *fi = NULL;
+	int idx;
+
+	fi = (struct socket_info_fd *)calloc(1, sizeof(struct socket_info_fd));
+	if (fi == NULL) {
+		errno = ENOMEM;
+		return -1;
+	}
+
+	idx = swrap_add_socket_info(si);
+	if (idx == -1) {
+		free(fi);
+		return -1;
+	}
+
+	fi->fd = fd;
+	fi->si_index = idx;
+
+	SWRAP_DLIST_ADD(socket_fds, fi);
+
+	return idx;
+}
+
 static int convert_un_in(const struct sockaddr_un *un, struct sockaddr *in, socklen_t *len)
 {
 	unsigned int iface;
-- 
2.16.1


>From db8716e09e7fe9da21c4654ff2e11ccc5a74f343 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Tue, 29 Aug 2017 14:34:28 +0530
Subject: [PATCH 10/23] swrap: Use swrap_create_socket within swrap_socket

Replace the current logic of socket creation within swrap_socket
with more cleaner version using swrap_create_socket.

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 32 +++++---------------------------
 1 file changed, 5 insertions(+), 27 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 81d3d52..ee994f3 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -2866,10 +2866,8 @@ static int swrap_socket(int family, int type, int protocol)
 {
 	struct socket_info *si = NULL;
 	struct socket_info _si = { 0 };
-	struct socket_info *free_si = NULL;
-	struct socket_info_fd *fi = NULL;
 	int fd;
-	int idx;
+	int ret;
 	int real_type = type;
 
 	/*
@@ -2984,35 +2982,15 @@ static int swrap_socket(int family, int type, int protocol)
 		return -1;
 	}
 
-	fi = (struct socket_info_fd *)calloc(1, sizeof(struct socket_info_fd));
-	if (fi == NULL) {
-		errno = ENOMEM;
-		return -1;
-	}
-
-	idx = socket_wrapper_first_free_index();
-	if (idx == -1) {
+	ret = swrap_create_socket(si, fd);
+	if (ret == -1) {
 		return -1;
 	}
 
-	free_si = SOCKET_INFO_GET(idx);
-
-	first_free = SOCKET_INFO_GET_NEXT_FREE(free_si);
-
-	*free_si = _si;
-
-	SOCKET_INFO_INC_REFCOUNT(free_si);
-	SOCKET_INFO_SET_NEXT_FREE(free_si, 0);
-
-	fi->fd = fd;
-	fi->si_index = idx;
-
-	SWRAP_DLIST_ADD(socket_fds, fi);
-
 	SWRAP_LOG(SWRAP_LOG_TRACE,
 		  "Created %s socket for protocol %s",
-		  si->family == AF_INET ? "IPv4" : "IPv6",
-		  si->type == SOCK_DGRAM ? "UDP" : "TCP");
+		  family == AF_INET ? "IPv4" : "IPv6",
+		  real_type == SOCK_DGRAM ? "UDP" : "TCP");
 
 	return fd;
 }
-- 
2.16.1


>From def2c03f6940225dcb15b8bcdfb8889ef59cdb7a Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Tue, 29 Aug 2017 14:34:58 +0530
Subject: [PATCH 11/23] swrap: Use swrap_create_socket within swrap_accept

Replace the current logic of socket creation within swrap_accept
with more cleaner version using swrap_create_socket.

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 40 ++++++++++++----------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index ee994f3..4a38442 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -3077,7 +3077,7 @@ static int swrap_accept(int s,
 			int flags)
 {
 	struct socket_info *parent_si, *child_si;
-	struct socket_info_fd *child_fi;
+	struct socket_info new_si = { 0 };
 	int fd;
 	int idx;
 	struct swrap_address un_addr = {
@@ -3141,21 +3141,7 @@ static int swrap_accept(int s,
 		return ret;
 	}
 
-	idx = socket_wrapper_first_free_index();
-	if (idx == -1) {
-		return -1;
-	}
-
-	child_si = SOCKET_INFO_GET(idx);
-
-	child_fi = (struct socket_info_fd *)calloc(1, sizeof(struct socket_info_fd));
-	if (child_fi == NULL) {
-		close(fd);
-		errno = ENOMEM;
-		return -1;
-	}
-
-	child_fi->fd = fd;
+	child_si = &new_si;
 
 	child_si->family = parent_si->family;
 	child_si->type = parent_si->type;
@@ -3181,7 +3167,6 @@ static int swrap_accept(int s,
 			       &un_my_addr.sa.s,
 			       &un_my_addr.sa_socklen);
 	if (ret == -1) {
-		free(child_fi);
 		close(fd);
 		return ret;
 	}
@@ -3193,7 +3178,6 @@ static int swrap_accept(int s,
 				       &in_my_addr.sa.s,
 				       &in_my_addr.sa_socklen);
 	if (ret == -1) {
-		free(child_fi);
 		close(fd);
 		return ret;
 	}
@@ -3207,18 +3191,18 @@ static int swrap_accept(int s,
 	};
 	memcpy(&child_si->myname.sa.ss, &in_my_addr.sa.ss, in_my_addr.sa_socklen);
 
-	SOCKET_INFO_INC_REFCOUNT(child_si);
-	first_free = SOCKET_INFO_GET_NEXT_FREE(child_si);
-	SOCKET_INFO_SET_NEXT_FREE(child_si, 0);
-
-	child_fi->si_index = idx;
-
-	SWRAP_DLIST_ADD(socket_fds, child_fi);
+	idx = swrap_create_socket(&new_si, fd);
+	if (idx == -1) {
+		close (fd);
+		return -1;
+	}
 
 	if (addr != NULL) {
-		swrap_pcap_dump_packet(child_si, addr, SWRAP_ACCEPT_SEND, NULL, 0);
-		swrap_pcap_dump_packet(child_si, addr, SWRAP_ACCEPT_RECV, NULL, 0);
-		swrap_pcap_dump_packet(child_si, addr, SWRAP_ACCEPT_ACK, NULL, 0);
+		struct socket_info *si = SOCKET_INFO_GET(idx);
+
+		swrap_pcap_dump_packet(si, addr, SWRAP_ACCEPT_SEND, NULL, 0);
+		swrap_pcap_dump_packet(si, addr, SWRAP_ACCEPT_RECV, NULL, 0);
+		swrap_pcap_dump_packet(si, addr, SWRAP_ACCEPT_ACK, NULL, 0);
 	}
 
 	return fd;
-- 
2.16.1


>From 8e774e86a9615c481a3ba502e0f42c9761ab5b8e Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Tue, 29 Aug 2017 14:29:16 +0530
Subject: [PATCH 12/23] swrap: Remove swrap_first_free_index

swrap_first_free_index is no longer used as the whole logic now
implemented within swrap_create_socket.

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 4a38442..3d88035 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -1357,28 +1357,6 @@ static unsigned int socket_wrapper_default_iface(void)
 	return 1;/* 127.0.0.1 */
 }
 
-/*
- * Return the first free entry (if any) and make
- * it re-usable again (by nulling it out)
- */
-static int socket_wrapper_first_free_index(void)
-{
-	struct socket_info *si = NULL;
-	int next_free;
-
-	if (first_free == -1) {
-		errno = ENFILE;
-		return -1;
-	}
-
-	si = SOCKET_INFO_GET(first_free);
-	next_free = SOCKET_INFO_GET_NEXT_FREE(si);
-	ZERO_STRUCTP(si);
-	SOCKET_INFO_SET_NEXT_FREE(si, next_free);
-
-	return first_free;
-}
-
 static int swrap_add_socket_info(struct socket_info *si_input)
 {
 	struct socket_info *si = NULL;
-- 
2.16.1


>From f808dad3a4603f0dc0446a111942926d2187a162 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Thu, 13 Jul 2017 15:20:15 +0530
Subject: [PATCH 13/23] swrap: Rearrange swrap_close

In preparation to implement thread safety, re-ordering lines
of code to properly align to locking calls.

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 3d88035..252749e 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -5675,15 +5675,19 @@ static int swrap_close(int fd)
 	}
 
 	si_index = fi->si_index;
+	si = SOCKET_INFO_GET(si_index);
 
 	SWRAP_DLIST_REMOVE(socket_fds, fi);
-	free(fi);
 
 	ret = libc_close(fd);
 
-	si = SOCKET_INFO_GET(si_index);
+	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
+	first_free = si_index;
+
 	SOCKET_INFO_DEC_REFCOUNT(si);
 
+	free(fi);
+
 	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
 		/* there are still references left */
 		return ret;
@@ -5702,9 +5706,6 @@ static int swrap_close(int fd)
 		unlink(si->un_addr.sun_path);
 	}
 
-	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
-	first_free = si_index;
-
 	return ret;
 }
 
-- 
2.16.1


>From 1620ae030c302432a873d1dbdee9e80bcc336207 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Thu, 13 Jul 2017 15:13:07 +0530
Subject: [PATCH 14/23] swrap: Rearrange swrap_remove_stale

In preparation to implement thread safety, re-ordering lines
of code to properly align to lockign calls

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 252749e..acc24b7 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -1911,15 +1911,19 @@ static void swrap_remove_stale(int fd)
 		return;
 	}
 
+	SWRAP_LOG(SWRAP_LOG_TRACE, "remove stale wrapper for %d", fd);
+
 	si_index = fi->si_index;
+	si = SOCKET_INFO_GET(si_index);
 
-	SWRAP_LOG(SWRAP_LOG_TRACE, "remove stale wrapper for %d", fd);
 	SWRAP_DLIST_REMOVE(socket_fds, fi);
-	free(fi);
 
-	si = SOCKET_INFO_GET(si_index);
+	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
+	first_free = si_index;
 	SOCKET_INFO_DEC_REFCOUNT(si);
 
+	free(fi);
+
 	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
 		return;
 	}
@@ -1927,9 +1931,6 @@ static void swrap_remove_stale(int fd)
 	if (si->un_addr.sun_path[0] != '\0') {
 		unlink(si->un_addr.sun_path);
 	}
-
-	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
-	first_free = si_index;
 }
 
 static int sockaddr_convert_to_un(struct socket_info *si,
-- 
2.16.1


>From 28ed6d8c8c07dbc7df6658374eb31ba97cadb560 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Tue, 28 Mar 2017 07:13:47 +0000
Subject: [PATCH 15/23] swrap: Implement thread safety using pthread mutexes

Added a new mutex variable to socket_info structure along with
new macros for locking and unlocking mutex corresponding to
each socket_info entry. Apart from individual mutex defined in
socket_info structure, 4 new mutexes are added to protect the
concurrent access of globally used swrap parameters from different
threads.

All other individual wrappers and helper routines are also made
capable of acquiring relevant mutex locks before operating on such
global parameters.

Pair-Programmed-With: Michael Adam <obnox at samba.org>

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 215 insertions(+), 8 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index acc24b7..e4cc92f 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -188,6 +188,16 @@ enum swrap_dbglvl_e {
 #define SOCKET_INFO_CONTAINER(si) \
 	(struct socket_info_container *)(si)
 
+#define SWRAP_LOCK_SI(si) do { \
+	struct socket_info_container *sic = SOCKET_INFO_CONTAINER(si); \
+	pthread_mutex_lock(&sic->mutex); \
+} while(0)
+
+#define SWRAP_UNLOCK_SI(si) do { \
+	struct socket_info_container *sic = SOCKET_INFO_CONTAINER(si); \
+	pthread_mutex_unlock(&sic->mutex); \
+} while(0)
+
 #define SOCKET_INFO_GET_REFCOUNT(si) \
 	(SOCKET_INFO_CONTAINER(si))->refcount
 
@@ -209,7 +219,7 @@ enum swrap_dbglvl_e {
 #define SOCKET_INFO_GET(i) \
 	(struct socket_info *)(&(sockets[i].info))
 
-#define SWRAP_DLIST_ADD(list,item) do { \
+#define DLIST_ADD(list, item) do { \
 	if (!(list)) { \
 		(item)->prev	= NULL; \
 		(item)->next	= NULL; \
@@ -222,7 +232,13 @@ enum swrap_dbglvl_e {
 	} \
 } while (0)
 
-#define SWRAP_DLIST_REMOVE(list,item) do { \
+#define SWRAP_DLIST_ADD(list, item) do { \
+	SWRAP_LOCK(list); \
+	DLIST_ADD(list, item); \
+	SWRAP_UNLOCK(list); \
+} while (0)
+
+#define DLIST_REMOVE(list, item) do { \
 	if ((list) == (item)) { \
 		(list)		= (item)->next; \
 		if (list) { \
@@ -240,10 +256,15 @@ enum swrap_dbglvl_e {
 	(item)->next	= NULL; \
 } while (0)
 
-#define SWRAP_DLIST_ADD_AFTER(list, item, el) \
-do { \
+#define SWRAP_DLIST_REMOVE(list,item) do { \
+	SWRAP_LOCK(list); \
+	DLIST_REMOVE(list, item); \
+	SWRAP_UNLOCK(list); \
+} while (0)
+
+#define DLIST_ADD_AFTER(list, item, el) do { \
 	if ((list) == NULL || (el) == NULL) { \
-		SWRAP_DLIST_ADD(list, item); \
+		DLIST_ADD(list, item); \
 	} else { \
 		(item)->prev = (el); \
 		(item)->next = (el)->next; \
@@ -254,6 +275,12 @@ do { \
 	} \
 } while (0)
 
+#define SWRAP_DLIST_ADD_AFTER(list, item, el) do { \
+	SWRAP_LOCK(list); \
+	DLIST_ADD_AFTER(list, item, el); \
+	SWRAP_UNLOCK(list); \
+} while (0)
+
 #if defined(HAVE_GETTIMEOFDAY_TZ) || defined(HAVE_GETTIMEOFDAY_TZ_VOID)
 #define swrapGetTimeOfDay(tval) gettimeofday(tval,NULL)
 #else
@@ -354,6 +381,7 @@ struct socket_info_container
 	struct socket_info info;
 	unsigned int refcount;
 	int next_free;
+	pthread_mutex_t mutex;
 };
 
 static struct socket_info_container *sockets;
@@ -369,6 +397,26 @@ static struct socket_info_fd *socket_fds;
 /* The mutex for accessing the global libc.symbols */
 static pthread_mutex_t libc_symbol_binding_mutex = PTHREAD_MUTEX_INITIALIZER;
 
+/* The mutex for syncronizing the port selection during swrap_auto_bind() */
+static pthread_mutex_t autobind_start_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/*
+ * Global mutex to guard the initialization of array of socket_info structures.
+ */
+static pthread_mutex_t sockets_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/*
+ * Global mutex to protect modification of the socket_fds linked
+ * list structure by different threads within a process.
+ */
+static pthread_mutex_t socket_fds_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/*
+ * Global mutex to synchronize the query for first free index in array of
+ * socket_info structures by different threads within a process.
+ */
+static pthread_mutex_t first_free_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 /* Function prototypes */
 
 bool socket_wrapper_enabled(void);
@@ -1304,7 +1352,10 @@ static void socket_wrapper_init_sockets(void)
 {
 	size_t i;
 
+	SWRAP_LOCK(sockets);
+
 	if (sockets != NULL) {
+		SWRAP_UNLOCK(sockets);
 		return;
 	}
 
@@ -1316,17 +1367,24 @@ static void socket_wrapper_init_sockets(void)
 	if (sockets == NULL) {
 		SWRAP_LOG(SWRAP_LOG_ERROR,
 			  "Failed to allocate sockets array.\n");
+		SWRAP_UNLOCK(sockets);
 		exit(-1);
 	}
 
+	SWRAP_LOCK(first_free);
+
 	first_free = 0;
 
 	for (i = 0; i < max_sockets; i++) {
 		SOCKET_INFO_SET_NEXT_FREE(&sockets[i], i+1);
+		sockets[i].mutex = (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER;
 	}
 
 	/* mark the end of the free list */
 	SOCKET_INFO_SET_NEXT_FREE(&sockets[max_sockets-1], -1);
+
+	SWRAP_UNLOCK(first_free);
+	SWRAP_UNLOCK(sockets);
 }
 
 bool socket_wrapper_enabled(void)
@@ -1367,7 +1425,9 @@ static int swrap_add_socket_info(struct socket_info *si_input)
 		return -1;
 	}
 
+	SWRAP_LOCK(first_free);
 	if (first_free == -1) {
+		SWRAP_UNLOCK(first_free);
 		errno = ENFILE;
 		return -1;
 	}
@@ -1375,10 +1435,15 @@ static int swrap_add_socket_info(struct socket_info *si_input)
 	si_index = first_free;
 	si = SOCKET_INFO_GET(si_index);
 
+	SWRAP_LOCK_SI(si);
+
 	first_free = SOCKET_INFO_GET_NEXT_FREE(si);
 	*si = *si_input;
 	SOCKET_INFO_INC_REFCOUNT(si);
 
+	SWRAP_UNLOCK_SI(si);
+	SWRAP_UNLOCK(first_free);
+
 	return si_index;
 }
 
@@ -1778,13 +1843,17 @@ static struct socket_info_fd *find_socket_info_fd(int fd)
 {
 	struct socket_info_fd *f;
 
+	SWRAP_LOCK(socket_fds);
+
 	for (f = socket_fds; f; f = f->next) {
 		if (f->fd == fd) {
-			return f;
+			break;
 		}
 	}
 
-	return NULL;
+	SWRAP_UNLOCK(socket_fds);
+
+	return f;
 }
 
 static int find_socket_info_index(int fd)
@@ -1916,6 +1985,9 @@ static void swrap_remove_stale(int fd)
 	si_index = fi->si_index;
 	si = SOCKET_INFO_GET(si_index);
 
+	SWRAP_LOCK(first_free);
+	SWRAP_LOCK_SI(si);
+
 	SWRAP_DLIST_REMOVE(socket_fds, fi);
 
 	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
@@ -1925,12 +1997,17 @@ static void swrap_remove_stale(int fd)
 	free(fi);
 
 	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
+		SWRAP_UNLOCK_SI(si);
+		SWRAP_UNLOCK(first_free);
 		return;
 	}
 
 	if (si->un_addr.sun_path[0] != '\0') {
 		unlink(si->un_addr.sun_path);
 	}
+
+	SWRAP_UNLOCK_SI(si);
+	SWRAP_UNLOCK(first_free);
 }
 
 static int sockaddr_convert_to_un(struct socket_info *si,
@@ -3083,16 +3160,26 @@ static int swrap_accept(int s,
 #endif
 	}
 
+
+	/*
+	 * prevent parent_si from being altered / closed
+	 * while we read it
+	 */
+	SWRAP_LOCK_SI(parent_si);
+
 	/*
 	 * assume out sockaddr have the same size as the in parent
 	 * socket family
 	 */
 	in_addr.sa_socklen = socket_length(parent_si->family);
 	if (in_addr.sa_socklen <= 0) {
+		SWRAP_UNLOCK_SI(parent_si);
 		errno = EINVAL;
 		return -1;
 	}
 
+	SWRAP_UNLOCK_SI(parent_si);
+
 #ifdef HAVE_ACCEPT4
 	ret = libc_accept4(s, &un_addr.sa.s, &un_addr.sa_socklen, flags);
 #else
@@ -3109,6 +3196,8 @@ static int swrap_accept(int s,
 
 	fd = ret;
 
+	SWRAP_LOCK_SI(parent_si);
+
 	ret = sockaddr_convert_from_un(parent_si,
 				       &un_addr.sa.un,
 				       un_addr.sa_socklen,
@@ -3116,6 +3205,7 @@ static int swrap_accept(int s,
 				       &in_addr.sa.s,
 				       &in_addr.sa_socklen);
 	if (ret == -1) {
+		SWRAP_UNLOCK_SI(parent_si);
 		close(fd);
 		return ret;
 	}
@@ -3129,6 +3219,8 @@ static int swrap_accept(int s,
 	child_si->is_server = 1;
 	child_si->connected = 1;
 
+	SWRAP_UNLOCK_SI(parent_si);
+
 	child_si->peername = (struct swrap_address) {
 		.sa_socklen = in_addr.sa_socklen,
 	};
@@ -3179,9 +3271,11 @@ static int swrap_accept(int s,
 	if (addr != NULL) {
 		struct socket_info *si = SOCKET_INFO_GET(idx);
 
+		SWRAP_LOCK_SI(si);
 		swrap_pcap_dump_packet(si, addr, SWRAP_ACCEPT_SEND, NULL, 0);
 		swrap_pcap_dump_packet(si, addr, SWRAP_ACCEPT_RECV, NULL, 0);
 		swrap_pcap_dump_packet(si, addr, SWRAP_ACCEPT_ACK, NULL, 0);
+		SWRAP_UNLOCK_SI(si);
 	}
 
 	return fd;
@@ -3223,6 +3317,8 @@ static int swrap_auto_bind(int fd, struct socket_info *si, int family)
 	int port;
 	struct stat st;
 
+	SWRAP_LOCK(autobind_start);
+
 	if (autobind_start_init != 1) {
 		autobind_start_init = 1;
 		autobind_start = getpid();
@@ -3341,6 +3437,7 @@ static int swrap_auto_bind(int fd, struct socket_info *si, int family)
 	ret = 0;
 
 done:
+	SWRAP_UNLOCK(autobind_start);
 	return ret;
 }
 
@@ -3362,6 +3459,8 @@ static int swrap_connect(int s, const struct sockaddr *serv_addr,
 		return libc_connect(s, serv_addr, addrlen);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	if (si->bound == 0) {
 		ret = swrap_auto_bind(s, si, serv_addr->sa_family);
 		if (ret == -1) {
@@ -3445,6 +3544,7 @@ static int swrap_connect(int s, const struct sockaddr *serv_addr,
 	}
 
 done:
+	SWRAP_UNLOCK_SI(si);
 	return ret;
 }
 
@@ -3473,6 +3573,8 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 		return libc_bind(s, myaddr, addrlen);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	switch (si->family) {
 	case AF_INET: {
 		const struct sockaddr_in *sin;
@@ -3519,6 +3621,7 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 	}
 
 	if (bind_error != 0) {
+		SWRAP_UNLOCK_SI(si);
 		errno = bind_error;
 		return -1;
 	}
@@ -3526,6 +3629,7 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 #if 0 /* FIXME */
 	in_use = check_addr_port_in_use(myaddr, addrlen);
 	if (in_use) {
+		SWRAP_UNLOCK_SI(si);
 		errno = EADDRINUSE;
 		return -1;
 	}
@@ -3541,6 +3645,7 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 				     1,
 				     &si->bcast);
 	if (ret == -1) {
+		SWRAP_UNLOCK_SI(si);
 		return -1;
 	}
 
@@ -3556,6 +3661,8 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 		si->bound = 1;
 	}
 
+	SWRAP_UNLOCK_SI(si);
+
 	return ret;
 }
 
@@ -3659,16 +3766,21 @@ static int swrap_listen(int s, int backlog)
 		return libc_listen(s, backlog);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	if (si->bound == 0) {
 		ret = swrap_auto_bind(s, si, si->family);
 		if (ret == -1) {
+			SWRAP_UNLOCK_SI(si);
 			errno = EADDRINUSE;
-			return ret;
+			return -1;
 		}
 	}
 
 	ret = libc_listen(s, backlog);
 
+	SWRAP_UNLOCK_SI(si);
+
 	return ret;
 }
 
@@ -3840,20 +3952,26 @@ static int swrap_getpeername(int s, struct sockaddr *name, socklen_t *addrlen)
 		return libc_getpeername(s, name, addrlen);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	if (si->peername.sa_socklen == 0)
 	{
+		SWRAP_UNLOCK_SI(si);
 		errno = ENOTCONN;
 		return -1;
 	}
 
 	len = MIN(*addrlen, si->peername.sa_socklen);
 	if (len == 0) {
+		SWRAP_UNLOCK_SI(si);
 		return 0;
 	}
 
 	memcpy(name, &si->peername.sa.ss, len);
 	*addrlen = si->peername.sa_socklen;
 
+	SWRAP_UNLOCK_SI(si);
+
 	return 0;
 }
 
@@ -3879,14 +3997,19 @@ static int swrap_getsockname(int s, struct sockaddr *name, socklen_t *addrlen)
 		return libc_getsockname(s, name, addrlen);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	len = MIN(*addrlen, si->myname.sa_socklen);
 	if (len == 0) {
+		SWRAP_UNLOCK_SI(si);
 		return 0;
 	}
 
 	memcpy(name, &si->myname.sa.ss, len);
 	*addrlen = si->myname.sa_socklen;
 
+	SWRAP_UNLOCK_SI(si);
+
 	return 0;
 }
 
@@ -3923,6 +4046,8 @@ static int swrap_getsockopt(int s, int level, int optname,
 				       optlen);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	if (level == SOL_SOCKET) {
 		switch (optname) {
 #ifdef SO_DOMAIN
@@ -4005,6 +4130,7 @@ static int swrap_getsockopt(int s, int level, int optname,
 	ret = -1;
 
 done:
+	SWRAP_UNLOCK_SI(si);
 	return ret;
 }
 
@@ -4043,6 +4169,8 @@ static int swrap_setsockopt(int s, int level, int optname,
 				       optlen);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	if (level == IPPROTO_TCP) {
 		switch (optname) {
 #ifdef TCP_NODELAY
@@ -4107,6 +4235,7 @@ static int swrap_setsockopt(int s, int level, int optname,
 	}
 
 done:
+	SWRAP_UNLOCK_SI(si);
 	return ret;
 }
 
@@ -4131,6 +4260,8 @@ static int swrap_vioctl(int s, unsigned long int r, va_list va)
 		return libc_vioctl(s, r, va);
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	va_copy(ap, va);
 
 	rc = libc_vioctl(s, r, va);
@@ -4149,6 +4280,7 @@ static int swrap_vioctl(int s, unsigned long int r, va_list va)
 
 	va_end(ap);
 
+	SWRAP_UNLOCK_SI(si);
 	return rc;
 }
 
@@ -4466,11 +4598,14 @@ static ssize_t swrap_sendmsg_before(int fd,
 		*bcast = 0;
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	switch (si->type) {
 	case SOCK_STREAM: {
 		unsigned long mtu;
 
 		if (!si->connected) {
+			SWRAP_UNLOCK_SI(si);
 			errno = ENOTCONN;
 			return -1;
 		}
@@ -4514,6 +4649,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 			msg_name = (const struct sockaddr *)msg->msg_name;
 
 			if (msg_name == NULL) {
+				SWRAP_UNLOCK_SI(si);
 				errno = ENOTCONN;
 				return -1;
 			}
@@ -4522,6 +4658,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 			ret = sockaddr_convert_to_un(si, msg_name, msg->msg_namelen,
 						     tmp_un, 0, bcast);
 			if (ret == -1) {
+				SWRAP_UNLOCK_SI(si);
 				return -1;
 			}
 
@@ -4539,10 +4676,12 @@ static ssize_t swrap_sendmsg_before(int fd,
 			ret = swrap_auto_bind(fd, si, si->family);
 			if (ret == -1) {
 				if (errno == ENOTSOCK) {
+					SWRAP_UNLOCK_SI(si);
 					swrap_remove_stale(fd);
 					return -ENOTSOCK;
 				} else {
 					SWRAP_LOG(SWRAP_LOG_ERROR, "swrap_sendmsg_before failed");
+					SWRAP_UNLOCK_SI(si);
 					return -1;
 				}
 			}
@@ -4559,6 +4698,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 					     0,
 					     NULL);
 		if (ret == -1) {
+			SWRAP_UNLOCK_SI(si);
 			return -1;
 		}
 
@@ -4572,6 +4712,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 		}
 
 		if (ret == -1) {
+			SWRAP_UNLOCK_SI(si);
 			return ret;
 		}
 
@@ -4579,6 +4720,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 		break;
 	default:
 		errno = EHOSTUNREACH;
+		SWRAP_UNLOCK_SI(si);
 		return -1;
 	}
 
@@ -4590,6 +4732,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 		ret = swrap_sendmsg_filter_cmsghdr(msg, &cmbuf, &cmlen);
 		if (ret < 0) {
 			free(cmbuf);
+			SWRAP_UNLOCK_SI(si);
 			return -1;
 		}
 
@@ -4604,6 +4747,8 @@ static ssize_t swrap_sendmsg_before(int fd,
 	}
 #endif
 
+	SWRAP_UNLOCK_SI(si);
+
 	return 0;
 }
 
@@ -4658,6 +4803,8 @@ static void swrap_sendmsg_after(int fd,
 	}
 	len = ofs;
 
+	SWRAP_LOCK_SI(si);
+
 	switch (si->type) {
 	case SOCK_STREAM:
 		if (ret == -1) {
@@ -4681,6 +4828,8 @@ static void swrap_sendmsg_after(int fd,
 		break;
 	}
 
+	SWRAP_UNLOCK_SI(si);
+
 	free(buf);
 	errno = saved_errno;
 }
@@ -4693,12 +4842,15 @@ static int swrap_recvmsg_before(int fd,
 	size_t i, len = 0;
 	ssize_t ret;
 
+	SWRAP_LOCK_SI(si);
+
 	(void)fd; /* unused */
 
 	switch (si->type) {
 	case SOCK_STREAM: {
 		unsigned int mtu;
 		if (!si->connected) {
+			SWRAP_UNLOCK_SI(si);
 			errno = ENOTCONN;
 			return -1;
 		}
@@ -4727,6 +4879,7 @@ static int swrap_recvmsg_before(int fd,
 	}
 	case SOCK_DGRAM:
 		if (msg->msg_name == NULL) {
+			SWRAP_UNLOCK_SI(si);
 			errno = EINVAL;
 			return -1;
 		}
@@ -4745,11 +4898,13 @@ static int swrap_recvmsg_before(int fd,
 				 * uses of that descriptor.
 				 */
 				if (errno == ENOTSOCK) {
+					SWRAP_UNLOCK_SI(si);
 					swrap_remove_stale(fd);
 					return -ENOTSOCK;
 				} else {
 					SWRAP_LOG(SWRAP_LOG_ERROR,
 						  "swrap_recvmsg_before failed");
+					SWRAP_UNLOCK_SI(si);
 					return -1;
 				}
 			}
@@ -4757,9 +4912,12 @@ static int swrap_recvmsg_before(int fd,
 		break;
 	default:
 		errno = EHOSTUNREACH;
+		SWRAP_UNLOCK_SI(si);
 		return -1;
 	}
 
+	SWRAP_UNLOCK_SI(si);
+
 	return 0;
 }
 
@@ -4792,6 +4950,8 @@ static int swrap_recvmsg_after(int fd,
 		avail += msg->msg_iov[i].iov_len;
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	/* Convert the socket address before we leave */
 	if (si->type == SOCK_DGRAM && un_addr != NULL) {
 		rc = sockaddr_convert_from_un(si,
@@ -4820,6 +4980,7 @@ static int swrap_recvmsg_after(int fd,
 	buf = (uint8_t *)malloc(remain);
 	if (buf == NULL) {
 		/* we just not capture the packet */
+		SWRAP_UNLOCK_SI(si);
 		errno = saved_errno;
 		return -1;
 	}
@@ -4877,11 +5038,13 @@ done:
 	    msg->msg_control != NULL) {
 		rc = swrap_msghdr_add_socket_info(si, msg);
 		if (rc < 0) {
+			SWRAP_UNLOCK_SI(si);
 			return -1;
 		}
 	}
 #endif
 
+	SWRAP_UNLOCK_SI(si);
 	return rc;
 }
 
@@ -5053,11 +5216,16 @@ static ssize_t swrap_sendto(int s, const void *buf, size_t len, int flags,
 				    un_addr.sa_socklen);
 		}
 
+		SWRAP_LOCK_SI(si);
+
 		swrap_pcap_dump_packet(si, to, SWRAP_SENDTO, buf, len);
 
+		SWRAP_UNLOCK_SI(si);
+
 		return len;
 	}
 
+	SWRAP_LOCK_SI(si);
 	/*
 	 * If it is a dgram socket and we are connected, don't include the
 	 * 'to' address.
@@ -5078,6 +5246,8 @@ static ssize_t swrap_sendto(int s, const void *buf, size_t len, int flags,
 				  msg.msg_namelen);
 	}
 
+	SWRAP_UNLOCK_SI(si);
+
 	swrap_sendmsg_after(s, si, &msg, to, ret);
 
 	return ret;
@@ -5409,6 +5579,8 @@ static ssize_t swrap_recvmsg(int s, struct msghdr *omsg, int flags)
 #endif
 	omsg->msg_iovlen = msg.msg_iovlen;
 
+	SWRAP_LOCK_SI(si);
+
 	/*
 	 * From the manpage:
 	 *
@@ -5428,6 +5600,8 @@ static ssize_t swrap_recvmsg(int s, struct msghdr *omsg, int flags)
 		omsg->msg_namelen = msg.msg_namelen;
 	}
 
+	SWRAP_UNLOCK_SI(si);
+
 	return ret;
 }
 
@@ -5463,12 +5637,17 @@ static ssize_t swrap_sendmsg(int s, const struct msghdr *omsg, int flags)
 
 	ZERO_STRUCT(msg);
 
+	SWRAP_LOCK_SI(si);
+
 	if (si->connected == 0) {
 		msg.msg_name = omsg->msg_name;             /* optional address */
 		msg.msg_namelen = omsg->msg_namelen;       /* size of address */
 	}
 	msg.msg_iov = omsg->msg_iov;               /* scatter/gather array */
 	msg.msg_iovlen = omsg->msg_iovlen;         /* # elements in msg_iov */
+
+	SWRAP_UNLOCK_SI(si);
+
 #ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
 	if (msg.msg_controllen > 0 && msg.msg_control != NULL) {
 		/* omsg is a const so use a local buffer for modifications */
@@ -5534,9 +5713,13 @@ static ssize_t swrap_sendmsg(int s, const struct msghdr *omsg, int flags)
 			libc_sendmsg(s, &msg, flags);
 		}
 
+		SWRAP_LOCK_SI(si);
+
 		swrap_pcap_dump_packet(si, to, SWRAP_SENDTO, buf, len);
 		free(buf);
 
+		SWRAP_UNLOCK_SI(si);
+
 		return len;
 	}
 
@@ -5678,6 +5861,9 @@ static int swrap_close(int fd)
 	si_index = fi->si_index;
 	si = SOCKET_INFO_GET(si_index);
 
+	SWRAP_LOCK(first_free);
+	SWRAP_LOCK_SI(si);
+
 	SWRAP_DLIST_REMOVE(socket_fds, fi);
 
 	ret = libc_close(fd);
@@ -5691,6 +5877,8 @@ static int swrap_close(int fd)
 
 	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
 		/* there are still references left */
+		SWRAP_UNLOCK_SI(si);
+		SWRAP_UNLOCK(first_free);
 		return ret;
 	}
 
@@ -5707,6 +5895,9 @@ static int swrap_close(int fd)
 		unlink(si->un_addr.sun_path);
 	}
 
+	SWRAP_UNLOCK_SI(si);
+	SWRAP_UNLOCK(first_free);
+
 	return ret;
 }
 
@@ -5745,9 +5936,13 @@ static int swrap_dup(int fd)
 		return -1;
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	SOCKET_INFO_INC_REFCOUNT(si);
 	fi->si_index = src_fi->si_index;
 
+	SWRAP_UNLOCK_SI(si);
+
 	/* Make sure we don't have an entry for the fd */
 	swrap_remove_stale(fi->fd);
 
@@ -5806,9 +6001,13 @@ static int swrap_dup2(int fd, int newfd)
 		return -1;
 	}
 
+	SWRAP_LOCK_SI(si);
+
 	SOCKET_INFO_INC_REFCOUNT(si);
 	fi->si_index = src_fi->si_index;
 
+	SWRAP_UNLOCK_SI(si);
+
 	/* Make sure we don't have an entry for the fd */
 	swrap_remove_stale(fi->fd);
 
@@ -5854,9 +6053,13 @@ static int swrap_vfcntl(int fd, int cmd, va_list va)
 			return -1;
 		}
 
+		SWRAP_LOCK_SI(si);
+
 		SOCKET_INFO_INC_REFCOUNT(si);
 		fi->si_index = src_fi->si_index;
 
+		SWRAP_UNLOCK_SI(si);
+
 		/* Make sure we don't have an entry for the fd */
 		swrap_remove_stale(fi->fd);
 
@@ -5981,6 +6184,10 @@ void swrap_destructor(void)
 
 	free(sockets);
 
+	pthread_mutex_destroy(&sockets_mutex);
+	pthread_mutex_destroy(&socket_fds_mutex);
+	pthread_mutex_destroy(&first_free_mutex);
+
 	if (swrap.libc.handle != NULL) {
 		dlclose(swrap.libc.handle);
 	}
-- 
2.16.1


>From fe6b6de107b27ed7171b06aa418a64cc9bd4d6df Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Mon, 15 Jan 2018 16:06:51 +0100
Subject: [PATCH 16/23] swrap: Use common exit point inside wrappers and
 functions

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 src/socket_wrapper.c | 108 ++++++++++++++++++++++++---------------------------
 1 file changed, 51 insertions(+), 57 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index e4cc92f..060b49c 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -1418,7 +1418,7 @@ static unsigned int socket_wrapper_default_iface(void)
 static int swrap_add_socket_info(struct socket_info *si_input)
 {
 	struct socket_info *si = NULL;
-	int si_index;
+	int si_index = -1;
 
 	if (si_input == NULL) {
 		errno = EINVAL;
@@ -1427,9 +1427,8 @@ static int swrap_add_socket_info(struct socket_info *si_input)
 
 	SWRAP_LOCK(first_free);
 	if (first_free == -1) {
-		SWRAP_UNLOCK(first_free);
 		errno = ENFILE;
-		return -1;
+		goto out;
 	}
 
 	si_index = first_free;
@@ -1442,6 +1441,8 @@ static int swrap_add_socket_info(struct socket_info *si_input)
 	SOCKET_INFO_INC_REFCOUNT(si);
 
 	SWRAP_UNLOCK_SI(si);
+
+out:
 	SWRAP_UNLOCK(first_free);
 
 	return si_index;
@@ -1997,15 +1998,14 @@ static void swrap_remove_stale(int fd)
 	free(fi);
 
 	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
-		SWRAP_UNLOCK_SI(si);
-		SWRAP_UNLOCK(first_free);
-		return;
+		goto out;
 	}
 
 	if (si->un_addr.sun_path[0] != '\0') {
 		unlink(si->un_addr.sun_path);
 	}
 
+out:
 	SWRAP_UNLOCK_SI(si);
 	SWRAP_UNLOCK(first_free);
 }
@@ -3621,17 +3621,17 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 	}
 
 	if (bind_error != 0) {
-		SWRAP_UNLOCK_SI(si);
 		errno = bind_error;
-		return -1;
+		ret = -1;
+		goto out;
 	}
 
 #if 0 /* FIXME */
 	in_use = check_addr_port_in_use(myaddr, addrlen);
 	if (in_use) {
-		SWRAP_UNLOCK_SI(si);
 		errno = EADDRINUSE;
-		return -1;
+		ret = -1;
+		goto out;
 	}
 #endif
 
@@ -3645,8 +3645,7 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 				     1,
 				     &si->bcast);
 	if (ret == -1) {
-		SWRAP_UNLOCK_SI(si);
-		return -1;
+		goto out;
 	}
 
 	unlink(un_addr.sa.un.sun_path);
@@ -3661,6 +3660,7 @@ static int swrap_bind(int s, const struct sockaddr *myaddr, socklen_t addrlen)
 		si->bound = 1;
 	}
 
+out:
 	SWRAP_UNLOCK_SI(si);
 
 	return ret;
@@ -3771,14 +3771,14 @@ static int swrap_listen(int s, int backlog)
 	if (si->bound == 0) {
 		ret = swrap_auto_bind(s, si, si->family);
 		if (ret == -1) {
-			SWRAP_UNLOCK_SI(si);
 			errno = EADDRINUSE;
-			return -1;
+			goto out;
 		}
 	}
 
 	ret = libc_listen(s, backlog);
 
+out:
 	SWRAP_UNLOCK_SI(si);
 
 	return ret;
@@ -3947,6 +3947,7 @@ static int swrap_getpeername(int s, struct sockaddr *name, socklen_t *addrlen)
 {
 	struct socket_info *si = find_socket_info(s);
 	socklen_t len;
+	int ret = -1;
 
 	if (!si) {
 		return libc_getpeername(s, name, addrlen);
@@ -3956,23 +3957,24 @@ static int swrap_getpeername(int s, struct sockaddr *name, socklen_t *addrlen)
 
 	if (si->peername.sa_socklen == 0)
 	{
-		SWRAP_UNLOCK_SI(si);
 		errno = ENOTCONN;
-		return -1;
+		goto out;
 	}
 
 	len = MIN(*addrlen, si->peername.sa_socklen);
 	if (len == 0) {
-		SWRAP_UNLOCK_SI(si);
-		return 0;
+		ret = 0;
+		goto out;
 	}
 
 	memcpy(name, &si->peername.sa.ss, len);
 	*addrlen = si->peername.sa_socklen;
 
+	ret = 0;
+out:
 	SWRAP_UNLOCK_SI(si);
 
-	return 0;
+	return ret;
 }
 
 #ifdef HAVE_ACCEPT_PSOCKLEN_T
@@ -3992,6 +3994,7 @@ static int swrap_getsockname(int s, struct sockaddr *name, socklen_t *addrlen)
 {
 	struct socket_info *si = find_socket_info(s);
 	socklen_t len;
+	int ret = -1;
 
 	if (!si) {
 		return libc_getsockname(s, name, addrlen);
@@ -4001,16 +4004,18 @@ static int swrap_getsockname(int s, struct sockaddr *name, socklen_t *addrlen)
 
 	len = MIN(*addrlen, si->myname.sa_socklen);
 	if (len == 0) {
-		SWRAP_UNLOCK_SI(si);
-		return 0;
+		ret = 0;
+		goto out;
 	}
 
 	memcpy(name, &si->myname.sa.ss, len);
 	*addrlen = si->myname.sa_socklen;
 
+	ret = 0;
+out:
 	SWRAP_UNLOCK_SI(si);
 
-	return 0;
+	return ret;
 }
 
 #ifdef HAVE_ACCEPT_PSOCKLEN_T
@@ -4586,7 +4591,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 				    int *bcast)
 {
 	size_t i, len = 0;
-	ssize_t ret;
+	ssize_t ret = -1;
 
 	if (to_un) {
 		*to_un = NULL;
@@ -4605,9 +4610,8 @@ static ssize_t swrap_sendmsg_before(int fd,
 		unsigned long mtu;
 
 		if (!si->connected) {
-			SWRAP_UNLOCK_SI(si);
 			errno = ENOTCONN;
-			return -1;
+			goto out;
 		}
 
 		if (msg->msg_iovlen == 0) {
@@ -4649,17 +4653,15 @@ static ssize_t swrap_sendmsg_before(int fd,
 			msg_name = (const struct sockaddr *)msg->msg_name;
 
 			if (msg_name == NULL) {
-				SWRAP_UNLOCK_SI(si);
 				errno = ENOTCONN;
-				return -1;
+				goto out;
 			}
 
 
 			ret = sockaddr_convert_to_un(si, msg_name, msg->msg_namelen,
 						     tmp_un, 0, bcast);
 			if (ret == -1) {
-				SWRAP_UNLOCK_SI(si);
-				return -1;
+				goto out;
 			}
 
 			if (to_un) {
@@ -4676,14 +4678,12 @@ static ssize_t swrap_sendmsg_before(int fd,
 			ret = swrap_auto_bind(fd, si, si->family);
 			if (ret == -1) {
 				if (errno == ENOTSOCK) {
-					SWRAP_UNLOCK_SI(si);
 					swrap_remove_stale(fd);
-					return -ENOTSOCK;
+					ret = -ENOTSOCK;
 				} else {
 					SWRAP_LOG(SWRAP_LOG_ERROR, "swrap_sendmsg_before failed");
-					SWRAP_UNLOCK_SI(si);
-					return -1;
 				}
+				goto out;
 			}
 		}
 
@@ -4698,8 +4698,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 					     0,
 					     NULL);
 		if (ret == -1) {
-			SWRAP_UNLOCK_SI(si);
-			return -1;
+			goto out;
 		}
 
 		ret = libc_connect(fd,
@@ -4712,16 +4711,14 @@ static ssize_t swrap_sendmsg_before(int fd,
 		}
 
 		if (ret == -1) {
-			SWRAP_UNLOCK_SI(si);
-			return ret;
+			goto out;
 		}
 
 		si->defer_connect = 0;
 		break;
 	default:
 		errno = EHOSTUNREACH;
-		SWRAP_UNLOCK_SI(si);
-		return -1;
+		goto out;
 	}
 
 #ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
@@ -4732,8 +4729,7 @@ static ssize_t swrap_sendmsg_before(int fd,
 		ret = swrap_sendmsg_filter_cmsghdr(msg, &cmbuf, &cmlen);
 		if (ret < 0) {
 			free(cmbuf);
-			SWRAP_UNLOCK_SI(si);
-			return -1;
+			goto out;
 		}
 
 		if (cmlen == 0) {
@@ -4747,9 +4743,11 @@ static ssize_t swrap_sendmsg_before(int fd,
 	}
 #endif
 
+	ret = 0;
+out:
 	SWRAP_UNLOCK_SI(si);
 
-	return 0;
+	return ret;
 }
 
 static void swrap_sendmsg_after(int fd,
@@ -4840,7 +4838,7 @@ static int swrap_recvmsg_before(int fd,
 				struct iovec *tmp_iov)
 {
 	size_t i, len = 0;
-	ssize_t ret;
+	int ret = -1;
 
 	SWRAP_LOCK_SI(si);
 
@@ -4850,9 +4848,8 @@ static int swrap_recvmsg_before(int fd,
 	case SOCK_STREAM: {
 		unsigned int mtu;
 		if (!si->connected) {
-			SWRAP_UNLOCK_SI(si);
 			errno = ENOTCONN;
-			return -1;
+			goto out;
 		}
 
 		if (msg->msg_iovlen == 0) {
@@ -4879,9 +4876,8 @@ static int swrap_recvmsg_before(int fd,
 	}
 	case SOCK_DGRAM:
 		if (msg->msg_name == NULL) {
-			SWRAP_UNLOCK_SI(si);
 			errno = EINVAL;
-			return -1;
+			goto out;
 		}
 
 		if (msg->msg_iovlen == 0) {
@@ -4898,27 +4894,26 @@ static int swrap_recvmsg_before(int fd,
 				 * uses of that descriptor.
 				 */
 				if (errno == ENOTSOCK) {
-					SWRAP_UNLOCK_SI(si);
 					swrap_remove_stale(fd);
-					return -ENOTSOCK;
+					ret = -ENOTSOCK;
 				} else {
 					SWRAP_LOG(SWRAP_LOG_ERROR,
 						  "swrap_recvmsg_before failed");
-					SWRAP_UNLOCK_SI(si);
-					return -1;
 				}
+				goto out;
 			}
 		}
 		break;
 	default:
 		errno = EHOSTUNREACH;
-		SWRAP_UNLOCK_SI(si);
-		return -1;
+		goto out;
 	}
 
+	ret = 0;
+out:
 	SWRAP_UNLOCK_SI(si);
 
-	return 0;
+	return ret;
 }
 
 static int swrap_recvmsg_after(int fd,
@@ -5877,9 +5872,7 @@ static int swrap_close(int fd)
 
 	if (SOCKET_INFO_GET_REFCOUNT(si) > 0) {
 		/* there are still references left */
-		SWRAP_UNLOCK_SI(si);
-		SWRAP_UNLOCK(first_free);
-		return ret;
+		goto out;
 	}
 
 	if (si->myname.sa_socklen > 0 && si->peername.sa_socklen > 0) {
@@ -5895,6 +5888,7 @@ static int swrap_close(int fd)
 		unlink(si->un_addr.sun_path);
 	}
 
+out:
 	SWRAP_UNLOCK_SI(si);
 	SWRAP_UNLOCK(first_free);
 
-- 
2.16.1


>From 7cb826566b37d3b4f6e85afd18cfe178e1f7b45e Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 22 Sep 2016 03:53:27 +0200
Subject: [PATCH 17/23] tests: Add new test to check mutex lock contention

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 tests/CMakeLists.txt        |  3 +-
 tests/test_thread_sockets.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_thread_sockets.c

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index b86cc84..28c1e05 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -38,7 +38,8 @@ set(SWRAP_TESTS
     test_swrap_unit
     test_max_sockets
     test_close_failure
-    test_fork_thread_deadlock)
+    test_fork_thread_deadlock
+    test_thread_sockets)
 
 if (HAVE_STRUCT_MSGHDR_MSG_CONTROL)
     set(SWRAP_TESTS ${SWRAP_TESTS} test_sendmsg_recvmsg_fd)
diff --git a/tests/test_thread_sockets.c b/tests/test_thread_sockets.c
new file mode 100644
index 0000000..364a001
--- /dev/null
+++ b/tests/test_thread_sockets.c
@@ -0,0 +1,72 @@
+#include "config.h"
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <pthread.h>
+
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <arpa/inet.h>
+#include <unistd.h>
+#include <grp.h>
+
+#define NUM_THREADS 10
+
+static void *thread_worker(void *arg)
+{
+	int i;
+
+	(void) arg; /* unused */
+
+	for (i = 0; i < 1000; i++) {
+		int s;
+
+		s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+		assert_return_code(s, errno);
+
+		close(s);
+	}
+
+	return NULL;
+}
+
+static void test_threads_socket(void **state)
+{
+	pthread_attr_t pthread_custom_attr;
+	pthread_t threads[NUM_THREADS];
+	int i;
+
+	(void) state; /* unused */
+
+	pthread_attr_init(&pthread_custom_attr);
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_create(&threads[i],
+			       &pthread_custom_attr,
+			       thread_worker,
+			       NULL);
+	}
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_join(threads[i], NULL);
+	}
+
+	pthread_attr_destroy(&pthread_custom_attr);
+}
+
+int main(void) {
+	int rc;
+
+	const struct CMUnitTest thread_tests[] = {
+		cmocka_unit_test(test_threads_socket),
+	};
+
+	rc = cmocka_run_group_tests(thread_tests, NULL, NULL);
+
+	return rc;
+}
-- 
2.16.1


>From 438fa065efa7f4db6b1fd81bda98284d25598d28 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Thu, 2 Mar 2017 07:12:50 +0000
Subject: [PATCH 18/23] tests: Modify echo server to accept multiple
 connections

In context of multiple threads, echo server must be capable of
accepting connections in a loop rather than be satisfied with
one incoming connection.

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 tests/echo_srv.c | 58 ++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/tests/echo_srv.c b/tests/echo_srv.c
index 5b784de..2f877f4 100644
--- a/tests/echo_srv.c
+++ b/tests/echo_srv.c
@@ -497,39 +497,51 @@ static void echo_tcp(int sock)
 
     int client_sock = -1;
     int s;
+    pid_t pid;
 
-    s = accept(sock, &addr.sa.s, &addr.sa_socklen);
-    if (s == -1) {
-        perror("accept");
-	goto done;
-    }
-
-    client_sock = socket_dup(s);
-    if (client_sock == -1) {
-        perror("socket_dup");
-	goto done;
-    }
-
-    /* Start ping pong */
     while (1) {
-        bret = recv(client_sock, buf, BUFSIZE, 0);
-        if (bret == -1) {
-            perror("recv");
+        s = accept(sock, &addr.sa.s, &addr.sa_socklen);
+        if (s == -1) {
+            perror("accept");
             goto done;
-        } else if (bret == 0) {
-            break;
         }
 
-        bret = send(client_sock, buf, bret, 0);
-        if (bret == -1) {
-            perror("send");
-            goto done;
+        pid = fork();
+        if (pid == -1) {
+            perror("fork");
+        } else if (pid == 0) {
+            close(sock);
+            client_sock = socket_dup(s);
+            if (client_sock == -1) {
+                perror("socket_dup");
+                goto done;
+            }
+
+            while (1) {
+                bret = recv(client_sock, buf, BUFSIZE, 0);
+                if (bret == -1) {
+                    perror("recv");
+                    goto done;
+                } else if (bret == 0) {
+                    break;
+                }
+
+                bret = send(client_sock, buf, bret, 0);
+                if (bret == -1) {
+                    perror("send");
+                    goto done;
+                }
+            }
+            close(s);
+            exit(0);
+        } else {
+            close(s);
         }
     }
 
 done:
     if (client_sock != -1) {
-	    close(client_sock);
+        close(client_sock);
     }
 }
 
-- 
2.16.1


>From 9ff93122b1cd65922ba9af97a8e11adb3b2f1432 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Thu, 2 Mar 2017 07:26:20 +0000
Subject: [PATCH 19/23] tests: New threaded test cases

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 tests/CMakeLists.txt                         |   6 +-
 tests/test_thread_echo_tcp_connect.c         | 100 +++++++++++
 tests/test_thread_echo_tcp_sendmsg_recvmsg.c | 241 +++++++++++++++++++++++++++
 tests/test_thread_echo_tcp_write_read.c      | 121 ++++++++++++++
 tests/test_thread_echo_udp_send_recv.c       | 124 ++++++++++++++
 5 files changed, 591 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_thread_echo_tcp_connect.c
 create mode 100644 tests/test_thread_echo_tcp_sendmsg_recvmsg.c
 create mode 100644 tests/test_thread_echo_tcp_write_read.c
 create mode 100644 tests/test_thread_echo_udp_send_recv.c

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 28c1e05..4dc996a 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -39,7 +39,11 @@ set(SWRAP_TESTS
     test_max_sockets
     test_close_failure
     test_fork_thread_deadlock
-    test_thread_sockets)
+    test_thread_sockets
+    test_thread_echo_tcp_connect
+    test_thread_echo_tcp_write_read
+    test_thread_echo_tcp_sendmsg_recvmsg
+    test_thread_echo_udp_send_recv)
 
 if (HAVE_STRUCT_MSGHDR_MSG_CONTROL)
     set(SWRAP_TESTS ${SWRAP_TESTS} test_sendmsg_recvmsg_fd)
diff --git a/tests/test_thread_echo_tcp_connect.c b/tests/test_thread_echo_tcp_connect.c
new file mode 100644
index 0000000..128b88b
--- /dev/null
+++ b/tests/test_thread_echo_tcp_connect.c
@@ -0,0 +1,100 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <pthread.h>
+
+#include "config.h"
+#include "torture.h"
+
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#define NUM_THREADS 10
+
+static int setup_echo_srv_tcp_ipv4(void **state)
+{
+	torture_setup_echo_srv_tcp_ipv4(state);
+
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	torture_teardown_echo_srv(state);
+
+	return 0;
+}
+
+static void *thread_worker(void *arg)
+{
+	struct torture_address addr = {
+		.sa_socklen = sizeof(struct sockaddr_in),
+	};
+	int rc;
+	int s;
+
+	(void) arg; /* unused */
+
+	s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	assert_int_not_equal(s, -1);
+
+	addr.sa.in.sin_family = AF_INET;
+	addr.sa.in.sin_port = htons(torture_server_port());
+
+	rc = inet_pton(addr.sa.in.sin_family,
+		       torture_server_address(AF_INET),
+		       &addr.sa.in.sin_addr);
+	assert_int_equal(rc, 1);
+
+	rc = connect(s, &addr.sa.s, addr.sa_socklen);
+	assert_return_code(rc, errno);
+
+	close(s);
+	return NULL;
+}
+
+static void test_connect_ipv4(void **state)
+{
+	pthread_attr_t pthread_custom_attr;
+	pthread_t threads[NUM_THREADS];
+	int i;
+
+	(void) state; /* unused */
+
+	pthread_attr_init(&pthread_custom_attr);
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_create(&threads[i],
+			       &pthread_custom_attr,
+			       thread_worker,
+			       NULL);
+	}
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_join(threads[i], NULL);
+	}
+
+	pthread_attr_destroy(&pthread_custom_attr);
+}
+
+int main(void) {
+	int rc;
+
+	const struct CMUnitTest tcp_connect_tests[] = {
+		cmocka_unit_test(test_connect_ipv4),
+	};
+
+	rc = cmocka_run_group_tests(tcp_connect_tests,
+				    setup_echo_srv_tcp_ipv4,
+				    teardown);
+
+	return rc;
+}
diff --git a/tests/test_thread_echo_tcp_sendmsg_recvmsg.c b/tests/test_thread_echo_tcp_sendmsg_recvmsg.c
new file mode 100644
index 0000000..a4920d5
--- /dev/null
+++ b/tests/test_thread_echo_tcp_sendmsg_recvmsg.c
@@ -0,0 +1,241 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <pthread.h>
+
+#include "config.h"
+#include "torture.h"
+
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#define NUM_THREADS 10
+
+static int setup_echo_srv_tcp_ipv4(void **state)
+{
+	torture_setup_echo_srv_tcp_ipv4(state);
+
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	torture_teardown_echo_srv(state);
+
+	return 0;
+}
+
+static void *thread_worker1(void *arg)
+{
+	struct torture_address addr = {
+		.sa_socklen = sizeof(struct sockaddr_storage),
+	};
+	char send_buf[64] = {0};
+	char recv_buf[64] = {0};
+	ssize_t ret;
+	int rc;
+	int i;
+	int s;
+
+	(void) arg; /* unused */
+
+	s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	assert_int_not_equal(s, -1);
+
+	addr.sa.in.sin_family = AF_INET;
+	addr.sa.in.sin_port = htons(torture_server_port());
+
+	rc = inet_pton(AF_INET,
+		       torture_server_address(AF_INET),
+		       &addr.sa.in.sin_addr);
+	assert_int_equal(rc, 1);
+
+	rc = connect(s, &addr.sa.s, addr.sa_socklen);
+
+	for (i = 0; i < 10; i++) {
+		struct torture_address reply_addr = {
+			.sa_socklen = sizeof(struct sockaddr_storage),
+		};
+		struct msghdr s_msg = {
+			.msg_namelen = 0,
+		};
+		struct msghdr r_msg = {
+			.msg_namelen = 0,
+		};
+		struct iovec s_iov;
+		struct iovec r_iov;
+
+		snprintf(send_buf, sizeof(send_buf), "packet.%d", i);
+
+		/* This should be ignored */
+		rc = inet_pton(AF_INET,
+			       "127.0.0.1",
+			       &addr.sa.in.sin_addr);
+		assert_int_equal(rc, 1);
+
+		s_msg.msg_name = &addr.sa.s;
+		s_msg.msg_namelen = addr.sa_socklen;
+
+		s_iov.iov_base = send_buf;
+		s_iov.iov_len = sizeof(send_buf);
+
+		s_msg.msg_iov = &s_iov;
+		s_msg.msg_iovlen = 1;
+
+		ret = sendmsg(s, &s_msg, 0);
+		assert_int_not_equal(ret, -1);
+
+		r_msg.msg_name = &reply_addr.sa.s;
+		r_msg.msg_namelen = reply_addr.sa_socklen;
+
+		r_iov.iov_base = recv_buf;
+		r_iov.iov_len = sizeof(recv_buf);
+
+		r_msg.msg_iov = &r_iov;
+		r_msg.msg_iovlen = 1;
+
+		ret = recvmsg(s, &r_msg, 0);
+		assert_int_not_equal(ret, -1);
+
+		assert_int_equal(r_msg.msg_namelen, 0);
+
+		assert_memory_equal(send_buf, recv_buf, sizeof(send_buf));
+	}
+
+	close(s);
+	return NULL;
+}
+
+static void *thread_worker2(void *arg)
+{
+	struct torture_address send_addr = {
+		.sa_socklen = sizeof(struct sockaddr_storage),
+	};
+	struct msghdr s_msg = {
+		.msg_namelen = 0,
+	};
+	struct msghdr r_msg = {
+		.msg_namelen = 0,
+	};
+	struct iovec iov;
+	char payload[] = "PACKET";
+	ssize_t ret;
+	int rc;
+	int s;
+
+	(void)arg; /* unused */
+
+	s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	assert_int_not_equal(s, -1);
+
+	send_addr.sa.in.sin_family = AF_INET;
+	send_addr.sa.in.sin_port = htons(torture_server_port());
+
+	rc = inet_pton(AF_INET,
+		       torture_server_address(AF_INET),
+		       &send_addr.sa.in.sin_addr);
+	assert_int_equal(rc, 1);
+
+	rc = connect(s, &send_addr.sa.s, send_addr.sa_socklen);
+
+	/* msg_name = NULL */
+
+	iov.iov_base = (void *)payload;
+	iov.iov_len = sizeof(payload);
+
+	s_msg.msg_iov = &iov;
+	s_msg.msg_iovlen = 1;
+
+	ret = sendmsg(s, &s_msg, 0);
+	assert_int_not_equal(ret, -1);
+
+	/* msg_name = NULL */
+
+	memset(payload, 0, sizeof(payload));
+
+	r_msg.msg_iov = &iov;
+	r_msg.msg_iovlen = 1;
+
+	ret = recvmsg(s, &r_msg, 0);
+	assert_int_not_equal(ret, -1);
+
+	assert_int_equal(r_msg.msg_namelen, 0);
+	assert_null(r_msg.msg_name);
+
+	close(s);
+	return NULL;
+}
+
+static void test_sendmsg_recvmsg_ipv4(void **state)
+{
+	pthread_attr_t pthread_custom_attr;
+	pthread_t threads[NUM_THREADS];
+	int i;
+
+	(void) state; /* unused */
+
+	pthread_attr_init(&pthread_custom_attr);
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_create(&threads[i],
+			       &pthread_custom_attr,
+			       thread_worker1,
+			       NULL);
+	}
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_join(threads[i], NULL);
+	}
+
+	pthread_attr_destroy(&pthread_custom_attr);
+}
+
+static void test_sendmsg_recvmsg_ipv4_null(void **state)
+{
+	pthread_attr_t pthread_custom_attr;
+	pthread_t threads[NUM_THREADS];
+	int i;
+
+	(void) state; /* unused */
+
+	pthread_attr_init(&pthread_custom_attr);
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_create(&threads[i],
+			       &pthread_custom_attr,
+			       thread_worker2,
+			       NULL);
+	}
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_join(threads[i], NULL);
+	}
+
+	pthread_attr_destroy(&pthread_custom_attr);
+}
+
+
+int main(void) {
+	int rc;
+
+	const struct CMUnitTest sendmsg_tests[] = {
+		cmocka_unit_test_setup_teardown(test_sendmsg_recvmsg_ipv4,
+						setup_echo_srv_tcp_ipv4,
+						teardown),
+		cmocka_unit_test_setup_teardown(test_sendmsg_recvmsg_ipv4_null,
+						setup_echo_srv_tcp_ipv4,
+						teardown),
+	};
+
+	rc = cmocka_run_group_tests(sendmsg_tests, NULL, NULL);
+
+	return rc;
+}
diff --git a/tests/test_thread_echo_tcp_write_read.c b/tests/test_thread_echo_tcp_write_read.c
new file mode 100644
index 0000000..2a86ba2
--- /dev/null
+++ b/tests/test_thread_echo_tcp_write_read.c
@@ -0,0 +1,121 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <pthread.h>
+
+#include "config.h"
+#include "torture.h"
+
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#define NUM_THREADS 10
+
+static int setup_echo_srv_tcp_ipv4(void **state)
+{
+	torture_setup_echo_srv_tcp_ipv4(state);
+
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	torture_teardown_echo_srv(state);
+
+	return 0;
+}
+
+static void *thread_worker(void *arg)
+{
+	struct torture_address addr = {
+		.sa_socklen = sizeof(struct sockaddr_in),
+	};
+	ssize_t ret;
+	int rc;
+	int i;
+	int s;
+
+	(void) arg; /* unused */
+
+	s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	assert_int_not_equal(s, -1);
+
+	addr.sa.in.sin_family = AF_INET;
+	addr.sa.in.sin_port = htons(torture_server_port());
+
+	rc = inet_pton(addr.sa.in.sin_family,
+		       torture_server_address(AF_INET),
+		       &addr.sa.in.sin_addr);
+	assert_int_equal(rc, 1);
+
+	rc = connect(s, &addr.sa.s, addr.sa_socklen);
+	assert_return_code(rc, errno);
+
+	for (i = 0; i < 10; i++) {
+		char send_buf[64] = {0};
+		char recv_buf[64] = {0};
+
+		snprintf(send_buf, sizeof(send_buf), "packet.%d", i);
+
+		ret = write(s,
+			    send_buf,
+			    sizeof(send_buf));
+		assert_return_code(rc, errno);
+
+		ret = read(s,
+			   recv_buf,
+			   sizeof(recv_buf));
+		assert_int_not_equal(ret, -1);
+
+		assert_memory_equal(send_buf, recv_buf, sizeof(send_buf));
+	}
+
+	close(s);
+	return NULL;
+}
+
+static void test_write_read_ipv4(void **state)
+{
+	pthread_attr_t pthread_custom_attr;
+	pthread_t threads[NUM_THREADS];
+	int i;
+
+	(void) state; /* unused */
+
+	pthread_attr_init(&pthread_custom_attr);
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_create(&threads[i],
+			       &pthread_custom_attr,
+			       thread_worker,
+			       NULL);
+	}
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_join(threads[i], NULL);
+	}
+
+	pthread_attr_destroy(&pthread_custom_attr);
+}
+
+int main(void) {
+	int rc;
+
+	const struct CMUnitTest tcp_write_tests[] = {
+		cmocka_unit_test_setup_teardown(test_write_read_ipv4,
+						setup_echo_srv_tcp_ipv4,
+						teardown),
+	};
+
+	rc = cmocka_run_group_tests(tcp_write_tests, NULL, NULL);
+
+	return rc;
+}
diff --git a/tests/test_thread_echo_udp_send_recv.c b/tests/test_thread_echo_udp_send_recv.c
new file mode 100644
index 0000000..dd265d8
--- /dev/null
+++ b/tests/test_thread_echo_udp_send_recv.c
@@ -0,0 +1,124 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <pthread.h>
+
+#include "config.h"
+#include "torture.h"
+
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#define NUM_THREADS 10
+
+static int setup_echo_srv_udp_ipv4(void **state)
+{
+	torture_setup_echo_srv_udp_ipv4(state);
+
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	torture_teardown_echo_srv(state);
+
+	return 0;
+}
+
+static void *thread_worker(void *arg)
+{
+	struct torture_address addr = {
+		.sa_socklen = sizeof(struct sockaddr_in),
+	};
+	ssize_t ret;
+	int rc;
+	int i;
+	int s;
+
+	(void) arg; /* unused */
+
+	s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+	assert_int_not_equal(s, -1);
+
+	addr.sa.in.sin_family = AF_INET;
+	addr.sa.in.sin_port = htons(torture_server_port());
+
+	rc = inet_pton(AF_INET,
+		       torture_server_address(AF_INET),
+		       &addr.sa.in.sin_addr);
+	assert_int_equal(rc, 1);
+
+	rc = connect(s, &addr.sa.s, addr.sa_socklen);
+	assert_int_equal(rc, 0);
+
+	for (i = 0; i < 10; i++) {
+		char send_buf[64] = {0};
+		char recv_buf[64] = {0};
+
+		snprintf(send_buf, sizeof(send_buf), "packet.%d", i);
+
+		ret = send(s,
+			   send_buf,
+			   sizeof(send_buf),
+			   0);
+		assert_int_not_equal(ret, -1);
+
+		ret = recv(s,
+			   recv_buf,
+			   sizeof(recv_buf),
+			   0);
+		assert_int_not_equal(ret, -1);
+
+		assert_memory_equal(send_buf, recv_buf, sizeof(send_buf));
+	}
+
+	close(s);
+	return NULL;
+}
+
+static void test_send_recv_ipv4(void **state)
+{
+	pthread_attr_t pthread_custom_attr;
+	pthread_t threads[NUM_THREADS];
+	int i;
+
+	(void) state; /* unused */
+
+	pthread_attr_init(&pthread_custom_attr);
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_create(&threads[i],
+			       &pthread_custom_attr,
+			       thread_worker,
+			       NULL);
+	}
+
+	for (i = 0; i < NUM_THREADS; i++) {
+		pthread_join(threads[i], NULL);
+	}
+
+	pthread_attr_destroy(&pthread_custom_attr);
+}
+
+
+int main(void) {
+	int rc;
+
+	const struct CMUnitTest send_tests[] = {
+		cmocka_unit_test_setup_teardown(test_send_recv_ipv4,
+						setup_echo_srv_udp_ipv4,
+						teardown),
+	};
+
+	rc = cmocka_run_group_tests(send_tests, NULL, NULL);
+
+	return rc;
+}
-- 
2.16.1


>From ee0d69eb19133152298523fdc513d1874323900a Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Thu, 16 Mar 2017 09:48:12 +0000
Subject: [PATCH 20/23] swrap: Use process-shared robust mutexes

In preparation to implement fd-passing for socket_wrapper,
it is required to have mutexes to be shared among different
precesses.

Pair-Programmed-With: Michael Adam <obnox at samba.org>
Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 060b49c..bb3668d 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -403,7 +403,7 @@ static pthread_mutex_t autobind_start_mutex = PTHREAD_MUTEX_INITIALIZER;
 /*
  * Global mutex to guard the initialization of array of socket_info structures.
  */
-static pthread_mutex_t sockets_mutex = PTHREAD_MUTEX_INITIALIZER;
+pthread_mutex_t sockets_mutex;
 
 /*
  * Global mutex to protect modification of the socket_fds linked
@@ -415,7 +415,7 @@ static pthread_mutex_t socket_fds_mutex = PTHREAD_MUTEX_INITIALIZER;
  * Global mutex to synchronize the query for first free index in array of
  * socket_info structures by different threads within a process.
  */
-static pthread_mutex_t first_free_mutex = PTHREAD_MUTEX_INITIALIZER;
+pthread_mutex_t first_free_mutex;
 
 /* Function prototypes */
 
@@ -1315,6 +1315,39 @@ done:
 	return max_mtu;
 }
 
+static int socket_wrapper_init_mutex(pthread_mutex_t *m)
+{
+	pthread_mutexattr_t ma;
+	int ret;
+
+	ret = pthread_mutexattr_init(&ma);
+	if (ret != 0) {
+		return ret;
+	}
+
+	ret = pthread_mutexattr_settype(&ma, PTHREAD_MUTEX_ERRORCHECK);
+	if (ret != 0) {
+		goto done;
+	}
+
+	ret = pthread_mutexattr_setpshared(&ma, PTHREAD_PROCESS_SHARED);
+	if (ret != 0) {
+		goto done;
+	}
+
+	ret = pthread_mutexattr_setrobust(&ma, PTHREAD_MUTEX_ROBUST);
+	if (ret != 0) {
+		goto done;
+	}
+
+	ret = pthread_mutex_init(m, &ma);
+
+done:
+	pthread_mutexattr_destroy(&ma);
+
+	return ret;
+}
+
 static size_t socket_wrapper_max_sockets(void)
 {
 	const char *s;
@@ -1351,6 +1384,7 @@ done:
 static void socket_wrapper_init_sockets(void)
 {
 	size_t i;
+	int ret;
 
 	SWRAP_LOCK(sockets);
 
@@ -1377,7 +1411,15 @@ static void socket_wrapper_init_sockets(void)
 
 	for (i = 0; i < max_sockets; i++) {
 		SOCKET_INFO_SET_NEXT_FREE(&sockets[i], i+1);
-		sockets[i].mutex = (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER;
+		ret = socket_wrapper_init_mutex(&sockets[i].mutex);
+		if (ret != 0) {
+			SWRAP_LOG(SWRAP_LOG_ERROR,
+				  "Failed to initialize pthread mutexes.\n");
+			free(sockets);
+			SWRAP_UNLOCK(first_free);
+			SWRAP_UNLOCK(sockets);
+			exit(-1);
+		}
 	}
 
 	/* mark the end of the free list */
@@ -6157,6 +6199,9 @@ void swrap_constructor(void)
 	pthread_atfork(&swrap_thread_prepare,
 		       &swrap_thread_parent,
 		       &swrap_thread_child);
+
+	socket_wrapper_init_mutex(&sockets_mutex);
+	socket_wrapper_init_mutex(&first_free_mutex);
 }
 
 /****************************
-- 
2.16.1


>From f856c6c1bf65234baef4cc3cad601c7f85061aca Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 13 Jul 2017 01:25:19 +0200
Subject: [PATCH 21/23] swrap: Move metadata into socket_info_meta structure

Separating out the metadata related information to another
sub-structure to make it more clean and structured.

Pair-Programmed-With: Anoop C S <anoopcs at redhat.com>
Signed-off-by: Anoop C S <anoopcs at redhat.com>
Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 src/socket_wrapper.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index bb3668d..653d5b2 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -190,16 +190,16 @@ enum swrap_dbglvl_e {
 
 #define SWRAP_LOCK_SI(si) do { \
 	struct socket_info_container *sic = SOCKET_INFO_CONTAINER(si); \
-	pthread_mutex_lock(&sic->mutex); \
+	pthread_mutex_lock(&sic->meta.mutex); \
 } while(0)
 
 #define SWRAP_UNLOCK_SI(si) do { \
 	struct socket_info_container *sic = SOCKET_INFO_CONTAINER(si); \
-	pthread_mutex_unlock(&sic->mutex); \
+	pthread_mutex_unlock(&sic->meta.mutex); \
 } while(0)
 
 #define SOCKET_INFO_GET_REFCOUNT(si) \
-	(SOCKET_INFO_CONTAINER(si))->refcount
+	(SOCKET_INFO_CONTAINER(si))->meta.refcount
 
 #define SOCKET_INFO_INC_REFCOUNT(si) do { \
 	 SOCKET_INFO_GET_REFCOUNT(si) += 1; \
@@ -210,7 +210,7 @@ enum swrap_dbglvl_e {
 } while(0)
 
 #define SOCKET_INFO_GET_NEXT_FREE(si) \
-	(SOCKET_INFO_CONTAINER(si))->next_free
+	(SOCKET_INFO_CONTAINER(si))->meta.next_free
 
 #define SOCKET_INFO_SET_NEXT_FREE(si, next_free) do { \
 	SOCKET_INFO_GET_NEXT_FREE(si) = (next_free); \
@@ -376,14 +376,19 @@ struct socket_info
 	} io;
 };
 
-struct socket_info_container
+struct socket_info_meta
 {
-	struct socket_info info;
 	unsigned int refcount;
 	int next_free;
 	pthread_mutex_t mutex;
 };
 
+struct socket_info_container
+{
+	struct socket_info info;
+	struct socket_info_meta meta;
+};
+
 static struct socket_info_container *sockets;
 static size_t max_sockets = 0;
 
@@ -1411,7 +1416,7 @@ static void socket_wrapper_init_sockets(void)
 
 	for (i = 0; i < max_sockets; i++) {
 		SOCKET_INFO_SET_NEXT_FREE(&sockets[i], i+1);
-		ret = socket_wrapper_init_mutex(&sockets[i].mutex);
+		ret = socket_wrapper_init_mutex(&sockets[i].meta.mutex);
 		if (ret != 0) {
 			SWRAP_LOG(SWRAP_LOG_ERROR,
 				  "Failed to initialize pthread mutexes.\n");
-- 
2.16.1


>From 140f4a0e2ea63219703081e920cc739be6efb8bc Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Wed, 31 Jan 2018 22:46:23 +0530
Subject: [PATCH 22/23] swrap: Update free-list only when refcount is zero

Signed-off-by: Anoop C S <anoopcs at redhat.com>
---
 src/socket_wrapper.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 653d5b2..d3d127d 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -2038,8 +2038,6 @@ static void swrap_remove_stale(int fd)
 
 	SWRAP_DLIST_REMOVE(socket_fds, fi);
 
-	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
-	first_free = si_index;
 	SOCKET_INFO_DEC_REFCOUNT(si);
 
 	free(fi);
@@ -2052,6 +2050,9 @@ static void swrap_remove_stale(int fd)
 		unlink(si->un_addr.sun_path);
 	}
 
+	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
+	first_free = si_index;
+
 out:
 	SWRAP_UNLOCK_SI(si);
 	SWRAP_UNLOCK(first_free);
@@ -5910,9 +5911,6 @@ static int swrap_close(int fd)
 
 	ret = libc_close(fd);
 
-	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
-	first_free = si_index;
-
 	SOCKET_INFO_DEC_REFCOUNT(si);
 
 	free(fi);
@@ -5935,6 +5933,9 @@ static int swrap_close(int fd)
 		unlink(si->un_addr.sun_path);
 	}
 
+	SOCKET_INFO_SET_NEXT_FREE(si, first_free);
+	first_free = si_index;
+
 out:
 	SWRAP_UNLOCK_SI(si);
 	SWRAP_UNLOCK(first_free);
-- 
2.16.1


>From fc9c6db4da1316a883a9bb75b014d9bb040774f1 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Wed, 31 Jan 2018 22:43:21 +0530
Subject: [PATCH 23/23] tests: Add test case to validate free-list indexes

Signed-off-by: Anoop C S <anoopcs at redhat.com>
---
 tests/CMakeLists.txt              |  3 +-
 tests/test_tcp_socket_overwrite.c | 74 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_tcp_socket_overwrite.c

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 4dc996a..4a66bcc 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -43,7 +43,8 @@ set(SWRAP_TESTS
     test_thread_echo_tcp_connect
     test_thread_echo_tcp_write_read
     test_thread_echo_tcp_sendmsg_recvmsg
-    test_thread_echo_udp_send_recv)
+    test_thread_echo_udp_send_recv
+    test_tcp_socket_overwrite)
 
 if (HAVE_STRUCT_MSGHDR_MSG_CONTROL)
     set(SWRAP_TESTS ${SWRAP_TESTS} test_sendmsg_recvmsg_fd)
diff --git a/tests/test_tcp_socket_overwrite.c b/tests/test_tcp_socket_overwrite.c
new file mode 100644
index 0000000..9695e33
--- /dev/null
+++ b/tests/test_tcp_socket_overwrite.c
@@ -0,0 +1,74 @@
+#include "torture.h"
+
+#include <cmocka.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <errno.h>
+
+static int setup(void **state)
+{
+	torture_setup_socket_dir(state);
+
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	torture_teardown_socket_dir(state);
+
+	return 0;
+}
+
+static void test_tcp_socket_overwrite(void **state)
+{
+	struct torture_address addr_in = {
+		.sa_socklen = sizeof(struct sockaddr_in),
+	};
+
+	int s, dup_s, new_s, rc;
+
+	(void) state; /* unused */
+
+	s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	assert_int_not_equal(s, -1);
+
+	dup_s = dup(s);
+	assert_int_not_equal(dup_s, -1);
+
+	close(dup_s);
+
+	new_s = socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP);
+	assert_int_not_equal(new_s, -1);
+
+	close(new_s);
+
+	addr_in = (struct torture_address) {
+		.sa_socklen = sizeof(struct sockaddr_in),
+		.sa.in = (struct sockaddr_in) {
+			.sin_family = AF_INET,
+		},
+	};
+
+	rc = inet_pton(AF_INET, "127.0.0.20", &addr_in.sa.in.sin_addr);
+	assert_int_equal(rc, 1);
+
+	/* bind should fail during socklen check if old socket info
+	 * is overwritten by new socket info */
+	rc = bind(s, &addr_in.sa.s, addr_in.sa_socklen);
+	assert_return_code(rc, errno);
+
+	close(s);
+}
+
+int main(void) {
+	int rc;
+
+	const struct CMUnitTest tcp_socket_overwrite_tests[] = {
+		cmocka_unit_test(test_tcp_socket_overwrite),
+	};
+
+	rc = cmocka_run_group_tests(tcp_socket_overwrite_tests,
+				    setup, teardown);
+
+	return rc;
+}
-- 
2.16.1



More information about the samba-technical mailing list