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 <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This commit is contained in:
parent
bead59946a
commit
7b3c618ad0
@ -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",
|
||||
|
@ -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();
|
||||
|
Loading…
Reference in New Issue
Block a user