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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.