From: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-devel@nongnu.org
Cc: Ding Hui <dinghui@sangfor.com.cn>,
Gerd Hoffmann <kraxel@redhat.com>,
qemu-stable@nongnu.org
Subject: [PULL 1/3] vnc: fix resource leak when websocket channel error
Date: Wed, 4 Nov 2020 16:46:43 +0100 [thread overview]
Message-ID: <20201104154645.15877-2-kraxel@redhat.com> (raw)
In-Reply-To: <20201104154645.15877-1-kraxel@redhat.com>
From: Ding Hui <dinghui@sangfor.com.cn>
When we connect to vnc by websocket channel, and disconnect
(maybe by some network exception) before handshake,
qemu will left CLOSE_WAIT socket and never close it
After 04d2529da2 ("ui: convert VNC server to use QIOChannelSocket")
and dd154c4d9f ("io: fix handling of EOF / error conditions in websock GSource"),
the vnc call qio_channel_add_watch only care about G_IO_IN,
but mising G_IO_HUP and G_IO_ERR.
When the websocket channel get EOF or error, it cannot callback,
because the caller ignore the event, that leads to resource leak
We need handle G_IO_HUP and G_IO_ERR event, then cleanup the channel
Fixes: 04d2529da2 ("ui: convert VNC server to use QIOChannelSocket")
Fixes: dd154c4d9f ("io: fix handling of EOF / error conditions in websock GSource")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ding Hui <dinghui@sangfor.com.cn>
Message-id: 20201029032241.11040-1-dinghui@sangfor.com.cn
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ui/vnc-auth-sasl.c | 3 ++-
ui/vnc-auth-vencrypt.c | 3 ++-
ui/vnc-jobs.c | 3 ++-
ui/vnc-ws.c | 20 ++++++++++++++++----
ui/vnc.c | 24 ++++++++++++++++++------
5 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 0517b2ead9ce..f67111a3662a 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -111,7 +111,8 @@ size_t vnc_client_write_sasl(VncState *vs)
g_source_remove(vs->ioc_tag);
}
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vnc_client_io, vs, NULL);
}
return ret;
diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index f072e16aceb1..d9c212ff3286 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -79,7 +79,8 @@ static void vnc_tls_handshake_done(QIOTask *task,
g_source_remove(vs->ioc_tag);
}
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_OUT,
+ vnc_client_io, vs, NULL);
start_auth_vencrypt_subauth(vs);
}
}
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 929391f85d69..dbbfbefe5619 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -151,7 +151,8 @@ void vnc_jobs_consume_buffer(VncState *vs)
}
if (vs->disconnecting == FALSE) {
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_OUT,
+ vnc_client_io, vs, NULL);
}
}
buffer_move(&vs->output, &vs->jobs_buffer);
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 95c9703c7240..6d79f3e5a5d8 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -41,13 +41,14 @@ static void vncws_tls_handshake_done(QIOTask *task,
g_source_remove(vs->ioc_tag);
}
vs->ioc_tag = qio_channel_add_watch(
- QIO_CHANNEL(vs->ioc), G_IO_IN, vncws_handshake_io, vs, NULL);
+ QIO_CHANNEL(vs->ioc), G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vncws_handshake_io, vs, NULL);
}
}
gboolean vncws_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
- GIOCondition condition G_GNUC_UNUSED,
+ GIOCondition condition,
void *opaque)
{
VncState *vs = opaque;
@@ -59,6 +60,11 @@ gboolean vncws_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
vs->ioc_tag = 0;
}
+ if (condition & (G_IO_HUP | G_IO_ERR)) {
+ vnc_client_error(vs);
+ return TRUE;
+ }
+
tls = qio_channel_tls_new_server(
vs->ioc,
vs->vd->tlscreds,
@@ -105,13 +111,14 @@ static void vncws_handshake_done(QIOTask *task,
g_source_remove(vs->ioc_tag);
}
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vnc_client_io, vs, NULL);
}
}
gboolean vncws_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
- GIOCondition condition G_GNUC_UNUSED,
+ GIOCondition condition,
void *opaque)
{
VncState *vs = opaque;
@@ -122,6 +129,11 @@ gboolean vncws_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
vs->ioc_tag = 0;
}
+ if (condition & (G_IO_HUP | G_IO_ERR)) {
+ vnc_client_error(vs);
+ return TRUE;
+ }
+
wioc = qio_channel_websock_new_server(vs->ioc);
qio_channel_set_name(QIO_CHANNEL(wioc), "vnc-ws-server-websock");
diff --git a/ui/vnc.c b/ui/vnc.c
index f006aa1afdb2..49235056f7a8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1398,7 +1398,8 @@ static size_t vnc_client_write_plain(VncState *vs)
g_source_remove(vs->ioc_tag);
}
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vnc_client_io, vs, NULL);
}
return ret;
@@ -1435,7 +1436,8 @@ static void vnc_client_write(VncState *vs)
g_source_remove(vs->ioc_tag);
}
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vnc_client_io, vs, NULL);
}
vnc_unlock_output(vs);
}
@@ -1551,6 +1553,12 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED,
VncState *vs = opaque;
assert(vs->magic == VNC_MAGIC);
+
+ if (condition & (G_IO_HUP | G_IO_ERR)) {
+ vnc_disconnect_start(vs);
+ return TRUE;
+ }
+
if (condition & G_IO_IN) {
if (vnc_client_read(vs) < 0) {
/* vs is free()ed here */
@@ -1612,7 +1620,8 @@ void vnc_write(VncState *vs, const void *data, size_t len)
g_source_remove(vs->ioc_tag);
}
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_OUT,
+ vnc_client_io, vs, NULL);
}
buffer_append(&vs->output, data, len);
@@ -3077,14 +3086,17 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
vs->websocket = 1;
if (vd->tlscreds) {
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN, vncws_tls_handshake_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vncws_tls_handshake_io, vs, NULL);
} else {
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN, vncws_handshake_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vncws_handshake_io, vs, NULL);
}
} else {
vs->ioc_tag = qio_channel_add_watch(
- vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
+ vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR,
+ vnc_client_io, vs, NULL);
}
vnc_client_cache_addr(vs);
--
2.27.0
next prev parent reply other threads:[~2020-11-04 15:52 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-04 15:46 [PULL 0/3] Fixes 20201104 patches Gerd Hoffmann
2020-11-04 15:46 ` Gerd Hoffmann [this message]
2020-11-04 15:46 ` [PULL 2/3] ati: check x y display parameter values Gerd Hoffmann
2020-11-04 15:46 ` [PULL 3/3] roms/Makefile: Add qboot to .PHONY list Gerd Hoffmann
2020-11-05 11:10 ` [PULL 0/3] Fixes 20201104 patches Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201104154645.15877-2-kraxel@redhat.com \
--to=kraxel@redhat.com \
--cc=dinghui@sangfor.com.cn \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).