[PULL 0/3] Fixes 20201104 patches

[PULL 0/3] Fixes 20201104 patches

[Gerd Hoffmann]

The following changes since commit 3d6e32347a3b57dac7f469a07c5f520e69bd070a:

  Update version for v5.2.0-rc0 release (2020-11-03 21:11:57 +0000)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/fixes-20201104-pull-request

for you to fetch changes up to 577b808b0974fa4af53131cdfece6e9de3c6e4fd:

  roms/Makefile: Add qboot to .PHONY list (2020-11-04 08:25:17 +0100)

----------------------------------------------------------------
misc bugfixes for 5.2

----------------------------------------------------------------

Bruce Rogers (1):
  roms/Makefile: Add qboot to .PHONY list

Ding Hui (1):
  vnc: fix resource leak when websocket channel error

Prasad J Pandit (1):
  ati: check x y display parameter values

 hw/display/ati_2d.c    | 10 ++++++----
 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 ++++++++++++++++++------
 roms/Makefile          |  2 +-
 7 files changed, 47 insertions(+), 18 deletions(-)

-- 
2.27.0

[PULL 1/3] vnc: fix resource leak when websocket channel error

[Gerd Hoffmann]

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

[PULL 2/3] ati: check x y display parameter values

[Gerd Hoffmann]

From: Prasad J Pandit <pjp@fedoraproject.org>

The source and destination x,y display parameters in ati_2d_blt()
may run off the vga limits if either of s->regs.[src|dst]_[xy] is
zero. Check the parameter values to avoid potential crash.

Reported-by: Gaoning Pan <pgn@zju.edu.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-id: 20201021103818.1704030-1-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/ati_2d.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 23a8ae0cd8ce..4dc10ea79529 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -75,8 +75,9 @@ void ati_2d_blt(ATIVGAState *s)
         dst_stride *= bpp;
     }
     uint8_t *end = s->vga.vram_ptr + s->vga.vram_size;
-    if (dst_bits >= end || dst_bits + dst_x + (dst_y + s->regs.dst_height) *
-        dst_stride >= end) {
+    if (dst_x > 0x3fff || dst_y > 0x3fff || dst_bits >= end
+        || dst_bits + dst_x
+         + (dst_y + s->regs.dst_height) * dst_stride >= end) {
         qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
         return;
     }
@@ -107,8 +108,9 @@ void ati_2d_blt(ATIVGAState *s)
             src_bits += s->regs.crtc_offset & 0x07ffffff;
             src_stride *= bpp;
         }
-        if (src_bits >= end || src_bits + src_x +
-            (src_y + s->regs.dst_height) * src_stride >= end) {
+        if (src_x > 0x3fff || src_y > 0x3fff || src_bits >= end
+            || src_bits + src_x
+             + (src_y + s->regs.dst_height) * src_stride >= end) {
             qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
             return;
         }
-- 
2.27.0

[PULL 3/3] roms/Makefile: Add qboot to .PHONY list

[Gerd Hoffmann]

From: Bruce Rogers <brogers@suse.com>

Adding qboot to the .PHONY directive will allow a
make -C roms qboot invocation to work as expected

Signed-off-by: Bruce Rogers <brogers@suse.com>
Message-id: 20201020152512.837769-1-brogers@suse.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 roms/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roms/Makefile b/roms/Makefile
index 1489d47350f2..7045e374d339 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -102,7 +102,7 @@ build-seabios-config-%: config.%
 		OUT=$(CURDIR)/seabios/builds/$*/ all
 
 
-.PHONY: sgabios skiboot
+.PHONY: sgabios skiboot qboot
 sgabios:
 	$(MAKE) -C sgabios
 	cp sgabios/sgabios.bin ../pc-bios
-- 
2.27.0

Re: [PULL 0/3] Fixes 20201104 patches

[Peter Maydell]

On Wed, 4 Nov 2020 at 15:49, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 3d6e32347a3b57dac7f469a07c5f520e69bd070a:
>
>   Update version for v5.2.0-rc0 release (2020-11-03 21:11:57 +0000)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/fixes-20201104-pull-request
>
> for you to fetch changes up to 577b808b0974fa4af53131cdfece6e9de3c6e4fd:
>
>   roms/Makefile: Add qboot to .PHONY list (2020-11-04 08:25:17 +0100)
>
> ----------------------------------------------------------------
> misc bugfixes for 5.2
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM