mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

io: ensure UNIX client doesn't unlink server socket

Browse Source 
The qio_channel_socket_close method for was mistakenly unlinking the
UNIX server socket, even if the channel was a client connection. This
was not noticed with chardevs, since they never call close, but with the
VNC server, this caused the VNC server socket to be deleted after the
first client quit.

The qio_channel_socket_close method also needlessly reimplemented the
logic that already exists in socket_listen_cleanup(). Just call that
method directly, for listen sockets only.

This fixes a regression introduced in QEMU 3.0.0 with

  commit d66f78e1ea
  Author: Pavel Balaev <mail@void.so>
  Date:   Mon May 21 19:17:35 2018 +0300

    Delete AF_UNIX socket after close

Fixes launchpad #1795100

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2019-01-14 11:33:18 +00:00
parent 6d809e7da9
commit 73564c407c
2 changed files with 80 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
io/channel-socket.c
View File

@ -688,10 +688,13 @@ qio_channel_socket_close(QIOChannel *ioc,
int rc = 0;
if (sioc->fd != -1) {
SocketAddress *addr = socket_local_address(sioc->fd, errp);
#ifdef WIN32
WSAEventSelect(sioc->fd, NULL, 0);
#endif
if (qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN)) {
socket_listen_cleanup(sioc->fd, errp);
}
if (closesocket(sioc->fd) < 0) {
sioc->fd = -1;
error_setg_errno(errp, errno,
@ -699,20 +702,6 @@ qio_channel_socket_close(QIOChannel *ioc,
return -1;
}
sioc->fd = -1;
if (addr && addr->type == SOCKET_ADDRESS_TYPE_UNIX
&& addr->u.q_unix.path) {
if (unlink(addr->u.q_unix.path) < 0 && errno != ENOENT) {
error_setg_errno(errp, errno,
"Failed to unlink socket %s",
addr->u.q_unix.path);
rc = -1;
}
}
if (addr) {
qapi_free_SocketAddress(addr);
}
}
return rc;
}

86
tests/test-io-channel-socket.c
View File

@ -49,6 +49,7 @@ static void test_io_channel_set_socket_bufs(QIOChannel *src,
static void test_io_channel_setup_sync(SocketAddress *listen_addr,
SocketAddress *connect_addr,
QIOChannel **srv,
QIOChannel **src,
QIOChannel **dst)
{
@ -78,7 +79,7 @@ static void test_io_channel_setup_sync(SocketAddress *listen_addr,
test_io_channel_set_socket_bufs(*src, *dst);
object_unref(OBJECT(lioc));
*srv = QIO_CHANNEL(lioc);
}
@ -99,6 +100,7 @@ static void test_io_channel_complete(QIOTask *task,
static void test_io_channel_setup_async(SocketAddress *listen_addr,
SocketAddress *connect_addr,
QIOChannel **srv,
QIOChannel **src,
QIOChannel **dst)
{
@ -146,21 +148,34 @@ static void test_io_channel_setup_async(SocketAddress *listen_addr,
qio_channel_set_delay(*src, false);
test_io_channel_set_socket_bufs(*src, *dst);
object_unref(OBJECT(lioc));
*srv = QIO_CHANNEL(lioc);
g_main_loop_unref(data.loop);
}
static void test_io_channel_socket_path_exists(SocketAddress *addr,
bool expectExists)
{
if (addr->type != SOCKET_ADDRESS_TYPE_UNIX) {
return;
}
g_assert(g_file_test(addr->u.q_unix.path,
G_FILE_TEST_EXISTS) == expectExists);
}
static void test_io_channel(bool async,
SocketAddress *listen_addr,
SocketAddress *connect_addr,
bool passFD)
{
QIOChannel *src, *dst;
QIOChannel *src, *dst, *srv;
QIOChannelTest *test;
if (async) {
test_io_channel_setup_async(listen_addr, connect_addr, &src, &dst);
test_io_channel_setup_async(listen_addr, connect_addr,
&srv, &src, &dst);
g_assert(!passFD ||
qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
@ -169,14 +184,25 @@ static void test_io_channel(bool async,
g_assert(qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_SHUTDOWN));
g_assert(qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_SHUTDOWN));
test_io_channel_socket_path_exists(listen_addr, true);
test = qio_channel_test_new();
qio_channel_test_run_threads(test, true, src, dst);
qio_channel_test_validate(test);
test_io_channel_socket_path_exists(listen_addr, true);
/* unref without close, to ensure finalize() cleans up */
object_unref(OBJECT(src));
object_unref(OBJECT(dst));
test_io_channel_socket_path_exists(listen_addr, true);
test_io_channel_setup_async(listen_addr, connect_addr, &src, &dst);
object_unref(OBJECT(srv));
test_io_channel_socket_path_exists(listen_addr, false);
test_io_channel_setup_async(listen_addr, connect_addr,
&srv, &src, &dst);
g_assert(!passFD ||
qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
@ -189,10 +215,24 @@ static void test_io_channel(bool async,
qio_channel_test_run_threads(test, false, src, dst);
qio_channel_test_validate(test);
/* close before unref, to ensure finalize copes with already closed */
qio_channel_close(src, &error_abort);
qio_channel_close(dst, &error_abort);
test_io_channel_socket_path_exists(listen_addr, true);
object_unref(OBJECT(src));
object_unref(OBJECT(dst));
test_io_channel_socket_path_exists(listen_addr, true);
qio_channel_close(srv, &error_abort);
test_io_channel_socket_path_exists(listen_addr, false);
object_unref(OBJECT(srv));
test_io_channel_socket_path_exists(listen_addr, false);
} else {
test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst);
test_io_channel_setup_sync(listen_addr, connect_addr,
&srv, &src, &dst);
g_assert(!passFD ||
qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
@ -201,14 +241,25 @@ static void test_io_channel(bool async,
g_assert(qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_SHUTDOWN));
g_assert(qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_SHUTDOWN));
test_io_channel_socket_path_exists(listen_addr, true);
test = qio_channel_test_new();
qio_channel_test_run_threads(test, true, src, dst);
qio_channel_test_validate(test);
test_io_channel_socket_path_exists(listen_addr, true);
/* unref without close, to ensure finalize() cleans up */
object_unref(OBJECT(src));
object_unref(OBJECT(dst));
test_io_channel_socket_path_exists(listen_addr, true);
test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst);
object_unref(OBJECT(srv));
test_io_channel_socket_path_exists(listen_addr, false);
test_io_channel_setup_sync(listen_addr, connect_addr,
&srv, &src, &dst);
g_assert(!passFD ||
qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
@ -221,8 +272,23 @@ static void test_io_channel(bool async,
qio_channel_test_run_threads(test, false, src, dst);
qio_channel_test_validate(test);
test_io_channel_socket_path_exists(listen_addr, true);
/* close before unref, to ensure finalize copes with already closed */
qio_channel_close(src, &error_abort);
qio_channel_close(dst, &error_abort);
test_io_channel_socket_path_exists(listen_addr, true);
object_unref(OBJECT(src));
object_unref(OBJECT(dst));
test_io_channel_socket_path_exists(listen_addr, true);
qio_channel_close(srv, &error_abort);
test_io_channel_socket_path_exists(listen_addr, false);
object_unref(OBJECT(srv));
test_io_channel_socket_path_exists(listen_addr, false);
}
}
@ -316,7 +382,6 @@ static void test_io_channel_unix(bool async)
qapi_free_SocketAddress(listen_addr);
qapi_free_SocketAddress(connect_addr);
g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS) == FALSE);
}
@ -335,7 +400,7 @@ 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;
QIOChannel *src, *dst, *srv;
int testfd;
int fdsend[3];
int *fdrecv = NULL;
@ -359,7 +424,7 @@ static void test_io_channel_unix_fd_pass(void)
connect_addr->type = SOCKET_ADDRESS_TYPE_UNIX;
connect_addr->u.q_unix.path = g_strdup(TEST_SOCKET);
test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst);
test_io_channel_setup_sync(listen_addr, connect_addr, &srv, &src, &dst);
memcpy(bufsend, "Hello World", G_N_ELEMENTS(bufsend));
@ -412,6 +477,7 @@ static void test_io_channel_unix_fd_pass(void)
object_unref(OBJECT(src));
object_unref(OBJECT(dst));
object_unref(OBJECT(srv));
qapi_free_SocketAddress(listen_addr);
qapi_free_SocketAddress(connect_addr);
unlink(TEST_SOCKET);