The branch, master has been updated
via 0422a95 test_echo_tcp_sendmsg_recvmsg_fd: add
test_tcp_sendmsg_recvmsg_fd_mixed() tests
via b35e3ce test_echo_tcp_sendmsg_recvmsg_fd: add
test_tcp_sendmsg_recvmsg_fd_different() tests
via 2d93f75 test_echo_tcp_sendmsg_recvmsg_fd: also test passing the
same socket up to 6 times
via 5aa939f test_echo_tcp_sendmsg_recvmsg_fd: split out
test_tcp_sendmsg_recvmsg_fd_same()
via cbd5910 test_echo_tcp_sendmsg_recvmsg_fd: split out
test_tcp_sendmsg_recvmsg_fd_array()
via b2e366d tests/echo_srv: allow more than once tcp connection at a
time
via e896cec swrap: fix fd-passing without 4 padding bytes
via 3c7ef47 swrap: fix invalid read in swrap_sendmsg_unix_scm_rights()
from 13a6aca swrap: fix copy on write leak of ~38M for every fork.
https://git.samba.org/?p=socket_wrapper.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 0422a957c1b8959c5b48d7188a524296dc9b778b
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Feb 5 16:25:43 2021 +0100
test_echo_tcp_sendmsg_recvmsg_fd: add test_tcp_sendmsg_recvmsg_fd_mixed()
tests
Here we mix sockets and other valid file descriptors.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit b35e3ce2e2b78fb7a1ccf6d5e011239c14fe27a9
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Feb 5 14:40:45 2021 +0100
test_echo_tcp_sendmsg_recvmsg_fd: add
test_tcp_sendmsg_recvmsg_fd_different() tests
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit 2d93f75669a432723c90e20c3f9b2e2abfa55545
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Feb 5 14:20:16 2021 +0100
test_echo_tcp_sendmsg_recvmsg_fd: also test passing the same socket up to 6
times
Note SWRAP_MAX_PASSED_FDS is currently 6.
This test demonstrates that even 64-bit systems required commit:
"swrap: fix fd-passing without 4 padding bytes"
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit 5aa939f2eb3f3ab04b65fde031a0c1f151928121
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Feb 5 14:10:03 2021 +0100
test_echo_tcp_sendmsg_recvmsg_fd: split out
test_tcp_sendmsg_recvmsg_fd_same()
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit cbd5910b1814cad1f1c4e9c9e8515f217edcd4d2
Author: Stefan Metzmacher <me...@samba.org>
Date: Thu Feb 4 17:04:30 2021 +0100
test_echo_tcp_sendmsg_recvmsg_fd: split out
test_tcp_sendmsg_recvmsg_fd_array()
This will allow us to test more combinations in order to
get better coverage. For now we just test a single fd.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit b2e366d87bf8b340fe5e0b0186369cda7a94d186
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Feb 5 15:41:27 2021 +0100
tests/echo_srv: allow more than once tcp connection at a time
We should not wait for the last connection to disconnect,
there would not be any reason to use fork at all.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit e896cecf1fc08d091d09fa29f1cfbfc22dab89ff
Author: Stefan Metzmacher <me...@samba.org>
Date: Thu Feb 4 16:20:13 2021 +0100
swrap: fix fd-passing without 4 padding bytes
We noticed the problem on 32 bit platforms and sending a single
application fd, the hidden pipe-fd doesn't fit into the padding
bytes. This can also happen on 64 bit platforms and an even number
of application fds.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit 3c7ef4751527bd8c93d5431d9f1e36c4fe648f3d
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Feb 5 19:36:26 2021 +0100
swrap: fix invalid read in swrap_sendmsg_unix_scm_rights()
Here the fds_out array is larger than the fds_in array, so we can
only copy the fds_in array using size_fds_in, leaving the last slot
of fds_out untouched, which is filled by fds_out[num_fds_in] = pipefd[0]
later.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
src/socket_wrapper.c | 71 ++++-
tests/echo_srv.c | 2 +-
tests/test_echo_tcp_sendmsg_recvmsg_fd.c | 510 +++++++++++++++++++++++++++----
3 files changed, 518 insertions(+), 65 deletions(-)
Changeset truncated at 500 lines:
diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 43a5892..ece3493 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -5450,7 +5450,7 @@ static int swrap_sendmsg_unix_scm_rights(const struct
cmsghdr *cmsg,
*new_cmsg = *cmsg;
__fds_out.p = CMSG_DATA(new_cmsg);
fds_out = __fds_out.fds;
- memcpy(fds_out, fds_in, size_fds_out);
+ memcpy(fds_out, fds_in, size_fds_in);
new_cmsg->cmsg_len = cmsg->cmsg_len;
for (i = 0; i < num_fds_in; i++) {
@@ -5962,8 +5962,48 @@ static ssize_t swrap_sendmsg_after_unix(struct msghdr
*msg_tmp,
static int swrap_recvmsg_before_unix(struct msghdr *msg_in,
struct msghdr *msg_tmp)
{
+#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
+ const size_t cm_extra_space = CMSG_SPACE(sizeof(int));
+ uint8_t *cm_data = NULL;
+ size_t cm_data_space = 0;
+
+ *msg_tmp = *msg_in;
+
+ SWRAP_LOG(SWRAP_LOG_TRACE,
+ "msg_in->msg_controllen=%zu",
+ (size_t)msg_in->msg_controllen);
+
+ /* Nothing to do */
+ if (msg_in->msg_controllen == 0 || msg_in->msg_control == NULL) {
+ return 0;
+ }
+
+ /*
+ * We need to give the kernel a bit more space in order
+ * recv the pipe fd, added by swrap_sendmsg_before_unix()).
+ * swrap_recvmsg_after_unix() will hide it again.
+ */
+ cm_data_space = msg_in->msg_controllen;
+ if (cm_data_space < (INT32_MAX - cm_extra_space)) {
+ cm_data_space += cm_extra_space;
+ }
+ cm_data = calloc(1, cm_data_space);
+ if (cm_data == NULL) {
+ return -1;
+ }
+ memcpy(cm_data, msg_in->msg_control, msg_in->msg_controllen);
+
+ msg_tmp->msg_controllen = cm_data_space;
+ msg_tmp->msg_control = cm_data;
+
+ SWRAP_LOG(SWRAP_LOG_TRACE,
+ "msg_tmp->msg_controllen=%zu",
+ (size_t)msg_tmp->msg_controllen);
+ return 0;
+#else /* HAVE_STRUCT_MSGHDR_MSG_CONTROL */
*msg_tmp = *msg_in;
return 0;
+#endif /* ! HAVE_STRUCT_MSGHDR_MSG_CONTROL */
}
static ssize_t swrap_recvmsg_after_unix(struct msghdr *msg_tmp,
@@ -5976,9 +6016,14 @@ static ssize_t swrap_recvmsg_after_unix(struct msghdr
*msg_tmp,
size_t cm_data_space = 0;
int rc = -1;
+ SWRAP_LOG(SWRAP_LOG_TRACE,
+ "msg_tmp->msg_controllen=%zu",
+ (size_t)msg_tmp->msg_controllen);
+
/* Nothing to do */
if (msg_tmp->msg_controllen == 0 || msg_tmp->msg_control == NULL) {
- goto done;
+ *msg_out = *msg_tmp;
+ return ret;
}
for (cmsg = CMSG_FIRSTHDR(msg_tmp);
@@ -6006,15 +6051,27 @@ static ssize_t swrap_recvmsg_after_unix(struct msghdr
*msg_tmp,
}
/*
- * msg_tmp->msg_control is still the buffer of the caller.
+ * msg_tmp->msg_control was created by swrap_recvmsg_before_unix()
+ * and msg_out->msg_control is still the buffer of the caller.
*/
- memcpy(msg_tmp->msg_control, cm_data, cm_data_space);
- msg_tmp->msg_controllen = cm_data_space;
+ SAFE_FREE(msg_tmp->msg_control);
+ msg_tmp->msg_control = msg_out->msg_control;
+ msg_tmp->msg_controllen = msg_out->msg_controllen;
+ *msg_out = *msg_tmp;
+
+ cm_data_space = MIN(cm_data_space, msg_out->msg_controllen);
+ memcpy(msg_out->msg_control, cm_data, cm_data_space);
+ msg_out->msg_controllen = cm_data_space;
SAFE_FREE(cm_data);
-done:
-#endif /* ! HAVE_STRUCT_MSGHDR_MSG_CONTROL */
+
+ SWRAP_LOG(SWRAP_LOG_TRACE,
+ "msg_out->msg_controllen=%zu",
+ (size_t)msg_out->msg_controllen);
+ return ret;
+#else /* HAVE_STRUCT_MSGHDR_MSG_CONTROL */
*msg_out = *msg_tmp;
return ret;
+#endif /* ! HAVE_STRUCT_MSGHDR_MSG_CONTROL */
}
static ssize_t swrap_sendmsg_before(int fd,
diff --git a/tests/echo_srv.c b/tests/echo_srv.c
index 0aefa9a..fbb6637 100644
--- a/tests/echo_srv.c
+++ b/tests/echo_srv.c
@@ -537,6 +537,7 @@ static void echo_tcp(int sock)
pid_t pid;
while (1) {
+ waitpid(-1, NULL, WNOHANG);
s = accept(sock, &addr.sa.s, &addr.sa_socklen);
if (s == -1 && errno == ECONNABORTED) {
continue;
@@ -575,7 +576,6 @@ static void echo_tcp(int sock)
close(s);
exit(0);
}
- waitpid(-1, NULL, 0);
close(s);
}
diff --git a/tests/test_echo_tcp_sendmsg_recvmsg_fd.c
b/tests/test_echo_tcp_sendmsg_recvmsg_fd.c
index 8ae1a60..215d5ff 100644
--- a/tests/test_echo_tcp_sendmsg_recvmsg_fd.c
+++ b/tests/test_echo_tcp_sendmsg_recvmsg_fd.c
@@ -33,33 +33,86 @@ static int teardown(void **state)
return 0;
}
-static void test_tcp_sendmsg_recvmsg_fd(void **state)
+struct test_fd {
+ int fd;
+ struct torture_address sock_addr;
+ struct torture_address peer_addr;
+};
+
+static int test_fill_test_fd(struct test_fd *tfd, int fd)
{
- struct torture_address addr = {
- .sa_socklen = sizeof(struct sockaddr_in),
+ struct torture_address saddr = {
+ .sa_socklen = sizeof(struct sockaddr_storage),
};
- int pass_sock_fd;
- int sv[2];
- int child_fd, parent_fd;
- pid_t pid;
- int rc;
+ struct torture_address paddr = {
+ .sa_socklen = sizeof(struct sockaddr_storage),
+ };
+ int ret;
- (void) state; /* unused */
+ *tfd = (struct test_fd) { .fd = fd, };
- /* create socket file descriptor to be passed */
- pass_sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
- assert_int_not_equal(pass_sock_fd, -1);
+ ret = getsockname(fd, &saddr.sa.s, &saddr.sa_socklen);
+ if (ret == -1 && errno == ENOTSOCK) {
+ return 0;
+ }
+ if (ret == -1) {
+ return ret;
+ }
- addr.sa.in.sin_family = AF_INET;
- addr.sa.in.sin_port = htons(torture_server_port());
+ ret = getpeername(fd, &paddr.sa.s, &paddr.sa_socklen);
+ if (ret == -1) {
+ return ret;
+ }
- rc = inet_pton(addr.sa.in.sin_family,
- torture_server_address(AF_INET),
- &addr.sa.in.sin_addr);
- assert_int_equal(rc, 1);
+ tfd->sock_addr = saddr;
+ tfd->peer_addr = paddr;
+ return 0;
+}
- rc = connect(pass_sock_fd, &addr.sa.s, addr.sa_socklen);
- assert_int_equal(rc, 0);
+static void _assert_torture_address_equal(const struct torture_address *ga,
+ const struct torture_address *ea,
+ const char * const file,
+ const int line)
+{
+ _assert_int_equal(ga->sa_socklen, ea->sa_socklen, file, line);
+ if (ga->sa_socklen == 0) {
+ return;
+ }
+ _assert_memory_equal(&ga->sa, &ea->sa, ga->sa_socklen, file, line);
+}
+
+#define assert_test_fd_equal(gfd, efd) \
+ _assert_test_fd_equal(gfd, efd, __FILE__, __LINE__)
+
+static void _assert_test_fd_equal(const struct test_fd *gfd,
+ const struct test_fd *efd,
+ const char * const file,
+ const int line)
+{
+ if (efd->fd == -1) {
+ _assert_int_equal(gfd->fd, -1, file, line);
+ return;
+ }
+
+ _assert_int_not_equal(gfd->fd, -1, file, line);
+
+ _assert_torture_address_equal(&gfd->sock_addr, &efd->sock_addr, file,
line);
+ _assert_torture_address_equal(&gfd->peer_addr, &efd->peer_addr, file,
line);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_array(const int *fds, size_t num_fds)
+{
+ struct test_fd tfds[num_fds];
+ size_t idx;
+ int sv[2];
+ int child_fd, parent_fd;
+ pid_t pid;
+ int rc;
+
+ for (idx = 0; idx < num_fds; idx++) {
+ rc = test_fill_test_fd(&tfds[idx], fds[idx]);
+ assert_int_equal(rc, 0);
+ }
/* create unix domain socket stream */
rc = socketpair(AF_LOCAL, SOCK_STREAM, 0, sv);
@@ -73,17 +126,11 @@ static void test_tcp_sendmsg_recvmsg_fd(void **state)
if (pid == 0) {
/* Child */
- struct torture_address peer_addr = {
- .sa_socklen = sizeof(struct sockaddr_in),
- };
struct msghdr child_msg;
- char cmsgbuf[CMSG_SPACE(sizeof(int))];
+ int recv_fd_array[num_fds];
+ char cmsgbuf[CMSG_SPACE(sizeof(int)*num_fds)];
struct cmsghdr *cmsg;
- int rcv_sock_fd, port;
ssize_t ret;
- char send_buf[64] = {0};
- char recv_buf[64] = {0};
- char ipstr[INET_ADDRSTRLEN];
char byte = { 0, };
struct iovec iov;
@@ -104,45 +151,70 @@ static void test_tcp_sendmsg_recvmsg_fd(void **state)
assert_non_null(cmsg);
assert_int_equal(cmsg->cmsg_type, SCM_RIGHTS);
- memcpy(&rcv_sock_fd, CMSG_DATA(cmsg), sizeof(rcv_sock_fd));
- assert_int_not_equal(rcv_sock_fd, -1);
-
- /* extract peer info from received socket fd */
- ret = getpeername(rcv_sock_fd, &peer_addr.sa.s,
&peer_addr.sa_socklen);
- assert_int_not_equal(ret, -1);
-
- port = ntohs(peer_addr.sa.in.sin_port);
- inet_ntop(AF_INET, &peer_addr.sa.in.sin_addr, ipstr,
sizeof(ipstr));
-
- /* check whether it is the same socket previously connected */
- assert_string_equal(ipstr, torture_server_address(AF_INET));
- assert_int_equal(port, torture_server_port());
+ memcpy(recv_fd_array, CMSG_DATA(cmsg), sizeof(int)*num_fds);
+ for (idx = 0; idx < num_fds; idx++) {
+ assert_int_not_equal(recv_fd_array[idx], -1);
+ }
- snprintf(send_buf, sizeof(send_buf), "packet");
+ for (idx = 0; idx < num_fds; idx++) {
+ struct test_fd recv_tfd = { .fd = -1, };
- ret = write(rcv_sock_fd,
- send_buf,
- sizeof(send_buf));
- assert_int_not_equal(ret, -1);
+ ret = test_fill_test_fd(&recv_tfd, recv_fd_array[idx]);
+ assert_int_equal(ret, 0);
- ret = read(rcv_sock_fd,
- recv_buf,
- sizeof(recv_buf));
- assert_int_not_equal(ret, -1);
+ assert_test_fd_equal(&recv_tfd, &tfds[idx]);
+ }
- assert_memory_equal(send_buf, recv_buf, sizeof(send_buf));
+ for (idx = 0; idx < num_fds; idx++) {
+ int recv_fd = recv_fd_array[idx];
+ char send_buf[64] = {0,};
+ char recv_buf[64] = {0,};
+
+ if (tfds[idx].sock_addr.sa_socklen == 0) {
+ /*
+ * skip fds not belonging to
+ * a socket.
+ */
+ continue;
+ }
+
+ if (tfds[idx].sock_addr.sa.s.sa_family == AF_UNIX) {
+ /*
+ * skip fds not belonging to
+ * a socket.
+ */
+ continue;
+ }
+
+ snprintf(send_buf, sizeof(send_buf), "packet");
+
+ ret = write(recv_fd,
+ send_buf,
+ sizeof(send_buf));
+ assert_int_not_equal(ret, -1);
+
+ ret = read(recv_fd,
+ recv_buf,
+ sizeof(recv_buf));
+ assert_int_not_equal(ret, -1);
+
+ assert_memory_equal(send_buf, recv_buf,
sizeof(send_buf));
+ }
exit(0);
} else {
/* Parent */
struct msghdr parent_msg;
struct cmsghdr *cmsg;
- char cmsgbuf[CMSG_SPACE(sizeof(pass_sock_fd))];
+ char cmsgbuf[CMSG_SPACE(sizeof(int)*num_fds)];
+ int pass_fd_array[num_fds];
char byte = '!';
struct iovec iov;
int cs;
- (void) state; /* unused */
+ for (idx = 0; idx < num_fds; idx++) {
+ pass_fd_array[idx] = tfds[idx].fd;
+ }
iov.iov_base = &byte;
iov.iov_len = 1;
@@ -156,10 +228,10 @@ static void test_tcp_sendmsg_recvmsg_fd(void **state)
cmsg = CMSG_FIRSTHDR(&parent_msg);
cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_RIGHTS;
- cmsg->cmsg_len = CMSG_LEN(sizeof(pass_sock_fd));
+ cmsg->cmsg_len = CMSG_LEN(sizeof(int)*num_fds);
/* place previously connected socket fd as ancillary data */
- memcpy(CMSG_DATA(cmsg), &pass_sock_fd, sizeof(pass_sock_fd));
+ memcpy(CMSG_DATA(cmsg), pass_fd_array, sizeof(int)*num_fds);
parent_msg.msg_controllen = cmsg->cmsg_len;
rc = sendmsg(parent_fd, &parent_msg, 0);
@@ -173,13 +245,337 @@ static void test_tcp_sendmsg_recvmsg_fd(void **state)
}
}
+static void test_tcp_sendmsg_recvmsg_fd_same(size_t num_fds)
+{
+ struct torture_address addr = {
+ .sa_socklen = sizeof(struct sockaddr_in),
+ };
+ int pass_sock_fd;
+ int fd_array[num_fds];
+ size_t idx;
+ int rc;
+
+ /* create socket file descriptor to be passed */
+ pass_sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+ assert_int_not_equal(pass_sock_fd, -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(pass_sock_fd, &addr.sa.s, addr.sa_socklen);
+ assert_int_equal(rc, 0);
+
+ for (idx = 0; idx < num_fds; idx++) {
+ fd_array[idx] = pass_sock_fd;
+ }
+
+ test_tcp_sendmsg_recvmsg_fd_array(fd_array, num_fds);
+
+ close(pass_sock_fd);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_1(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_same(1);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_2s(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_same(2);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_3s(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_same(3);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_4s(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_same(4);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_5s(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_same(5);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_6s(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_same(6);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_different(size_t num_fds)
+{
+ int fd_array[num_fds];
+ size_t idx;
+
+ for (idx = 0; idx < num_fds; idx++) {
+ struct torture_address addr = {
+ .sa_socklen = sizeof(struct sockaddr_in),
+ };
+ int pass_sock_fd;
+ int rc;
+
+ /* create socket file descriptor to be passed */
+ pass_sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+ assert_int_not_equal(pass_sock_fd, -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(pass_sock_fd, &addr.sa.s, addr.sa_socklen);
+ assert_int_equal(rc, 0);
+
+ fd_array[idx] = pass_sock_fd;
+ }
+
+ test_tcp_sendmsg_recvmsg_fd_array(fd_array, num_fds);
+
+ for (idx = 0; idx < num_fds; idx++) {
+ close(fd_array[idx]);
+ }
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_2d(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_different(2);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_3d(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_different(3);
+}
+
+static void test_tcp_sendmsg_recvmsg_fd_4d(void **state)
+{
+ (void) state; /* unused */
+ test_tcp_sendmsg_recvmsg_fd_different(4);
+}
+
--
Socket Wrapper Repository
