All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v2 0/4] Increase bind() converage - GH#538
@ 2019-09-26 15:13 Martin Doucha
  2019-09-26 15:13 ` [LTP] [PATCH v2 1/4] Update syscalls/bind02 to new API Martin Doucha
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Martin Doucha @ 2019-09-26 15:13 UTC (permalink / raw)
  To: ltp

This patchset updates syscalls/bind02 to new API and adds a test that bind()
opens sockets for incoming connections. The test is split into two programs,
one for stream-oriented sockets and the other for datagram-oriented sockets.

Changes from previous version:
- minor code style fixes in syscalls/bind02
- refactoring of include/tst_net.h (patch #2)
- code from libbind.c and some setup code moved to tst_net.c (patch #3)
- added UDPLITE and SCTP test cases
- common constants and test_case data structure moved to libbind.h

Petr Vorel wrote:
> Both tests also share a lot of code. I understand you don't want to mix TCP
> and UDP tests (I would probably do), but could you at least move setup()
> and constants into libbind.h?

I tried to write a single test file for both at first but the code flow is
slightly different which would result in a huge if/else mess. In SOCK_STREAM
test file, main thread gets a new socket from accept() so it can (and should)
use read()/write() to communicate with peer. On the other hand, SOCK_DGRAM
test file has to do its own handshake and main thread has to use recvfrom()
and sendto() because the main thread never gets a point-to-point connection
to peer.

I've decided not to write test cases for SOCK_SEQPACKET+IPPROTO_SCTP (yet)
because this type of socket doesn't support connect() at all. The call will
succeed but write() will then raise SIGPIPE. So I'd have to write a third
test file with yet another slightly different code flow.


Martin Doucha (4):
  Update syscalls/bind02 to new API
  Create separate .c file for include/tst_net.h
  Add socket address initialization functions to tst_net library
  Add connection tests for bind()

 include/tst_net.h                         | 122 ++-----------
 lib/tst_net.c                             | 205 ++++++++++++++++++++++
 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/bind/.gitignore |   2 +
 testcases/kernel/syscalls/bind/Makefile   |   2 +
 testcases/kernel/syscalls/bind/bind02.c   | 154 +++++-----------
 testcases/kernel/syscalls/bind/bind04.c   | 180 +++++++++++++++++++
 testcases/kernel/syscalls/bind/bind05.c   | 192 ++++++++++++++++++++
 testcases/kernel/syscalls/bind/libbind.h  |  29 +++
 9 files changed, 672 insertions(+), 216 deletions(-)
 create mode 100644 lib/tst_net.c
 create mode 100644 testcases/kernel/syscalls/bind/bind04.c
 create mode 100644 testcases/kernel/syscalls/bind/bind05.c
 create mode 100644 testcases/kernel/syscalls/bind/libbind.h

-- 
2.23.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 1/4] Update syscalls/bind02 to new API
  2019-09-26 15:13 [LTP] [PATCH v2 0/4] Increase bind() converage - GH#538 Martin Doucha
@ 2019-09-26 15:13 ` Martin Doucha
  2019-10-04 12:36   ` Cyril Hrubis
  2019-09-26 15:13 ` [LTP] [PATCH v2 2/4] Create separate .c file for include/tst_net.h Martin Doucha
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Doucha @ 2019-09-26 15:13 UTC (permalink / raw)
  To: ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/bind/bind02.c | 154 +++++++-----------------
 1 file changed, 42 insertions(+), 112 deletions(-)

diff --git a/testcases/kernel/syscalls/bind/bind02.c b/testcases/kernel/syscalls/bind/bind02.c
index 90b0e9d8e..532831265 100644
--- a/testcases/kernel/syscalls/bind/bind02.c
+++ b/testcases/kernel/syscalls/bind/bind02.c
@@ -1,146 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
  *   Copyright (c) International Business Machines  Corp., 2004
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *      07/2004 Written by Dan Jones
+ *      07/2004 Ported to LTP format by Robbie Williamson
+ *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
  */
 
 /*
- * Test Name: bind02
- *
  * Test Description:
- *  Make sure bind() gives EACCESS error for (non-root) users.
- *
- * Usage:  <for command-line>
- *  bind01 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -e   : Turn on errno logging.
- *             -i n : Execute test n times.
- *             -I x : Execute test for x seconds.
- *             -P x : Pause for x seconds between iterations.
- *             -t   : Turn on syscall timing.
- *
- * HISTORY
- *      07/2004 Written by Dan Jones
- *      07/2004 Ported to LTP format by Robbie Williamson
- *
- * RESTRICTIONS:
- *  None.
- *
+ *  Make sure bind() of privileged port gives EACCESS error for non-root users.
  */
 
-#include <stdio.h>
-#include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
-#include <errno.h>
-#include <fcntl.h>
 #include <pwd.h>
 #include <grp.h>
 
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <sys/un.h>
-
 #include <netinet/in.h>
 
-#include "test.h"
-
-char *TCID = "bind02";
-int testno;
-int TST_TOTAL = 1;
-
-/* This port needs to be a Privledged port */
-#define TCP_PRIVLEGED_COM_PORT 463
-
-struct passwd *pw;
-struct group *gr;
+#include "tst_test.h"
 
-uid_t uid;
-gid_t gid;
+/* This port needs to be a privileged port */
+#define TCP_PRIVILEGED_PORT 463
+#define TEST_USERNAME "nobody"
 
-int rc;
-
-void try_bind(void)
+void run(void)
 {
 	struct sockaddr_in servaddr;
-	int sockfd, r_value;
-
-	// Set effective user/group
-	if ((rc = setegid(gid)) == -1) {
-		tst_brkm(TBROK | TERRNO, 0, "setegid(%u) failed", gid);
-	}
-	if ((rc = seteuid(uid)) == -1) {
-		tst_brkm(TBROK | TERRNO, 0, "seteuid(%u) failed", uid);
-	}
-
-	if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
-		tst_brkm(TBROK | TERRNO, 0, "socket() failed");
-	}
+	int sockfd;
 
+	sockfd = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
 	memset(&servaddr, 0, sizeof(servaddr));
 	servaddr.sin_family = AF_INET;
-	servaddr.sin_port = htons(TCP_PRIVLEGED_COM_PORT);
+	servaddr.sin_port = htons(TCP_PRIVILEGED_PORT);
 	servaddr.sin_addr.s_addr = htonl(INADDR_ANY);
-	r_value = bind(sockfd, (struct sockaddr *)&servaddr, sizeof(servaddr));
-	if (r_value) {
-		if (errno == EACCES) {
-			tst_resm(TPASS, "correct error");
-		} else {
-			tst_resm(TFAIL, "incorrect error, %d", r_value);
-		}
-	} else {
-		tst_resm(TFAIL, "user was able to bind successfully");
-	}
-
+	TEST(bind(sockfd, (struct sockaddr *)&servaddr, sizeof(servaddr)));
 	close(sockfd);
 
-	// Set effective user/group
-	if ((rc = setegid(0)) == -1) {
-		tst_brkm(TBROK | TERRNO, 0, "setegid(0) reset failed");
-	}
-	if ((rc = seteuid(uid)) == -1) {
-		/* XXX: is this seteuid() correct !?  it isnt a reset if we
-		 *      made the same exact call above ...
-		 */
-		tst_brkm(TBROK | TERRNO, 0, "seteuid(%u) reset failed", uid);
+	if (!TST_RET) {
+		tst_res(TFAIL, "bind() succeeded unexpectedly");
+	} else if (TST_ERR == EACCES) {
+		tst_res(TPASS, "bind() failed as expected");
+	} else {
+		tst_res(TFAIL | TERRNO, "Unexpected error");
 	}
-
 }
 
-int main(int argc, char *argv[])
+void setup(void)
 {
-	char *username = "nobody";
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	tst_require_root();
+	struct passwd *pw;
+	struct group *gr;
 
-	if ((pw = getpwnam(username)) == NULL) {
-		tst_brkm(TBROK, 0, "Username - %s - not found", username);
-	}
-
-	if ((gr = getgrgid(pw->pw_gid)) == NULL) {
-		tst_brkm(TBROK | TERRNO, 0, "getgrgid(%u) failed", pw->pw_gid);
-	}
-
-	uid = pw->pw_uid;
-	gid = gr->gr_gid;
+	pw = SAFE_GETPWNAM(TEST_USERNAME);
+	gr = SAFE_GETGRGID(pw->pw_gid);
 
-	tst_resm(TINFO, "Socket will try to be bind by user: %s, group: %s",
-		 pw->pw_name, gr->gr_name);
+	tst_res(TINFO, "Switching credentials to user: %s, group: %s",
+		pw->pw_name, gr->gr_name);
+	SAFE_SETEGID(gr->gr_gid);
+	SAFE_SETEUID(pw->pw_uid);
+}
 
-	try_bind();
-	tst_exit();
+void cleanup(void)
+{
+	SAFE_SETEGID(0);
+	SAFE_SETEUID(0);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.needs_root = 1,
+	.setup = setup,
+	.cleanup = cleanup
+};
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 2/4] Create separate .c file for include/tst_net.h
  2019-09-26 15:13 [LTP] [PATCH v2 0/4] Increase bind() converage - GH#538 Martin Doucha
  2019-09-26 15:13 ` [LTP] [PATCH v2 1/4] Update syscalls/bind02 to new API Martin Doucha
@ 2019-09-26 15:13 ` Martin Doucha
  2019-10-04 12:40   ` Cyril Hrubis
  2019-09-26 15:13 ` [LTP] [PATCH v2 3/4] Add socket address initialization functions to tst_net library Martin Doucha
  2019-09-26 15:13 ` [LTP] [PATCH v2 4/4] Add connection tests for bind() Martin Doucha
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Doucha @ 2019-09-26 15:13 UTC (permalink / raw)
  To: ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 include/tst_net.h | 118 +++-------------------------------------
 lib/tst_net.c     | 134 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 110 deletions(-)
 create mode 100644 lib/tst_net.c

diff --git a/include/tst_net.h b/include/tst_net.h
index cb97b7b61..740f25bac 100644
--- a/include/tst_net.h
+++ b/include/tst_net.h
@@ -16,7 +16,7 @@
  */
 
 #include <arpa/inet.h>
-#include <errno.h>
+#include <stdio.h>
 
 #define MAX_IPV4_PREFIX 32
 #define MAX_IPV6_PREFIX 128
@@ -42,112 +42,10 @@ static inline void print_svar_change(const char *name, const char *val)
 		printf("export %s=\"${%s:-%s}\"\n", name, name, val);
 }
 
-/*
- * Function bit_count is from ipcalc project, ipcalc.c.
- */
-static int bit_count(uint32_t i)
-{
-	int c = 0;
-	unsigned int seen_one = 0;
-
-	while (i > 0) {
-		if (i & 1) {
-			seen_one = 1;
-			c++;
-		} else {
-			if (seen_one)
-				return -1;
-		}
-		i >>= 1;
-	}
-
-	return c;
-}
-
-/*
- * Function mask2prefix is from ipcalc project, ipcalc.c.
- */
-static int mask2prefix(struct in_addr mask)
-{
-	return bit_count(ntohl(mask.s_addr));
-}
-
-/*
- * Function ipv4_mask_to_int is from ipcalc project, ipcalc.c.
- */
-static int ipv4_mask_to_int(const char *prefix)
-{
-	int ret;
-	struct in_addr in;
-
-	ret = inet_pton(AF_INET, prefix, &in);
-	if (ret == 0)
-		return -1;
-
-	return mask2prefix(in);
-}
-
-/*
- * Function safe_atoi is from ipcalc project, ipcalc.c.
- */
-static int safe_atoi(const char *s, int *ret_i)
-{
-	char *x = NULL;
-	long l;
-
-	errno = 0;
-	l = strtol(s, &x, 0);
-
-	if (!x || x == s || *x || errno)
-		return errno > 0 ? -errno : -EINVAL;
-
-	if ((long)(int)l != l)
-		return -ERANGE;
-
-	*ret_i = (int)l;
-
-	return 0;
-}
-
-/*
- * Function get_prefix use code from ipcalc project, str_to_prefix/ipcalc.c.
- */
-static int get_prefix(const char *ip_str, int is_ipv6)
-{
-	char *prefix_str = NULL;
-	int prefix = -1, r;
-
-	prefix_str = strchr(ip_str, '/');
-	if (!prefix_str)
-		return -1;
-
-	*(prefix_str++) = '\0';
-
-	if (!is_ipv6 && strchr(prefix_str, '.'))
-		prefix = ipv4_mask_to_int(prefix_str);
-	else {
-		r = safe_atoi(prefix_str, &prefix);
-		if (r != 0)
-			tst_brk_comment("conversion error: '%s' is not integer",
-					prefix_str);
-	}
-
-	if (prefix < 0 || ((is_ipv6 && prefix > MAX_IPV6_PREFIX) ||
-		(!is_ipv6 && prefix > MAX_IPV4_PREFIX)))
-		tst_brk_comment("bad %s prefix: %s", is_ipv6 ?  "IPv6" : "IPv4",
-				prefix_str);
-
-	return prefix;
-}
-
-static void get_in_addr(const char *ip_str, struct in_addr *ip)
-{
-	if (inet_pton(AF_INET, ip_str, ip) <= 0)
-		tst_brk_comment("bad IPv4 address: '%s'", ip_str);
-}
-
-static void get_in6_addr(const char *ip_str, struct in6_addr *ip6)
-{
-	if (inet_pton(AF_INET6, ip_str, ip6) <= 0)
-		tst_brk_comment("bad IPv6 address: '%s'", ip_str);
-}
+int bit_count(uint32_t i);
+int mask2prefix(struct in_addr mask);
+int ipv4_mask_to_int(const char *prefix);
+int safe_atoi(const char *s, int *ret_i);
+int get_prefix(const char *ip_str, int is_ipv6);
+void get_in_addr(const char *ip_str, struct in_addr *ip);
+void get_in6_addr(const char *ip_str, struct in6_addr *ip6);
diff --git a/lib/tst_net.c b/lib/tst_net.c
new file mode 100644
index 000000000..4166641f1
--- /dev/null
+++ b/lib/tst_net.c
@@ -0,0 +1,134 @@
+/*
+ * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_net.h"
+
+/*
+ * Function bit_count is from ipcalc project, ipcalc.c.
+ */
+int bit_count(uint32_t i)
+{
+	int c = 0;
+	unsigned int seen_one = 0;
+
+	while (i > 0) {
+		if (i & 1) {
+			seen_one = 1;
+			c++;
+		} else {
+			if (seen_one)
+				return -1;
+		}
+		i >>= 1;
+	}
+
+	return c;
+}
+
+/*
+ * Function mask2prefix is from ipcalc project, ipcalc.c.
+ */
+int mask2prefix(struct in_addr mask)
+{
+	return bit_count(ntohl(mask.s_addr));
+}
+
+/*
+ * Function ipv4_mask_to_int is from ipcalc project, ipcalc.c.
+ */
+int ipv4_mask_to_int(const char *prefix)
+{
+	int ret;
+	struct in_addr in;
+
+	ret = inet_pton(AF_INET, prefix, &in);
+	if (ret == 0)
+		return -1;
+
+	return mask2prefix(in);
+}
+
+/*
+ * Function safe_atoi is from ipcalc project, ipcalc.c.
+ */
+int safe_atoi(const char *s, int *ret_i)
+{
+	char *x = NULL;
+	long l;
+
+	errno = 0;
+	l = strtol(s, &x, 0);
+
+	if (!x || x == s || *x || errno)
+		return errno > 0 ? -errno : -EINVAL;
+
+	if ((long)(int)l != l)
+		return -ERANGE;
+
+	*ret_i = (int)l;
+
+	return 0;
+}
+
+/*
+ * Function get_prefix use code from ipcalc project, str_to_prefix/ipcalc.c.
+ */
+int get_prefix(const char *ip_str, int is_ipv6)
+{
+	char *prefix_str = NULL;
+	int prefix = -1, r;
+
+	prefix_str = strchr(ip_str, '/');
+	if (!prefix_str)
+		return -1;
+
+	*(prefix_str++) = '\0';
+
+	if (!is_ipv6 && strchr(prefix_str, '.'))
+		prefix = ipv4_mask_to_int(prefix_str);
+	else {
+		r = safe_atoi(prefix_str, &prefix);
+		if (r != 0)
+			tst_brk_comment("conversion error: '%s' is not integer",
+					prefix_str);
+	}
+
+	if (prefix < 0 || ((is_ipv6 && prefix > MAX_IPV6_PREFIX) ||
+		(!is_ipv6 && prefix > MAX_IPV4_PREFIX)))
+		tst_brk_comment("bad %s prefix: %s", is_ipv6 ?  "IPv6" : "IPv4",
+				prefix_str);
+
+	return prefix;
+}
+
+void get_in_addr(const char *ip_str, struct in_addr *ip)
+{
+	if (inet_pton(AF_INET, ip_str, ip) <= 0)
+		tst_brk_comment("bad IPv4 address: '%s'", ip_str);
+}
+
+void get_in6_addr(const char *ip_str, struct in6_addr *ip6)
+{
+	if (inet_pton(AF_INET6, ip_str, ip6) <= 0)
+		tst_brk_comment("bad IPv6 address: '%s'", ip_str);
+}
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 3/4] Add socket address initialization functions to tst_net library
  2019-09-26 15:13 [LTP] [PATCH v2 0/4] Increase bind() converage - GH#538 Martin Doucha
  2019-09-26 15:13 ` [LTP] [PATCH v2 1/4] Update syscalls/bind02 to new API Martin Doucha
  2019-09-26 15:13 ` [LTP] [PATCH v2 2/4] Create separate .c file for include/tst_net.h Martin Doucha
@ 2019-09-26 15:13 ` Martin Doucha
  2019-10-04 12:42   ` Cyril Hrubis
  2019-09-26 15:13 ` [LTP] [PATCH v2 4/4] Add connection tests for bind() Martin Doucha
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Doucha @ 2019-09-26 15:13 UTC (permalink / raw)
  To: ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 include/tst_net.h | 16 +++++++++++
 lib/tst_net.c     | 71 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/include/tst_net.h b/include/tst_net.h
index 740f25bac..2c958da13 100644
--- a/include/tst_net.h
+++ b/include/tst_net.h
@@ -17,6 +17,9 @@
 
 #include <arpa/inet.h>
 #include <stdio.h>
+#include <sys/types.h>
+#include <netinet/in.h>
+#include <netinet/ip.h>
 
 #define MAX_IPV4_PREFIX 32
 #define MAX_IPV6_PREFIX 128
@@ -49,3 +52,16 @@ int safe_atoi(const char *s, int *ret_i);
 int get_prefix(const char *ip_str, int is_ipv6);
 void get_in_addr(const char *ip_str, struct in_addr *ip);
 void get_in6_addr(const char *ip_str, struct in6_addr *ip6);
+
+/*
+ * Find valid connection address for a given bound socket
+ */
+socklen_t get_connect_address(int sock, struct sockaddr_storage *addr);
+
+/*
+ * Initialize AF_INET/AF_INET6 socket address structure with address and port
+ */
+void init_sockaddr_inet(struct sockaddr_in *sa, const char *ip_str, uint16_t port);
+void init_sockaddr_inet_bin(struct sockaddr_in *sa, uint32_t ip_val, uint16_t port);
+void init_sockaddr_inet6(struct sockaddr_in6 *sa, const char *ip_str, uint16_t port);
+void init_sockaddr_inet6_bin(struct sockaddr_in6 *sa, const struct in6_addr *ip_val, uint16_t port);
diff --git a/lib/tst_net.c b/lib/tst_net.c
index 4166641f1..4ccd81eb9 100644
--- a/lib/tst_net.c
+++ b/lib/tst_net.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
+ * Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -132,3 +133,73 @@ void get_in6_addr(const char *ip_str, struct in6_addr *ip6)
 	if (inet_pton(AF_INET6, ip_str, ip6) <= 0)
 		tst_brk_comment("bad IPv6 address: '%s'", ip_str);
 }
+
+socklen_t get_connect_address(int sock, struct sockaddr_storage *addr)
+{
+	struct sockaddr_in *inet_ptr;
+	struct sockaddr_in6 *inet6_ptr;
+	size_t tmp_size;
+	socklen_t ret = sizeof(*addr);
+
+	SAFE_GETSOCKNAME(sock, (struct sockaddr*)addr, &ret);
+
+	// Sanitize wildcard addresses
+	switch (addr->ss_family) {
+	case AF_INET:
+		inet_ptr = (struct sockaddr_in*)addr;
+
+		switch (ntohl(inet_ptr->sin_addr.s_addr)) {
+		case INADDR_ANY:
+		case INADDR_BROADCAST:
+			inet_ptr->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+			break;
+		}
+
+		break;
+
+	case AF_INET6:
+		inet6_ptr = (struct sockaddr_in6*)addr;
+		tmp_size = sizeof(struct in6_addr);
+
+		if (!memcmp(&inet6_ptr->sin6_addr, &in6addr_any, tmp_size)) {
+			memcpy(&inet6_ptr->sin6_addr, &in6addr_loopback,
+				tmp_size);
+		}
+
+		break;
+	}
+
+	return ret;
+}
+
+void init_sockaddr_inet(struct sockaddr_in *sa, const char *ip_str, uint16_t port)
+{
+	memset(sa, 0, sizeof(struct sockaddr_in));
+	sa->sin_family = AF_INET;
+	sa->sin_port = htons(port);
+	get_in_addr(ip_str, &sa->sin_addr);
+}
+
+void init_sockaddr_inet_bin(struct sockaddr_in *sa, uint32_t ip_val, uint16_t port)
+{
+	memset(sa, 0, sizeof(struct sockaddr_in));
+	sa->sin_family = AF_INET;
+	sa->sin_port = htons(port);
+	sa->sin_addr.s_addr = htonl(ip_val);
+}
+
+void init_sockaddr_inet6(struct sockaddr_in6 *sa, const char *ip_str, uint16_t port)
+{
+	memset(sa, 0, sizeof(struct sockaddr_in6));
+	sa->sin6_family = AF_INET6;
+	sa->sin6_port = htons(port);
+	get_in6_addr(ip_str, &sa->sin6_addr);
+}
+
+void init_sockaddr_inet6_bin(struct sockaddr_in6 *sa, const struct in6_addr *ip_val, uint16_t port)
+{
+	memset(sa, 0, sizeof(struct sockaddr_in6));
+	sa->sin6_family = AF_INET6;
+	sa->sin6_port = htons(port);
+	memcpy(&sa->sin6_addr, ip_val, sizeof(struct in6_addr));
+}
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 4/4] Add connection tests for bind()
  2019-09-26 15:13 [LTP] [PATCH v2 0/4] Increase bind() converage - GH#538 Martin Doucha
                   ` (2 preceding siblings ...)
  2019-09-26 15:13 ` [LTP] [PATCH v2 3/4] Add socket address initialization functions to tst_net library Martin Doucha
@ 2019-09-26 15:13 ` Martin Doucha
  2019-10-04 13:03   ` Cyril Hrubis
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Doucha @ 2019-09-26 15:13 UTC (permalink / raw)
  To: ltp

Add two new test programs to verify that bind() will open sockets for incoming
connections. Both programs follow the same test scenario:
- Create and bind() a socket
- Wait for connection from peer thread
- Send request to peer thread
- Receive and verify response from peer thread

bind04 tests stream-oriented sockets (SOCK_STREAM and SOCK_SEQPACKET).
bind05 tests datagram-oriented sockets (SOCK_DGRAM).

Both programs test the following socket types:
- AF_UNIX (pathname and abstract addresses)
- AF_INET (loopback)
- AF_INET6 (loopback)

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/bind/.gitignore |   2 +
 testcases/kernel/syscalls/bind/Makefile   |   2 +
 testcases/kernel/syscalls/bind/bind04.c   | 180 ++++++++++++++++++++
 testcases/kernel/syscalls/bind/bind05.c   | 192 ++++++++++++++++++++++
 testcases/kernel/syscalls/bind/libbind.h  |  29 ++++
 6 files changed, 407 insertions(+)
 create mode 100644 testcases/kernel/syscalls/bind/bind04.c
 create mode 100644 testcases/kernel/syscalls/bind/bind05.c
 create mode 100644 testcases/kernel/syscalls/bind/libbind.h

diff --git a/runtest/syscalls b/runtest/syscalls
index 874ae4d4f..2738193a8 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -31,6 +31,8 @@ alarm07 alarm07
 bind01 bind01
 bind02 bind02
 bind03 bind03
+bind04 bind04
+bind05 bind05
 
 bpf_map01 bpf_map01
 bpf_prog01 bpf_prog01
diff --git a/testcases/kernel/syscalls/bind/.gitignore b/testcases/kernel/syscalls/bind/.gitignore
index 4ebea9ee7..e18ceea56 100644
--- a/testcases/kernel/syscalls/bind/.gitignore
+++ b/testcases/kernel/syscalls/bind/.gitignore
@@ -1,3 +1,5 @@
 /bind01
 /bind02
 /bind03
+/bind04
+/bind05
diff --git a/testcases/kernel/syscalls/bind/Makefile b/testcases/kernel/syscalls/bind/Makefile
index bd617d806..1fe390997 100644
--- a/testcases/kernel/syscalls/bind/Makefile
+++ b/testcases/kernel/syscalls/bind/Makefile
@@ -20,4 +20,6 @@ top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
 
+bind04 bind05:	LDLIBS		+= -lpthread
+
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/bind/bind04.c b/testcases/kernel/syscalls/bind/bind04.c
new file mode 100644
index 000000000..bd71ca82e
--- /dev/null
+++ b/testcases/kernel/syscalls/bind/bind04.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
+ */
+
+/*
+ * Create and bind socket for various standard stream protocols.
+ * Then connect to it and send some test data.
+ */
+
+#include <string.h>
+#include <stdlib.h>
+#include <time.h>
+#include <pthread.h>
+
+#include "tst_test.h"
+#include "tst_net.h"
+#include "tst_safe_pthread.h"
+#include "libbind.h"
+
+static struct sockaddr_un unix_addr = {
+	.sun_family = AF_UNIX,
+	.sun_path = MAIN_SOCKET_FILE
+};
+static struct sockaddr_un abstract_addr = {
+	.sun_family = AF_UNIX,
+	.sun_path = ABSTRACT_SOCKET_PATH
+};
+static struct sockaddr_in ipv4_addr;
+static struct sockaddr_in ipv4_any_addr;
+static struct sockaddr_in6 ipv6_addr;
+static struct sockaddr_in6 ipv6_any_addr;
+
+static struct test_case testcase_list[] = {
+	// UNIX sockets
+	{SOCK_STREAM, 0, (struct sockaddr*)&unix_addr, sizeof(unix_addr),
+		"AF_UNIX pathname stream"},
+	{SOCK_SEQPACKET, 0, (struct sockaddr*)&unix_addr, sizeof(unix_addr),
+		"AF_UNIX pathname seqpacket"},
+	{SOCK_STREAM, 0, (struct sockaddr*)&abstract_addr,
+		sizeof(abstract_addr), "AF_UNIX abstract stream"},
+	{SOCK_SEQPACKET, 0, (struct sockaddr*)&abstract_addr,
+		sizeof(abstract_addr), "AF_UNIX abstract seqpacket"},
+
+	// IPv4 sockets
+	{SOCK_STREAM, 0, (struct sockaddr*)&ipv4_addr, sizeof(ipv4_addr),
+		"IPv4 loop TCP variant 1"},
+	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv4_addr,
+		sizeof(ipv4_addr), "IPv4 loop TCP variant 2"},
+	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv4_addr,
+		sizeof(ipv4_addr), "IPv4 loop SCTP"},
+	{SOCK_STREAM, 0, (struct sockaddr*)&ipv4_any_addr,
+		sizeof(ipv4_any_addr), "IPv4 any TCP variant 1"},
+	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv4_any_addr,
+		sizeof(ipv4_any_addr), "IPv4 any TCP variant 2"},
+	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv4_any_addr,
+		sizeof(ipv4_any_addr), "IPv4 any SCTP"},
+
+	// IPv6 sockets
+	{SOCK_STREAM, 0, (struct sockaddr*)&ipv6_addr, sizeof(ipv6_addr),
+		"IPv6 loop TCP variant 1"},
+	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv6_addr,
+		sizeof(ipv6_addr), "IPv6 loop TCP variant 2"},
+	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv6_addr,
+		sizeof(ipv6_addr), "IPv6 loop SCTP"},
+	{SOCK_STREAM, 0, (struct sockaddr*)&ipv6_any_addr,
+		sizeof(ipv6_any_addr), "IPv6 any TCP variant 1"},
+	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv6_any_addr,
+		sizeof(ipv6_any_addr), "IPv6 any TCP variant 2"},
+	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv6_any_addr,
+		sizeof(ipv6_any_addr), "IPv6 any SCTP"}
+};
+
+static void setup(void)
+{
+	srand(time(0));
+
+	// Init addresses for IPv4/IPv6 test cases
+	init_sockaddr_inet(&ipv4_addr, IPV4_ADDRESS, 0);
+	init_sockaddr_inet_bin(&ipv4_any_addr, INADDR_ANY, 0);
+	init_sockaddr_inet6_bin(&ipv6_addr, &in6addr_loopback, 0);
+	init_sockaddr_inet6_bin(&ipv6_any_addr, &in6addr_any, 0);
+}
+
+static void *peer_thread(void *tc_ptr)
+{
+	const struct test_case *tc = (struct test_case*)tc_ptr;
+	int sock;
+	unsigned request;
+	const char *response;
+
+	sock = SAFE_SOCKET(tc->address->sa_family, tc->type, tc->protocol);
+	SAFE_CONNECT(sock, tc->address, tc->addrlen);
+	SAFE_READ(1, sock, &request, sizeof(request));
+
+	if (request < ARRAY_SIZE(testcase_list)) {
+		response = testcase_list[request].description;
+	} else {
+		response = "Invalid request value";
+	}
+
+	SAFE_WRITE(1, sock, response, strlen(response) + 1);
+	SAFE_CLOSE(sock);
+	return NULL;
+}
+
+static void test_bind(unsigned int n)
+{
+	struct test_case tc_copy, *tc = testcase_list + n;
+	struct sockaddr_storage listen_addr, remote_addr;
+	struct sockaddr_un *tmp_addr;
+	socklen_t remote_len = sizeof(struct sockaddr_storage);
+	int listen_sock, sock, size;
+	unsigned rand_index;
+	pthread_t thread_id;
+	char buffer[BUFFER_SIZE];
+	const char *exp_data;
+
+	// Create listen socket
+	tst_res(TINFO, "Testing %s", tc->description);
+	listen_sock = SAFE_SOCKET(tc->address->sa_family, tc->type,
+		tc->protocol);
+
+	TEST(bind(listen_sock, tc->address, tc->addrlen));
+
+	if (TST_RET) {
+		tst_res(TFAIL | TERRNO, "bind() failed");
+		close(listen_sock);
+		return;
+	}
+
+	// IPv4/IPv6 tests use wildcard addresses, resolve a valid connection
+	// address for peer thread
+	memcpy(&tc_copy, tc, sizeof(struct test_case));
+	tc_copy.addrlen = get_connect_address(listen_sock, &listen_addr);
+	tc_copy.address = (struct sockaddr*)&listen_addr;
+
+	// Start peer thread and wait for connection
+	SAFE_LISTEN(listen_sock, 1);
+	SAFE_PTHREAD_CREATE(&thread_id, NULL, peer_thread, &tc_copy);
+	sock = accept(listen_sock, (struct sockaddr*)&remote_addr,
+		&remote_len);
+
+	if (sock < 0) {
+		tst_brk(TBROK | TERRNO, "accept() failed");
+	}
+
+	// Send request
+	rand_index = rand() % ARRAY_SIZE(testcase_list);
+	SAFE_WRITE(1, sock, &rand_index, sizeof(rand_index));
+
+	// Read response
+	size = SAFE_READ(0, sock, buffer, BUFFER_SIZE - 1);
+	buffer[size] = '\0';
+	exp_data = testcase_list[rand_index].description;
+
+	if (!strcmp(buffer, exp_data)) {
+		tst_res(TPASS, "Communication successful");
+	} else {
+		tst_res(TFAIL, "Received invalid data. Expected: \"%s\". "
+			"Received: \"%s\"", exp_data, buffer);
+	}
+
+	// Cleanup
+	SAFE_CLOSE(sock);
+	SAFE_CLOSE(listen_sock);
+	pthread_join(thread_id, NULL);
+	tmp_addr = (struct sockaddr_un*)tc->address;
+
+	if (tc->address->sa_family == AF_UNIX && tmp_addr->sun_path[0]) {
+		SAFE_UNLINK(tmp_addr->sun_path);
+	}
+}
+
+static struct tst_test test = {
+	.test = test_bind,
+	.tcnt = ARRAY_SIZE(testcase_list),
+	.needs_tmpdir = 1,
+	.setup = setup,
+};
diff --git a/testcases/kernel/syscalls/bind/bind05.c b/testcases/kernel/syscalls/bind/bind05.c
new file mode 100644
index 000000000..5532843e6
--- /dev/null
+++ b/testcases/kernel/syscalls/bind/bind05.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
+ */
+
+/*
+ * Create and bind socket for various standard datagram protocols.
+ * Then connect to it and send some test data.
+ */
+
+#include <string.h>
+#include <stdlib.h>
+#include <time.h>
+#include <pthread.h>
+
+#include "tst_test.h"
+#include "tst_net.h"
+#include "tst_safe_pthread.h"
+#include "libbind.h"
+
+static struct sockaddr_un unix_addr = {
+	.sun_family = AF_UNIX,
+	.sun_path = MAIN_SOCKET_FILE
+};
+static struct sockaddr_un abstract_addr = {
+	.sun_family = AF_UNIX,
+	.sun_path = ABSTRACT_SOCKET_PATH
+};
+static struct sockaddr_un peer_addr = {
+	.sun_family = AF_UNIX,
+	.sun_path = PEER_SOCKET_FILE
+};
+static struct sockaddr_in ipv4_addr;
+static struct sockaddr_in ipv4_any_addr;
+static struct sockaddr_in6 ipv6_addr;
+static struct sockaddr_in6 ipv6_any_addr;
+
+static struct test_case testcase_list[] = {
+	// UNIX sockets
+	{SOCK_DGRAM, 0, (struct sockaddr*)&unix_addr, sizeof(unix_addr),
+		"AF_UNIX pathname datagram"},
+	{SOCK_DGRAM, 0, (struct sockaddr*)&abstract_addr,
+		sizeof(abstract_addr), "AF_UNIX abstract datagram"},
+
+	// IPv4 sockets
+	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv4_addr, sizeof(ipv4_addr),
+		"IPv4 loop UDP variant 1"},
+	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv4_addr,
+		sizeof(ipv4_addr), "IPv4 loop UDP variant 2"},
+	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv4_addr,
+		sizeof(ipv4_addr), "IPv4 loop UDP-Lite"},
+	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv4_any_addr,
+		sizeof(ipv4_any_addr), "IPv4 any UDP variant 1"},
+	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv4_any_addr,
+		sizeof(ipv4_any_addr), "IPv4 any UDP variant 2"},
+	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv4_any_addr,
+		sizeof(ipv4_any_addr), "IPv4 any UDP-Lite"},
+
+	// IPv6 sockets
+	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv6_addr, sizeof(ipv6_addr),
+		"IPv6 loop UDP variant 1"},
+	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv6_addr,
+		sizeof(ipv6_addr), "IPv6 loop UDP variant 2"},
+	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv6_addr,
+		sizeof(ipv6_addr), "IPv6 loop UDP-Lite"},
+	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv6_any_addr,
+		sizeof(ipv6_any_addr), "IPv6 any UDP variant 1"},
+	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv6_any_addr,
+		sizeof(ipv6_any_addr), "IPv6 any UDP variant 2"},
+	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv6_any_addr,
+		sizeof(ipv6_any_addr), "IPv6 any UDP-Lite"}
+};
+
+static void setup(void)
+{
+	srand(time(0));
+
+	// Init addresses for IPv4/IPv6 test cases
+	init_sockaddr_inet(&ipv4_addr, IPV4_ADDRESS, 0);
+	init_sockaddr_inet_bin(&ipv4_any_addr, INADDR_ANY, 0);
+	init_sockaddr_inet6_bin(&ipv6_addr, &in6addr_loopback, 0);
+	init_sockaddr_inet6_bin(&ipv6_any_addr, &in6addr_any, 0);
+}
+
+static void *peer_thread(void *tc_ptr)
+{
+	const struct test_case *tc = (struct test_case*)tc_ptr;
+	int sock;
+	unsigned request = 0;
+	const char *response;
+
+	sock = SAFE_SOCKET(tc->address->sa_family, tc->type, tc->protocol);
+
+	// Both sides of AF_UNIX/SOCK_DGRAM socket must be bound for
+	// bidirectional communication
+	if (tc->address->sa_family == AF_UNIX) {
+		SAFE_BIND(sock, (struct sockaddr*)&peer_addr,
+			sizeof(struct sockaddr_un));
+	}
+
+	SAFE_CONNECT(sock, tc->address, tc->addrlen);
+	SAFE_WRITE(1, sock, &request, sizeof(request));
+	SAFE_READ(1, sock, &request, sizeof(request));
+
+	if (request < ARRAY_SIZE(testcase_list)) {
+		response = testcase_list[request].description;
+	} else {
+		response = "Invalid request value";
+	}
+
+	SAFE_WRITE(1, sock, response, strlen(response) + 1);
+	SAFE_CLOSE(sock);
+
+	if (tc->address->sa_family == AF_UNIX) {
+		SAFE_UNLINK(PEER_SOCKET_FILE);
+	}
+
+	return NULL;
+}
+
+static void test_bind(unsigned int n)
+{
+	struct test_case tc_copy, *tc = testcase_list + n;
+	struct sockaddr_storage listen_addr, remote_addr;
+	struct sockaddr_un *tmp_addr;
+	socklen_t remote_len = sizeof(struct sockaddr_storage);
+	int sock, size;
+	unsigned rand_index;
+	pthread_t thread_id;
+	char buffer[BUFFER_SIZE];
+	const char *exp_data;
+
+	// Create listen socket
+	tst_res(TINFO, "Testing %s", tc->description);
+	sock = SAFE_SOCKET(tc->address->sa_family, tc->type, tc->protocol);
+
+	TEST(bind(sock, tc->address, tc->addrlen));
+
+	if (TST_RET) {
+		tst_res(TFAIL | TERRNO, "bind() failed");
+		close(sock);
+		return;
+	}
+
+	// IPv4/IPv6 tests use wildcard addresses, resolve a valid connection
+	// address for peer thread
+	memcpy(&tc_copy, tc, sizeof(struct test_case));
+	tc_copy.addrlen = get_connect_address(sock, &listen_addr);
+	tc_copy.address = (struct sockaddr*)&listen_addr;
+
+	// Start peer thread and wait for connection
+	SAFE_PTHREAD_CREATE(&thread_id, NULL, peer_thread, &tc_copy);
+	size = recvfrom(sock, &rand_index, sizeof(rand_index), 0,
+		(struct sockaddr*)&remote_addr, &remote_len);
+
+	if (size != sizeof(rand_index)) {
+		tst_brk(TBROK | TERRNO, "Error while waiting for connection");
+	}
+
+	// Send request
+	rand_index = rand() % ARRAY_SIZE(testcase_list);
+	SAFE_SENDTO(1, sock, &rand_index, sizeof(rand_index), 0,
+		(struct sockaddr*)&remote_addr, remote_len);
+
+	// Read test data
+	size = SAFE_READ(0, sock, buffer, BUFFER_SIZE - 1);
+	buffer[size] = '\0';
+	exp_data = testcase_list[rand_index].description;
+
+	if (!strcmp(buffer, exp_data)) {
+		tst_res(TPASS, "Communication successful");
+	} else {
+		tst_res(TFAIL, "Received invalid data. Expected: \"%s\". "
+			"Received: \"%s\"", exp_data, buffer);
+	}
+
+	// Cleanup
+	SAFE_CLOSE(sock);
+	pthread_join(thread_id, NULL);
+	tmp_addr = (struct sockaddr_un*)tc->address;
+
+	if (tc->address->sa_family == AF_UNIX && tmp_addr->sun_path[0]) {
+		SAFE_UNLINK(tmp_addr->sun_path);
+	}
+}
+
+static struct tst_test test = {
+	.test = test_bind,
+	.tcnt = ARRAY_SIZE(testcase_list),
+	.needs_tmpdir = 1,
+	.setup = setup,
+};
diff --git a/testcases/kernel/syscalls/bind/libbind.h b/testcases/kernel/syscalls/bind/libbind.h
new file mode 100644
index 000000000..e19758f1b
--- /dev/null
+++ b/testcases/kernel/syscalls/bind/libbind.h
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
+ */
+
+/*
+ * Common settings and data types for bind() connection tests
+ */
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <netinet/ip.h>
+
+#define MAIN_SOCKET_FILE "test.sock"
+#define ABSTRACT_SOCKET_PATH "\0test.sock"
+#define PEER_SOCKET_FILE "peer.sock"
+#define IPV4_ADDRESS "127.0.0.1"
+#define IPV6_ADDRESS "::1"
+#define BUFFER_SIZE 128
+
+struct test_case {
+	int type, protocol;
+	struct sockaddr *address;
+	socklen_t addrlen;
+	const char *description;
+};
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 1/4] Update syscalls/bind02 to new API
  2019-09-26 15:13 ` [LTP] [PATCH v2 1/4] Update syscalls/bind02 to new API Martin Doucha
@ 2019-10-04 12:36   ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2019-10-04 12:36 UTC (permalink / raw)
  To: ltp

Hi!
Pushed with minor changes, thanks.

* The new test library runs the test in a forked process, there is no
  need to reset the effective GID and UID in the cleanup anymore, so
  I've removed the cleanup function.

* Changed the test to explicitly test for -1 as a return value. We have
  to be pedantic here. We had bugs where syscalls returned non-zero
  value which was != -1 which broke programs that were explicitly
  testing for -1.

+ Minor things such as:
  - using TTERRNO instead of TERRNO, which means that you print the
    value of TST_ERR instead of errno which could be clobbered because
    you call close() after bind()
  - declared run() and setup() as a static functions
  - make use of SAFE_CLOSE() instead of close()

Full diff:

diff --git a/testcases/kernel/syscalls/bind/bind02.c b/testcases/kernel/syscalls/bind/bind02.c
index 532831265..65944cbe3 100644
--- a/testcases/kernel/syscalls/bind/bind02.c
+++ b/testcases/kernel/syscalls/bind/bind02.c
@@ -26,7 +26,7 @@
 #define TCP_PRIVILEGED_PORT 463
 #define TEST_USERNAME "nobody"
 
-void run(void)
+static void run(void)
 {
 	struct sockaddr_in servaddr;
 	int sockfd;
@@ -37,18 +37,18 @@ void run(void)
 	servaddr.sin_port = htons(TCP_PRIVILEGED_PORT);
 	servaddr.sin_addr.s_addr = htonl(INADDR_ANY);
 	TEST(bind(sockfd, (struct sockaddr *)&servaddr, sizeof(servaddr)));
-	close(sockfd);
+	SAFE_CLOSE(sockfd);
 
-	if (!TST_RET) {
-		tst_res(TFAIL, "bind() succeeded unexpectedly");
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "bind() returned %li, expected -1", TST_RET);
 	} else if (TST_ERR == EACCES) {
-		tst_res(TPASS, "bind() failed as expected");
+		tst_res(TPASS | TTERRNO, "bind() failed as expected");
 	} else {
-		tst_res(TFAIL | TERRNO, "Unexpected error");
+		tst_res(TFAIL | TTERRNO, "Unexpected error");
 	}
 }
 
-void setup(void)
+static void setup(void)
 {
 	struct passwd *pw;
 	struct group *gr;
@@ -62,15 +62,8 @@ void setup(void)
 	SAFE_SETEUID(pw->pw_uid);
 }
 
-void cleanup(void)
-{
-	SAFE_SETEGID(0);
-	SAFE_SETEUID(0);
-}
-
 static struct tst_test test = {
 	.test_all = run,
 	.needs_root = 1,
 	.setup = setup,
-	.cleanup = cleanup
 };

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 2/4] Create separate .c file for include/tst_net.h
  2019-09-26 15:13 ` [LTP] [PATCH v2 2/4] Create separate .c file for include/tst_net.h Martin Doucha
@ 2019-10-04 12:40   ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2019-10-04 12:40 UTC (permalink / raw)
  To: ltp

Hi!
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  include/tst_net.h | 118 +++-------------------------------------
>  lib/tst_net.c     | 134 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 142 insertions(+), 110 deletions(-)
>  create mode 100644 lib/tst_net.c

What is the reason for this change?

With this you are polluting the LTP test library with symbols that are
not prefixed by tst_. If nothing else these functions has to be prefixed
in order to avoid conflicts with functions declared in the testcases.

> diff --git a/include/tst_net.h b/include/tst_net.h
> index cb97b7b61..740f25bac 100644
> --- a/include/tst_net.h
> +++ b/include/tst_net.h
> @@ -16,7 +16,7 @@
>   */
>  
>  #include <arpa/inet.h>
> -#include <errno.h>
> +#include <stdio.h>
>  
>  #define MAX_IPV4_PREFIX 32
>  #define MAX_IPV6_PREFIX 128
> @@ -42,112 +42,10 @@ static inline void print_svar_change(const char *name, const char *val)
>  		printf("export %s=\"${%s:-%s}\"\n", name, name, val);
>  }
>  
> -/*
> - * Function bit_count is from ipcalc project, ipcalc.c.
> - */
> -static int bit_count(uint32_t i)
> -{
> -	int c = 0;
> -	unsigned int seen_one = 0;
> -
> -	while (i > 0) {
> -		if (i & 1) {
> -			seen_one = 1;
> -			c++;
> -		} else {
> -			if (seen_one)
> -				return -1;
> -		}
> -		i >>= 1;
> -	}
> -
> -	return c;
> -}
> -
> -/*
> - * Function mask2prefix is from ipcalc project, ipcalc.c.
> - */
> -static int mask2prefix(struct in_addr mask)
> -{
> -	return bit_count(ntohl(mask.s_addr));
> -}
> -
> -/*
> - * Function ipv4_mask_to_int is from ipcalc project, ipcalc.c.
> - */
> -static int ipv4_mask_to_int(const char *prefix)
> -{
> -	int ret;
> -	struct in_addr in;
> -
> -	ret = inet_pton(AF_INET, prefix, &in);
> -	if (ret == 0)
> -		return -1;
> -
> -	return mask2prefix(in);
> -}
> -
> -/*
> - * Function safe_atoi is from ipcalc project, ipcalc.c.
> - */
> -static int safe_atoi(const char *s, int *ret_i)
> -{
> -	char *x = NULL;
> -	long l;
> -
> -	errno = 0;
> -	l = strtol(s, &x, 0);
> -
> -	if (!x || x == s || *x || errno)
> -		return errno > 0 ? -errno : -EINVAL;
> -
> -	if ((long)(int)l != l)
> -		return -ERANGE;
> -
> -	*ret_i = (int)l;
> -
> -	return 0;
> -}
> -
> -/*
> - * Function get_prefix use code from ipcalc project, str_to_prefix/ipcalc.c.
> - */
> -static int get_prefix(const char *ip_str, int is_ipv6)
> -{
> -	char *prefix_str = NULL;
> -	int prefix = -1, r;
> -
> -	prefix_str = strchr(ip_str, '/');
> -	if (!prefix_str)
> -		return -1;
> -
> -	*(prefix_str++) = '\0';
> -
> -	if (!is_ipv6 && strchr(prefix_str, '.'))
> -		prefix = ipv4_mask_to_int(prefix_str);
> -	else {
> -		r = safe_atoi(prefix_str, &prefix);
> -		if (r != 0)
> -			tst_brk_comment("conversion error: '%s' is not integer",
> -					prefix_str);
> -	}
> -
> -	if (prefix < 0 || ((is_ipv6 && prefix > MAX_IPV6_PREFIX) ||
> -		(!is_ipv6 && prefix > MAX_IPV4_PREFIX)))
> -		tst_brk_comment("bad %s prefix: %s", is_ipv6 ?  "IPv6" : "IPv4",
> -				prefix_str);
> -
> -	return prefix;
> -}
> -
> -static void get_in_addr(const char *ip_str, struct in_addr *ip)
> -{
> -	if (inet_pton(AF_INET, ip_str, ip) <= 0)
> -		tst_brk_comment("bad IPv4 address: '%s'", ip_str);
> -}
> -
> -static void get_in6_addr(const char *ip_str, struct in6_addr *ip6)
> -{
> -	if (inet_pton(AF_INET6, ip_str, ip6) <= 0)
> -		tst_brk_comment("bad IPv6 address: '%s'", ip_str);
> -}
> +int bit_count(uint32_t i);
> +int mask2prefix(struct in_addr mask);
> +int ipv4_mask_to_int(const char *prefix);
> +int safe_atoi(const char *s, int *ret_i);
> +int get_prefix(const char *ip_str, int is_ipv6);
> +void get_in_addr(const char *ip_str, struct in_addr *ip);
> +void get_in6_addr(const char *ip_str, struct in6_addr *ip6);
> diff --git a/lib/tst_net.c b/lib/tst_net.c
> new file mode 100644
> index 000000000..4166641f1
> --- /dev/null
> +++ b/lib/tst_net.c
> @@ -0,0 +1,134 @@
> +/*
> + * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <errno.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_net.h"
> +
> +/*
> + * Function bit_count is from ipcalc project, ipcalc.c.
> + */
> +int bit_count(uint32_t i)
> +{
> +	int c = 0;
> +	unsigned int seen_one = 0;
> +
> +	while (i > 0) {
> +		if (i & 1) {
> +			seen_one = 1;
> +			c++;
> +		} else {
> +			if (seen_one)
> +				return -1;
> +		}
> +		i >>= 1;
> +	}
> +
> +	return c;
> +}
> +
> +/*
> + * Function mask2prefix is from ipcalc project, ipcalc.c.
> + */
> +int mask2prefix(struct in_addr mask)
> +{
> +	return bit_count(ntohl(mask.s_addr));
> +}
> +
> +/*
> + * Function ipv4_mask_to_int is from ipcalc project, ipcalc.c.
> + */
> +int ipv4_mask_to_int(const char *prefix)
> +{
> +	int ret;
> +	struct in_addr in;
> +
> +	ret = inet_pton(AF_INET, prefix, &in);
> +	if (ret == 0)
> +		return -1;
> +
> +	return mask2prefix(in);
> +}
> +
> +/*
> + * Function safe_atoi is from ipcalc project, ipcalc.c.
> + */
> +int safe_atoi(const char *s, int *ret_i)
> +{
> +	char *x = NULL;
> +	long l;
> +
> +	errno = 0;
> +	l = strtol(s, &x, 0);
> +
> +	if (!x || x == s || *x || errno)
> +		return errno > 0 ? -errno : -EINVAL;
> +
> +	if ((long)(int)l != l)
> +		return -ERANGE;
> +
> +	*ret_i = (int)l;
> +
> +	return 0;
> +}
> +
> +/*
> + * Function get_prefix use code from ipcalc project, str_to_prefix/ipcalc.c.
> + */
> +int get_prefix(const char *ip_str, int is_ipv6)
> +{
> +	char *prefix_str = NULL;
> +	int prefix = -1, r;
> +
> +	prefix_str = strchr(ip_str, '/');
> +	if (!prefix_str)
> +		return -1;
> +
> +	*(prefix_str++) = '\0';
> +
> +	if (!is_ipv6 && strchr(prefix_str, '.'))
> +		prefix = ipv4_mask_to_int(prefix_str);
> +	else {
> +		r = safe_atoi(prefix_str, &prefix);
> +		if (r != 0)
> +			tst_brk_comment("conversion error: '%s' is not integer",
> +					prefix_str);
> +	}
> +
> +	if (prefix < 0 || ((is_ipv6 && prefix > MAX_IPV6_PREFIX) ||
> +		(!is_ipv6 && prefix > MAX_IPV4_PREFIX)))
> +		tst_brk_comment("bad %s prefix: %s", is_ipv6 ?  "IPv6" : "IPv4",
> +				prefix_str);
> +
> +	return prefix;
> +}
> +
> +void get_in_addr(const char *ip_str, struct in_addr *ip)
> +{
> +	if (inet_pton(AF_INET, ip_str, ip) <= 0)
> +		tst_brk_comment("bad IPv4 address: '%s'", ip_str);
> +}
> +
> +void get_in6_addr(const char *ip_str, struct in6_addr *ip6)
> +{
> +	if (inet_pton(AF_INET6, ip_str, ip6) <= 0)
> +		tst_brk_comment("bad IPv6 address: '%s'", ip_str);
> +}
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 3/4] Add socket address initialization functions to tst_net library
  2019-09-26 15:13 ` [LTP] [PATCH v2 3/4] Add socket address initialization functions to tst_net library Martin Doucha
@ 2019-10-04 12:42   ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2019-10-04 12:42 UTC (permalink / raw)
  To: ltp

Hi!
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  include/tst_net.h | 16 +++++++++++
>  lib/tst_net.c     | 71 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 87 insertions(+)
> 
> diff --git a/include/tst_net.h b/include/tst_net.h
> index 740f25bac..2c958da13 100644
> --- a/include/tst_net.h
> +++ b/include/tst_net.h
> @@ -17,6 +17,9 @@
>  
>  #include <arpa/inet.h>
>  #include <stdio.h>
> +#include <sys/types.h>
> +#include <netinet/in.h>
> +#include <netinet/ip.h>
>  
>  #define MAX_IPV4_PREFIX 32
>  #define MAX_IPV6_PREFIX 128
> @@ -49,3 +52,16 @@ int safe_atoi(const char *s, int *ret_i);
>  int get_prefix(const char *ip_str, int is_ipv6);
>  void get_in_addr(const char *ip_str, struct in_addr *ip);
>  void get_in6_addr(const char *ip_str, struct in6_addr *ip6);
> +
> +/*
> + * Find valid connection address for a given bound socket
> + */
> +socklen_t get_connect_address(int sock, struct sockaddr_storage *addr);
> +
> +/*
> + * Initialize AF_INET/AF_INET6 socket address structure with address and port
> + */
> +void init_sockaddr_inet(struct sockaddr_in *sa, const char *ip_str, uint16_t port);
> +void init_sockaddr_inet_bin(struct sockaddr_in *sa, uint32_t ip_val, uint16_t port);
> +void init_sockaddr_inet6(struct sockaddr_in6 *sa, const char *ip_str, uint16_t port);
> +void init_sockaddr_inet6_bin(struct sockaddr_in6 *sa, const struct in6_addr *ip_val, uint16_t port);
> diff --git a/lib/tst_net.c b/lib/tst_net.c
> index 4166641f1..4ccd81eb9 100644
> --- a/lib/tst_net.c
> +++ b/lib/tst_net.c
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
> + * Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
>   *
>   * This program is free software: you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -132,3 +133,73 @@ void get_in6_addr(const char *ip_str, struct in6_addr *ip6)
>  	if (inet_pton(AF_INET6, ip_str, ip6) <= 0)
>  		tst_brk_comment("bad IPv6 address: '%s'", ip_str);
>  }
> +
> +socklen_t get_connect_address(int sock, struct sockaddr_storage *addr)
> +{
> +	struct sockaddr_in *inet_ptr;
> +	struct sockaddr_in6 *inet6_ptr;
> +	size_t tmp_size;
> +	socklen_t ret = sizeof(*addr);
> +
> +	SAFE_GETSOCKNAME(sock, (struct sockaddr*)addr, &ret);
> +
> +	// Sanitize wildcard addresses
> +	switch (addr->ss_family) {
> +	case AF_INET:
> +		inet_ptr = (struct sockaddr_in*)addr;
> +
> +		switch (ntohl(inet_ptr->sin_addr.s_addr)) {
> +		case INADDR_ANY:
> +		case INADDR_BROADCAST:
> +			inet_ptr->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> +			break;
> +		}
> +
> +		break;
> +
> +	case AF_INET6:
> +		inet6_ptr = (struct sockaddr_in6*)addr;
> +		tmp_size = sizeof(struct in6_addr);
> +
> +		if (!memcmp(&inet6_ptr->sin6_addr, &in6addr_any, tmp_size)) {
> +			memcpy(&inet6_ptr->sin6_addr, &in6addr_loopback,
> +				tmp_size);
> +		}
> +
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +void init_sockaddr_inet(struct sockaddr_in *sa, const char *ip_str, uint16_t port)
> +{
> +	memset(sa, 0, sizeof(struct sockaddr_in));
> +	sa->sin_family = AF_INET;
> +	sa->sin_port = htons(port);
> +	get_in_addr(ip_str, &sa->sin_addr);
> +}
> +
> +void init_sockaddr_inet_bin(struct sockaddr_in *sa, uint32_t ip_val, uint16_t port)
> +{
> +	memset(sa, 0, sizeof(struct sockaddr_in));
> +	sa->sin_family = AF_INET;
> +	sa->sin_port = htons(port);
> +	sa->sin_addr.s_addr = htonl(ip_val);
> +}
> +
> +void init_sockaddr_inet6(struct sockaddr_in6 *sa, const char *ip_str, uint16_t port)
> +{
> +	memset(sa, 0, sizeof(struct sockaddr_in6));
> +	sa->sin6_family = AF_INET6;
> +	sa->sin6_port = htons(port);
> +	get_in6_addr(ip_str, &sa->sin6_addr);
> +}
> +
> +void init_sockaddr_inet6_bin(struct sockaddr_in6 *sa, const struct in6_addr *ip_val, uint16_t port)
> +{
> +	memset(sa, 0, sizeof(struct sockaddr_in6));
> +	sa->sin6_family = AF_INET6;
> +	sa->sin6_port = htons(port);
> +	memcpy(&sa->sin6_addr, ip_val, sizeof(struct in6_addr));
> +}

These init functions should really be just static inline functions
instead, there is no point in putting that code into a library code.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v2 4/4] Add connection tests for bind()
  2019-09-26 15:13 ` [LTP] [PATCH v2 4/4] Add connection tests for bind() Martin Doucha
@ 2019-10-04 13:03   ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2019-10-04 13:03 UTC (permalink / raw)
  To: ltp

Hi!
> diff --git a/testcases/kernel/syscalls/bind/Makefile b/testcases/kernel/syscalls/bind/Makefile
> index bd617d806..1fe390997 100644
> --- a/testcases/kernel/syscalls/bind/Makefile
> +++ b/testcases/kernel/syscalls/bind/Makefile
> @@ -20,4 +20,6 @@ top_srcdir		?= ../../../..
>  
>  include $(top_srcdir)/include/mk/testcases.mk
>  
> +bind04 bind05:	LDLIBS		+= -lpthread

This should be CFLAGS += -phread istead, otherwise strange bugs will
happen on older distributions/less common architectures. See SYNOPSIS
for 'man pthread_create' for example.

>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/bind/bind04.c b/testcases/kernel/syscalls/bind/bind04.c
> new file mode 100644
> index 000000000..bd71ca82e
> --- /dev/null
> +++ b/testcases/kernel/syscalls/bind/bind04.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
> + */
> +
> +/*
> + * Create and bind socket for various standard stream protocols.
> + * Then connect to it and send some test data.
> + */
> +
> +#include <string.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <pthread.h>
> +
> +#include "tst_test.h"
> +#include "tst_net.h"
> +#include "tst_safe_pthread.h"
> +#include "libbind.h"
> +
> +static struct sockaddr_un unix_addr = {
> +	.sun_family = AF_UNIX,
> +	.sun_path = MAIN_SOCKET_FILE
> +};
> +static struct sockaddr_un abstract_addr = {
> +	.sun_family = AF_UNIX,
> +	.sun_path = ABSTRACT_SOCKET_PATH
> +};
> +static struct sockaddr_in ipv4_addr;
> +static struct sockaddr_in ipv4_any_addr;
> +static struct sockaddr_in6 ipv6_addr;
> +static struct sockaddr_in6 ipv6_any_addr;
> +
> +static struct test_case testcase_list[] = {
> +	// UNIX sockets
> +	{SOCK_STREAM, 0, (struct sockaddr*)&unix_addr, sizeof(unix_addr),
> +		"AF_UNIX pathname stream"},
> +	{SOCK_SEQPACKET, 0, (struct sockaddr*)&unix_addr, sizeof(unix_addr),
> +		"AF_UNIX pathname seqpacket"},
> +	{SOCK_STREAM, 0, (struct sockaddr*)&abstract_addr,
> +		sizeof(abstract_addr), "AF_UNIX abstract stream"},
> +	{SOCK_SEQPACKET, 0, (struct sockaddr*)&abstract_addr,
> +		sizeof(abstract_addr), "AF_UNIX abstract seqpacket"},
> +
> +	// IPv4 sockets
> +	{SOCK_STREAM, 0, (struct sockaddr*)&ipv4_addr, sizeof(ipv4_addr),
> +		"IPv4 loop TCP variant 1"},
> +	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv4_addr,
> +		sizeof(ipv4_addr), "IPv4 loop TCP variant 2"},
> +	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv4_addr,
> +		sizeof(ipv4_addr), "IPv4 loop SCTP"},
> +	{SOCK_STREAM, 0, (struct sockaddr*)&ipv4_any_addr,
> +		sizeof(ipv4_any_addr), "IPv4 any TCP variant 1"},
> +	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv4_any_addr,
> +		sizeof(ipv4_any_addr), "IPv4 any TCP variant 2"},
> +	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv4_any_addr,
> +		sizeof(ipv4_any_addr), "IPv4 any SCTP"},
> +
> +	// IPv6 sockets
> +	{SOCK_STREAM, 0, (struct sockaddr*)&ipv6_addr, sizeof(ipv6_addr),
> +		"IPv6 loop TCP variant 1"},
> +	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv6_addr,
> +		sizeof(ipv6_addr), "IPv6 loop TCP variant 2"},
> +	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv6_addr,
> +		sizeof(ipv6_addr), "IPv6 loop SCTP"},
> +	{SOCK_STREAM, 0, (struct sockaddr*)&ipv6_any_addr,
> +		sizeof(ipv6_any_addr), "IPv6 any TCP variant 1"},
> +	{SOCK_STREAM, IPPROTO_TCP, (struct sockaddr*)&ipv6_any_addr,
> +		sizeof(ipv6_any_addr), "IPv6 any TCP variant 2"},
> +	{SOCK_STREAM, IPPROTO_SCTP, (struct sockaddr*)&ipv6_any_addr,
> +		sizeof(ipv6_any_addr), "IPv6 any SCTP"}
> +};
> +
> +static void setup(void)
> +{
> +	srand(time(0));
> +
> +	// Init addresses for IPv4/IPv6 test cases

Here you are commenting the obvious, it's pretty clear from the
functions names what we do here. Generally I tend to comment only cases
that are not clear from the code, and even avoid code that is not clear
enough...

See also: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#commenting

> +	init_sockaddr_inet(&ipv4_addr, IPV4_ADDRESS, 0);
> +	init_sockaddr_inet_bin(&ipv4_any_addr, INADDR_ANY, 0);
> +	init_sockaddr_inet6_bin(&ipv6_addr, &in6addr_loopback, 0);
> +	init_sockaddr_inet6_bin(&ipv6_any_addr, &in6addr_any, 0);
> +}
> +
> +static void *peer_thread(void *tc_ptr)
> +{
> +	const struct test_case *tc = (struct test_case*)tc_ptr;
                                      ^
				      This is an useless cast.
> +	int sock;
> +	unsigned request;
> +	const char *response;
> +
> +	sock = SAFE_SOCKET(tc->address->sa_family, tc->type, tc->protocol);
> +	SAFE_CONNECT(sock, tc->address, tc->addrlen);
> +	SAFE_READ(1, sock, &request, sizeof(request));
> +
> +	if (request < ARRAY_SIZE(testcase_list)) {
> +		response = testcase_list[request].description;
> +	} else {
> +		response = "Invalid request value";
> +	}

This is very minor however LKML coding style prefers not to have curly
braces around blocks that consists only of a single line.

> +	SAFE_WRITE(1, sock, response, strlen(response) + 1);
> +	SAFE_CLOSE(sock);
> +	return NULL;
> +}
> +
> +static void test_bind(unsigned int n)
> +{
> +	struct test_case tc_copy, *tc = testcase_list + n;
> +	struct sockaddr_storage listen_addr, remote_addr;
> +	struct sockaddr_un *tmp_addr;
> +	socklen_t remote_len = sizeof(struct sockaddr_storage);
> +	int listen_sock, sock, size;
> +	unsigned rand_index;
> +	pthread_t thread_id;
> +	char buffer[BUFFER_SIZE];
> +	const char *exp_data;
> +
> +	// Create listen socket

Here as well, we basically do listen_sock = socket(...) here, why do we
add comment that does not add any more value that the actuall code?

> +	tst_res(TINFO, "Testing %s", tc->description);
> +	listen_sock = SAFE_SOCKET(tc->address->sa_family, tc->type,
> +		tc->protocol);
> +
> +	TEST(bind(listen_sock, tc->address, tc->addrlen));
> +
> +	if (TST_RET) {
> +		tst_res(TFAIL | TERRNO, "bind() failed");
> +		close(listen_sock);
> +		return;
> +	}
> +
> +	// IPv4/IPv6 tests use wildcard addresses, resolve a valid connection
> +	// address for peer thread
> +	memcpy(&tc_copy, tc, sizeof(struct test_case));
> +	tc_copy.addrlen = get_connect_address(listen_sock, &listen_addr);
> +	tc_copy.address = (struct sockaddr*)&listen_addr;
> +
> +	// Start peer thread and wait for connection
> +	SAFE_LISTEN(listen_sock, 1);
> +	SAFE_PTHREAD_CREATE(&thread_id, NULL, peer_thread, &tc_copy);
> +	sock = accept(listen_sock, (struct sockaddr*)&remote_addr,
> +		&remote_len);
> +
> +	if (sock < 0) {
		SAFE_CLOSE(listen_sock) here?
> +		tst_brk(TBROK | TERRNO, "accept() failed");
> +	}
> +
> +	// Send request
> +	rand_index = rand() % ARRAY_SIZE(testcase_list);
> +	SAFE_WRITE(1, sock, &rand_index, sizeof(rand_index));
> +
> +	// Read response
> +	size = SAFE_READ(0, sock, buffer, BUFFER_SIZE - 1);
> +	buffer[size] = '\0';
> +	exp_data = testcase_list[rand_index].description;
> +
> +	if (!strcmp(buffer, exp_data)) {
> +		tst_res(TPASS, "Communication successful");
> +	} else {
> +		tst_res(TFAIL, "Received invalid data. Expected: \"%s\". "
> +			"Received: \"%s\"", exp_data, buffer);

I tend to use single quotes (i.e. ') inside of C strings to avoid the
need to escape each and every double quote character...

> +	}
> +
> +	// Cleanup
> +	SAFE_CLOSE(sock);
> +	SAFE_CLOSE(listen_sock);
> +	pthread_join(thread_id, NULL);
> +	tmp_addr = (struct sockaddr_un*)tc->address;
> +
> +	if (tc->address->sa_family == AF_UNIX && tmp_addr->sun_path[0]) {
> +		SAFE_UNLINK(tmp_addr->sun_path);
> +	}

Well technically you can exit the test via tst_brk() in each SAFE_XXX()
call, which is the reason testcases usually cleanup asynchronously in
the cleanup() function. It's a bit more complex in a case of pthreads,
since you have to make sure that only one of the threads runs the
cleanup, it's described in:

https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#2214-thread-safety-in-the-ltp-library

In this case the unix socket will be deleted by the test library, if we
fail to do that and the sockets will be closed on the program exit, so
I guess that it's fine as it is.

> +}
> +
> +static struct tst_test test = {
> +	.test = test_bind,
> +	.tcnt = ARRAY_SIZE(testcase_list),
> +	.needs_tmpdir = 1,
> +	.setup = setup,
> +};
> diff --git a/testcases/kernel/syscalls/bind/bind05.c b/testcases/kernel/syscalls/bind/bind05.c
> new file mode 100644
> index 000000000..5532843e6
> --- /dev/null
> +++ b/testcases/kernel/syscalls/bind/bind05.c
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
> + */
> +
> +/*
> + * Create and bind socket for various standard datagram protocols.
> + * Then connect to it and send some test data.
> + */
> +
> +#include <string.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <pthread.h>
> +
> +#include "tst_test.h"
> +#include "tst_net.h"
> +#include "tst_safe_pthread.h"
> +#include "libbind.h"
> +
> +static struct sockaddr_un unix_addr = {
> +	.sun_family = AF_UNIX,
> +	.sun_path = MAIN_SOCKET_FILE
> +};
> +static struct sockaddr_un abstract_addr = {
> +	.sun_family = AF_UNIX,
> +	.sun_path = ABSTRACT_SOCKET_PATH
> +};
> +static struct sockaddr_un peer_addr = {
> +	.sun_family = AF_UNIX,
> +	.sun_path = PEER_SOCKET_FILE
> +};
> +static struct sockaddr_in ipv4_addr;
> +static struct sockaddr_in ipv4_any_addr;
> +static struct sockaddr_in6 ipv6_addr;
> +static struct sockaddr_in6 ipv6_any_addr;
> +
> +static struct test_case testcase_list[] = {
> +	// UNIX sockets
> +	{SOCK_DGRAM, 0, (struct sockaddr*)&unix_addr, sizeof(unix_addr),
> +		"AF_UNIX pathname datagram"},
> +	{SOCK_DGRAM, 0, (struct sockaddr*)&abstract_addr,
> +		sizeof(abstract_addr), "AF_UNIX abstract datagram"},
> +
> +	// IPv4 sockets
> +	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv4_addr, sizeof(ipv4_addr),
> +		"IPv4 loop UDP variant 1"},
> +	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv4_addr,
> +		sizeof(ipv4_addr), "IPv4 loop UDP variant 2"},
> +	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv4_addr,
> +		sizeof(ipv4_addr), "IPv4 loop UDP-Lite"},
> +	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv4_any_addr,
> +		sizeof(ipv4_any_addr), "IPv4 any UDP variant 1"},
> +	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv4_any_addr,
> +		sizeof(ipv4_any_addr), "IPv4 any UDP variant 2"},
> +	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv4_any_addr,
> +		sizeof(ipv4_any_addr), "IPv4 any UDP-Lite"},
> +
> +	// IPv6 sockets
> +	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv6_addr, sizeof(ipv6_addr),
> +		"IPv6 loop UDP variant 1"},
> +	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv6_addr,
> +		sizeof(ipv6_addr), "IPv6 loop UDP variant 2"},
> +	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv6_addr,
> +		sizeof(ipv6_addr), "IPv6 loop UDP-Lite"},
> +	{SOCK_DGRAM, 0, (struct sockaddr*)&ipv6_any_addr,
> +		sizeof(ipv6_any_addr), "IPv6 any UDP variant 1"},
> +	{SOCK_DGRAM, IPPROTO_UDP, (struct sockaddr*)&ipv6_any_addr,
> +		sizeof(ipv6_any_addr), "IPv6 any UDP variant 2"},
> +	{SOCK_DGRAM, IPPROTO_UDPLITE, (struct sockaddr*)&ipv6_any_addr,
> +		sizeof(ipv6_any_addr), "IPv6 any UDP-Lite"}
> +};
> +
> +static void setup(void)
> +{
> +	srand(time(0));
> +
> +	// Init addresses for IPv4/IPv6 test cases
> +	init_sockaddr_inet(&ipv4_addr, IPV4_ADDRESS, 0);
> +	init_sockaddr_inet_bin(&ipv4_any_addr, INADDR_ANY, 0);
> +	init_sockaddr_inet6_bin(&ipv6_addr, &in6addr_loopback, 0);
> +	init_sockaddr_inet6_bin(&ipv6_any_addr, &in6addr_any, 0);
> +}
> +
> +static void *peer_thread(void *tc_ptr)
> +{
> +	const struct test_case *tc = (struct test_case*)tc_ptr;
> +	int sock;
> +	unsigned request = 0;
> +	const char *response;
> +
> +	sock = SAFE_SOCKET(tc->address->sa_family, tc->type, tc->protocol);
> +
> +	// Both sides of AF_UNIX/SOCK_DGRAM socket must be bound for
> +	// bidirectional communication
> +	if (tc->address->sa_family == AF_UNIX) {
> +		SAFE_BIND(sock, (struct sockaddr*)&peer_addr,
> +			sizeof(struct sockaddr_un));
> +	}
> +
> +	SAFE_CONNECT(sock, tc->address, tc->addrlen);
> +	SAFE_WRITE(1, sock, &request, sizeof(request));
> +	SAFE_READ(1, sock, &request, sizeof(request));
> +
> +	if (request < ARRAY_SIZE(testcase_list)) {
> +		response = testcase_list[request].description;
> +	} else {
> +		response = "Invalid request value";
> +	}
> +
> +	SAFE_WRITE(1, sock, response, strlen(response) + 1);
> +	SAFE_CLOSE(sock);
> +
> +	if (tc->address->sa_family == AF_UNIX) {
> +		SAFE_UNLINK(PEER_SOCKET_FILE);
> +	}
> +
> +	return NULL;
> +}
> +
> +static void test_bind(unsigned int n)
> +{
> +	struct test_case tc_copy, *tc = testcase_list + n;
> +	struct sockaddr_storage listen_addr, remote_addr;
> +	struct sockaddr_un *tmp_addr;
> +	socklen_t remote_len = sizeof(struct sockaddr_storage);
> +	int sock, size;
> +	unsigned rand_index;
> +	pthread_t thread_id;
> +	char buffer[BUFFER_SIZE];
> +	const char *exp_data;
> +
> +	// Create listen socket
> +	tst_res(TINFO, "Testing %s", tc->description);
> +	sock = SAFE_SOCKET(tc->address->sa_family, tc->type, tc->protocol);
> +
> +	TEST(bind(sock, tc->address, tc->addrlen));
> +
> +	if (TST_RET) {
> +		tst_res(TFAIL | TERRNO, "bind() failed");
> +		close(sock);
> +		return;
> +	}
> +
> +	// IPv4/IPv6 tests use wildcard addresses, resolve a valid connection
> +	// address for peer thread
> +	memcpy(&tc_copy, tc, sizeof(struct test_case));
> +	tc_copy.addrlen = get_connect_address(sock, &listen_addr);
> +	tc_copy.address = (struct sockaddr*)&listen_addr;
> +
> +	// Start peer thread and wait for connection
> +	SAFE_PTHREAD_CREATE(&thread_id, NULL, peer_thread, &tc_copy);
> +	size = recvfrom(sock, &rand_index, sizeof(rand_index), 0,
> +		(struct sockaddr*)&remote_addr, &remote_len);
> +
> +	if (size != sizeof(rand_index)) {
> +		tst_brk(TBROK | TERRNO, "Error while waiting for connection");
> +	}
> +
> +	// Send request
> +	rand_index = rand() % ARRAY_SIZE(testcase_list);
> +	SAFE_SENDTO(1, sock, &rand_index, sizeof(rand_index), 0,
> +		(struct sockaddr*)&remote_addr, remote_len);
> +
> +	// Read test data
> +	size = SAFE_READ(0, sock, buffer, BUFFER_SIZE - 1);
> +	buffer[size] = '\0';
> +	exp_data = testcase_list[rand_index].description;
> +
> +	if (!strcmp(buffer, exp_data)) {
> +		tst_res(TPASS, "Communication successful");
> +	} else {
> +		tst_res(TFAIL, "Received invalid data. Expected: \"%s\". "
> +			"Received: \"%s\"", exp_data, buffer);
> +	}
> +
> +	// Cleanup
> +	SAFE_CLOSE(sock);
> +	pthread_join(thread_id, NULL);
> +	tmp_addr = (struct sockaddr_un*)tc->address;
> +
> +	if (tc->address->sa_family == AF_UNIX && tmp_addr->sun_path[0]) {
> +		SAFE_UNLINK(tmp_addr->sun_path);
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.test = test_bind,
> +	.tcnt = ARRAY_SIZE(testcase_list),
> +	.needs_tmpdir = 1,
> +	.setup = setup,
> +};

Same comments apply for this code as well.

> diff --git a/testcases/kernel/syscalls/bind/libbind.h b/testcases/kernel/syscalls/bind/libbind.h
> new file mode 100644
> index 000000000..e19758f1b
> --- /dev/null
> +++ b/testcases/kernel/syscalls/bind/libbind.h
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
> + */
> +
> +/*
> + * Common settings and data types for bind() connection tests
> + */
> +
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <arpa/inet.h>
> +#include <netinet/in.h>
> +#include <netinet/ip.h>
> +
> +#define MAIN_SOCKET_FILE "test.sock"
> +#define ABSTRACT_SOCKET_PATH "\0test.sock"
> +#define PEER_SOCKET_FILE "peer.sock"
> +#define IPV4_ADDRESS "127.0.0.1"
> +#define IPV6_ADDRESS "::1"
> +#define BUFFER_SIZE 128
> +
> +struct test_case {
> +	int type, protocol;
> +	struct sockaddr *address;
> +	socklen_t addrlen;
> +	const char *description;
> +};
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-10-04 13:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 15:13 [LTP] [PATCH v2 0/4] Increase bind() converage - GH#538 Martin Doucha
2019-09-26 15:13 ` [LTP] [PATCH v2 1/4] Update syscalls/bind02 to new API Martin Doucha
2019-10-04 12:36   ` Cyril Hrubis
2019-09-26 15:13 ` [LTP] [PATCH v2 2/4] Create separate .c file for include/tst_net.h Martin Doucha
2019-10-04 12:40   ` Cyril Hrubis
2019-09-26 15:13 ` [LTP] [PATCH v2 3/4] Add socket address initialization functions to tst_net library Martin Doucha
2019-10-04 12:42   ` Cyril Hrubis
2019-09-26 15:13 ` [LTP] [PATCH v2 4/4] Add connection tests for bind() Martin Doucha
2019-10-04 13:03   ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.