From e4d2edc9d0c58de421eb349871e90b67edec0b9c Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 22 Dec 2015 15:43:42 +0000 Subject: io: bind to loopback IP addrs in test suite The test suite currently binds to 0.0.0.0 or ::, which covers all interfaces of the machine. It is bad practice for test suite to open publically accessible ports on a machine, so switch to use loopback addrs 127.0.0.1 or ::1. Reported-by: Peter Maydell Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- tests/test-io-channel-socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c index 194d043..e76d54c 100644 --- a/tests/test-io-channel-socket.c +++ b/tests/test-io-channel-socket.c @@ -261,7 +261,7 @@ static void test_io_channel_ipv4(bool async) listen_addr->type = SOCKET_ADDRESS_KIND_INET; listen_addr->u.inet = g_new0(InetSocketAddress, 1); - listen_addr->u.inet->host = g_strdup("0.0.0.0"); + listen_addr->u.inet->host = g_strdup("127.0.0.1"); listen_addr->u.inet->port = NULL; /* Auto-select */ connect_addr->type = SOCKET_ADDRESS_KIND_INET; @@ -295,7 +295,7 @@ static void test_io_channel_ipv6(bool async) listen_addr->type = SOCKET_ADDRESS_KIND_INET; listen_addr->u.inet = g_new0(InetSocketAddress, 1); - listen_addr->u.inet->host = g_strdup("::"); + listen_addr->u.inet->host = g_strdup("::1"); listen_addr->u.inet->port = NULL; /* Auto-select */ connect_addr->type = SOCKET_ADDRESS_KIND_INET; -- cgit v1.1 From bead59946a8b54398f4ba3c9c8ecd15eeac78c53 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 21 Dec 2015 12:04:21 +0000 Subject: io: fix setting of QIO_CHANNEL_FEATURE_FD_PASS on server connections The QIO_CHANNEL_FEATURE_FD_PASS feature flag is set in the qio_channel_socket_set_fd() method, however, this only deals with client side connections. To ensure server side connections also have the feature flag set, we must set it in qio_channel_socket_accept() too. This also highlighted a typo fix where the code updated the sockaddr struct in the wrong object instance. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- io/channel-socket.c | 10 ++++++++-- tests/test-io-channel-socket.c | 29 +++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/io/channel-socket.c b/io/channel-socket.c index 90b3c73..eed2ff5 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -352,13 +352,19 @@ qio_channel_socket_accept(QIOChannelSocket *ioc, goto error; } - if (getsockname(cioc->fd, (struct sockaddr *)&ioc->localAddr, - &ioc->localAddrLen) < 0) { + if (getsockname(cioc->fd, (struct sockaddr *)&cioc->localAddr, + &cioc->localAddrLen) < 0) { error_setg_errno(errp, socket_error(), "Unable to query local socket address"); goto error; } +#ifndef WIN32 + if (cioc->localAddr.ss_family == AF_UNIX) { + QIO_CHANNEL(cioc)->features |= (1 << QIO_CHANNEL_FEATURE_FD_PASS); + } +#endif /* WIN32 */ + trace_qio_channel_socket_accept_complete(ioc, cioc, cioc->fd); return cioc; diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c index e76d54c..b0eb538 100644 --- a/tests/test-io-channel-socket.c +++ b/tests/test-io-channel-socket.c @@ -210,13 +210,19 @@ static void test_io_channel_setup_async(SocketAddress *listen_addr, static void test_io_channel(bool async, SocketAddress *listen_addr, - SocketAddress *connect_addr) + SocketAddress *connect_addr, + bool passFD) { QIOChannel *src, *dst; QIOChannelTest *test; if (async) { test_io_channel_setup_async(listen_addr, connect_addr, &src, &dst); + g_assert(!passFD || + qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS)); + g_assert(!passFD || + qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS)); + test = qio_channel_test_new(); qio_channel_test_run_threads(test, true, src, dst); qio_channel_test_validate(test); @@ -226,6 +232,11 @@ static void test_io_channel(bool async, test_io_channel_setup_async(listen_addr, connect_addr, &src, &dst); + g_assert(!passFD || + qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS)); + g_assert(!passFD || + qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS)); + test = qio_channel_test_new(); qio_channel_test_run_threads(test, false, src, dst); qio_channel_test_validate(test); @@ -235,6 +246,11 @@ static void test_io_channel(bool async, } else { test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst); + g_assert(!passFD || + qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS)); + g_assert(!passFD || + qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS)); + test = qio_channel_test_new(); qio_channel_test_run_threads(test, true, src, dst); qio_channel_test_validate(test); @@ -244,6 +260,11 @@ static void test_io_channel(bool async, test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst); + g_assert(!passFD || + qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS)); + g_assert(!passFD || + qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS)); + test = qio_channel_test_new(); qio_channel_test_run_threads(test, false, src, dst); qio_channel_test_validate(test); @@ -269,7 +290,7 @@ static void test_io_channel_ipv4(bool async) connect_addr->u.inet->host = g_strdup("127.0.0.1"); connect_addr->u.inet->port = NULL; /* Filled in later */ - test_io_channel(async, listen_addr, connect_addr); + test_io_channel(async, listen_addr, connect_addr, false); qapi_free_SocketAddress(listen_addr); qapi_free_SocketAddress(connect_addr); @@ -303,7 +324,7 @@ static void test_io_channel_ipv6(bool async) connect_addr->u.inet->host = g_strdup("::1"); connect_addr->u.inet->port = NULL; /* Filled in later */ - test_io_channel(async, listen_addr, connect_addr); + test_io_channel(async, listen_addr, connect_addr, false); qapi_free_SocketAddress(listen_addr); qapi_free_SocketAddress(connect_addr); @@ -337,7 +358,7 @@ static void test_io_channel_unix(bool async) connect_addr->u.q_unix = g_new0(UnixSocketAddress, 1); connect_addr->u.q_unix->path = g_strdup(TEST_SOCKET); - test_io_channel(async, listen_addr, connect_addr); + test_io_channel(async, listen_addr, connect_addr, true); qapi_free_SocketAddress(listen_addr); qapi_free_SocketAddress(connect_addr); -- cgit v1.1 From 7b3c618ad0cd0154993b5b5dbd34e0010960585a Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 21 Dec 2015 11:58:51 +0000 Subject: io: fix stack allocation when sending of file descriptors When sending file descriptors over a socket, we have to allocate a data buffer to hold the FDs in the scmsghdr. Unfortunately we allocated the buffer on the stack inside an if () {} block, but called sendmsg() outside the block. So the stack bytes holding the FDs were liable to be overwritten with other data. By luck this was not a problem when sending 1 FD, but if sending 2 or more then it would fail. The fix is to simply move the variables outside the nested 'if' block. To keep valgrind quiet we also zero-initialize the 'control' buffer. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- io/channel-socket.c | 7 ++- tests/test-io-channel-socket.c | 96 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/io/channel-socket.c b/io/channel-socket.c index eed2ff5..10a5b31 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -493,15 +493,14 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); ssize_t ret; struct msghdr msg = { NULL, }; + char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)] = { 0 }; + size_t fdsize = sizeof(int) * nfds; + struct cmsghdr *cmsg; msg.msg_iov = (struct iovec *)iov; msg.msg_iovlen = niov; if (nfds) { - char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)]; - size_t fdsize = sizeof(int) * nfds; - struct cmsghdr *cmsg; - if (nfds > SOCKET_MAX_FDS) { error_setg_errno(errp, -EINVAL, "Only %d FDs can be sent, got %zu", diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c index b0eb538..1a36a3c 100644 --- a/tests/test-io-channel-socket.c +++ b/tests/test-io-channel-socket.c @@ -376,6 +376,100 @@ static void test_io_channel_unix_async(void) { return test_io_channel_unix(true); } + +static void test_io_channel_unix_fd_pass(void) +{ + SocketAddress *listen_addr = g_new0(SocketAddress, 1); + SocketAddress *connect_addr = g_new0(SocketAddress, 1); + QIOChannel *src, *dst; + int testfd; + int fdsend[3]; + int *fdrecv = NULL; + size_t nfdrecv = 0; + size_t i; + char bufsend[12], bufrecv[12]; + struct iovec iosend[1], iorecv[1]; + +#define TEST_SOCKET "test-io-channel-socket.sock" +#define TEST_FILE "test-io-channel-socket.txt" + + testfd = open(TEST_FILE, O_RDWR|O_TRUNC|O_CREAT, 0700); + g_assert(testfd != -1); + fdsend[0] = testfd; + fdsend[1] = testfd; + fdsend[2] = testfd; + + listen_addr->type = SOCKET_ADDRESS_KIND_UNIX; + listen_addr->u.q_unix = g_new0(UnixSocketAddress, 1); + listen_addr->u.q_unix->path = g_strdup(TEST_SOCKET); + + connect_addr->type = SOCKET_ADDRESS_KIND_UNIX; + connect_addr->u.q_unix = g_new0(UnixSocketAddress, 1); + connect_addr->u.q_unix->path = g_strdup(TEST_SOCKET); + + test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst); + + memcpy(bufsend, "Hello World", G_N_ELEMENTS(bufsend)); + + iosend[0].iov_base = bufsend; + iosend[0].iov_len = G_N_ELEMENTS(bufsend); + + iorecv[0].iov_base = bufrecv; + iorecv[0].iov_len = G_N_ELEMENTS(bufrecv); + + g_assert(qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS)); + g_assert(qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS)); + + qio_channel_writev_full(src, + iosend, + G_N_ELEMENTS(iosend), + fdsend, + G_N_ELEMENTS(fdsend), + &error_abort); + + qio_channel_readv_full(dst, + iorecv, + G_N_ELEMENTS(iorecv), + &fdrecv, + &nfdrecv, + &error_abort); + + g_assert(nfdrecv == G_N_ELEMENTS(fdsend)); + /* Each recvd FD should be different from sent FD */ + for (i = 0; i < nfdrecv; i++) { + g_assert_cmpint(fdrecv[i], !=, testfd); + } + /* Each recvd FD should be different from each other */ + g_assert_cmpint(fdrecv[0], !=, fdrecv[1]); + g_assert_cmpint(fdrecv[0], !=, fdrecv[2]); + g_assert_cmpint(fdrecv[1], !=, fdrecv[2]); + + /* Check the I/O buf we sent at the same time matches */ + g_assert(memcmp(bufsend, bufrecv, G_N_ELEMENTS(bufsend)) == 0); + + /* Write some data into the FD we received */ + g_assert(write(fdrecv[0], bufsend, G_N_ELEMENTS(bufsend)) == + G_N_ELEMENTS(bufsend)); + + /* Read data from the original FD and make sure it matches */ + memset(bufrecv, 0, G_N_ELEMENTS(bufrecv)); + g_assert(lseek(testfd, 0, SEEK_SET) == 0); + g_assert(read(testfd, bufrecv, G_N_ELEMENTS(bufrecv)) == + G_N_ELEMENTS(bufrecv)); + g_assert(memcmp(bufsend, bufrecv, G_N_ELEMENTS(bufsend)) == 0); + + object_unref(OBJECT(src)); + object_unref(OBJECT(dst)); + qapi_free_SocketAddress(listen_addr); + qapi_free_SocketAddress(connect_addr); + unlink(TEST_SOCKET); + unlink(TEST_FILE); + close(testfd); + for (i = 0; i < nfdrecv; i++) { + close(fdrecv[i]); + } + g_free(fdrecv); +} #endif /* _WIN32 */ @@ -414,6 +508,8 @@ int main(int argc, char **argv) test_io_channel_unix_sync); g_test_add_func("/io/channel/socket/unix-async", test_io_channel_unix_async); + g_test_add_func("/io/channel/socket/unix-fd-pass", + test_io_channel_unix_fd_pass); #endif /* _WIN32 */ return g_test_run(); -- cgit v1.1