[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 3/8] char: fix use-after-free with dup chardev & reconnect
From: |
Marc-André Lureau |
Subject: |
[PULL 3/8] char: fix use-after-free with dup chardev & reconnect |
Date: |
Mon, 13 Jul 2020 12:24:19 +0400 |
With a reconnect socket, qemu_char_open() will start a background
thread. It should keep a reference on the chardev.
Fixes invalid read:
READ of size 8 at 0x6040000ac858 thread T7
#0 0x5555598d37b8 in unix_connect_saddr
/home/elmarco/src/qq/util/qemu-sockets.c:954
#1 0x5555598d4751 in socket_connect
/home/elmarco/src/qq/util/qemu-sockets.c:1109
#2 0x555559707c34 in qio_channel_socket_connect_sync
/home/elmarco/src/qq/io/channel-socket.c:145
#3 0x5555596adebb in tcp_chr_connect_client_task
/home/elmarco/src/qq/chardev/char-socket.c:1104
#4 0x555559723d55 in qio_task_thread_worker
/home/elmarco/src/qq/io/task.c:123
#5 0x5555598a6731 in qemu_thread_start
/home/elmarco/src/qq/util/qemu-thread-posix.c:519
#6 0x7ffff40d4431 in start_thread (/lib64/libpthread.so.0+0x9431)
#7 0x7ffff40029d2 in __clone (/lib64/libc.so.6+0x1019d2)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200420112012.567284-1-marcandre.lureau@redhat.com>
---
chardev/char-socket.c | 3 ++-
tests/test-char.c | 54 +++++++++++++++++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 320aa7c642f..ef62dbf3d73 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1129,7 +1129,8 @@ static void tcp_chr_connect_client_async(Chardev *chr)
*/
s->connect_task = qio_task_new(OBJECT(sioc),
qemu_chr_socket_connected,
- chr, NULL);
+ object_ref(OBJECT(chr)),
+ (GDestroyNotify)object_unref);
qio_task_run_in_thread(s->connect_task,
tcp_chr_connect_client_task,
s->addr,
diff --git a/tests/test-char.c b/tests/test-char.c
index 73ba1cf6010..9d8746414d7 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -904,6 +904,52 @@ typedef struct {
char_socket_cb event_cb;
} CharSocketClientTestConfig;
+static void char_socket_client_dupid_test(gconstpointer opaque)
+{
+ const CharSocketClientTestConfig *config = opaque;
+ QIOChannelSocket *ioc;
+ char *optstr;
+ Chardev *chr1, *chr2;
+ SocketAddress *addr;
+ QemuOpts *opts;
+ Error *local_err = NULL;
+
+ /*
+ * Setup a listener socket and determine get its address
+ * so we know the TCP port for the client later
+ */
+ ioc = qio_channel_socket_new();
+ g_assert_nonnull(ioc);
+ qio_channel_socket_listen_sync(ioc, config->addr, 1, &error_abort);
+ addr = qio_channel_socket_get_local_address(ioc, &error_abort);
+ g_assert_nonnull(addr);
+
+ /*
+ * Populate the chardev address based on what the server
+ * is actually listening on
+ */
+ optstr = char_socket_addr_to_opt_str(addr,
+ config->fd_pass,
+ config->reconnect,
+ false);
+
+ opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
+ optstr, true);
+ g_assert_nonnull(opts);
+ chr1 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+ g_assert_nonnull(chr1);
+
+ chr2 = qemu_chr_new_from_opts(opts, NULL, &local_err);
+ g_assert_null(chr2);
+ error_free_or_abort(&local_err);
+
+ object_unref(OBJECT(ioc));
+ qemu_opts_del(opts);
+ object_unparent(OBJECT(chr1));
+ qapi_free_SocketAddress(addr);
+ g_free(optstr);
+}
+
static void char_socket_client_test(gconstpointer opaque)
{
const CharSocketClientTestConfig *config = opaque;
@@ -1456,7 +1502,7 @@ int main(int argc, char **argv)
#define SOCKET_CLIENT_TEST(name, addr) \
static CharSocketClientTestConfig client1 ## name = \
- { addr, NULL, false, false, char_socket_event}; \
+ { addr, NULL, false, false, char_socket_event }; \
static CharSocketClientTestConfig client2 ## name = \
{ addr, NULL, true, false, char_socket_event }; \
static CharSocketClientTestConfig client3 ## name = \
@@ -1470,6 +1516,8 @@ int main(int argc, char **argv)
static CharSocketClientTestConfig client7 ## name = \
{ addr, ",reconnect=1", true, false, \
char_socket_event_with_error }; \
+ static CharSocketClientTestConfig client8 ## name = \
+ { addr, ",reconnect=1", false, false, char_socket_event }; \
g_test_add_data_func("/char/socket/client/mainloop/" # name, \
&client1 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/wait-conn/" # name, \
@@ -1483,7 +1531,9 @@ int main(int argc, char **argv)
g_test_add_data_func("/char/socket/client/wait-conn-fdpass/" # name, \
&client6 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/reconnect-error/" # name, \
- &client7 ##name, char_socket_client_test)
+ &client7 ##name, char_socket_client_test); \
+ g_test_add_data_func("/char/socket/client/dupid-reconnect/" # name, \
+ &client8 ##name, char_socket_client_dupid_test)
if (has_ipv4) {
SOCKET_SERVER_TEST(tcp, &tcpaddr);
--
2.27.0.221.ga08a83db2b
- [PULL 0/8] Chardev patches, Marc-André Lureau, 2020/07/13
- [PULL 1/8] char-socket: initialize reconnect timer only when the timer doesn't start, Marc-André Lureau, 2020/07/13
- [PULL 2/8] chardev: don't abort on attempt to add duplicated chardev, Marc-André Lureau, 2020/07/13
- [PULL 3/8] char: fix use-after-free with dup chardev & reconnect,
Marc-André Lureau <=
- [PULL 4/8] monitor/misc: Remove unused "chardev/char-mux.h" include, Marc-André Lureau, 2020/07/13
- [PULL 5/8] tests/test-char: Remove unused "chardev/char-mux.h" include, Marc-André Lureau, 2020/07/13
- [PULL 6/8] chardev: Restrict msmouse / wctablet / testdev to system emulation, Marc-André Lureau, 2020/07/13
- [PULL 7/8] chardev: Reduce "char-mux.h" scope, rename it "chardev-internal.h", Marc-André Lureau, 2020/07/13
- [PULL 8/8] chardev: Extract system emulation specific code, Marc-André Lureau, 2020/07/13
- Re: [PULL 0/8] Chardev patches, Peter Maydell, 2020/07/13