[PULL 0/6] Vga 20210901 patches

[PULL 0/6] Vga 20210901 patches

[Gerd Hoffmann]

The following changes since commit ad22d0583300df420819e6c89b1c022b998fac8a:

  Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.2-20210827' into staging (2021-08-27 11:34:12 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/vga-20210901-pull-request

for you to fetch changes up to 01f750f5fef1afd8f6abc0548910f87d473e26d5:

  hw/display/artist: Fix bug in coordinate extraction in artist_vram_read() and artist_vram_write() (2021-08-31 14:38:53 +0200)

----------------------------------------------------------------
vga: misc fixes and cleanups.

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

Dongwon Kim (1):
  virtio-gpu: no point of checking res->iov

Helge Deller (1):
  hw/display/artist: Fix bug in coordinate extraction in
    artist_vram_read() and artist_vram_write()

Jose R. Ziviani (1):
  vga: don't abort when adding a duplicate isa-vga device

Philippe Mathieu-Daudé (2):
  hw/display: Restrict virtio-gpu-udmabuf stubs to !Linux
  ui/console: Restrict udmabuf_fd() to Linux

Qiang Liu (1):
  hw/display/xlnx_dp: fix an out-of-bounds read in xlnx_dp_read

 include/ui/console.h                          |  2 ++
 hw/display/artist.c                           |  8 ++---
 hw/display/vga-isa.c                          | 10 ++++++
 .../display/virtio-gpu-udmabuf-stubs.c        |  0
 hw/display/virtio-gpu.c                       |  2 +-
 hw/display/xlnx_dp.c                          |  6 +++-
 tests/qtest/fuzz-xlnx-dp-test.c               | 33 +++++++++++++++++++
 ui/udmabuf.c                                  | 11 -------
 hw/display/meson.build                        |  3 +-
 stubs/meson.build                             |  1 -
 tests/qtest/meson.build                       |  1 +
 ui/meson.build                                |  6 ++--
 12 files changed, 62 insertions(+), 21 deletions(-)
 rename stubs/virtio-gpu-udmabuf.c => hw/display/virtio-gpu-udmabuf-stubs.c (100%)
 create mode 100644 tests/qtest/fuzz-xlnx-dp-test.c

-- 
2.31.1

[PULL 1/6] virtio-gpu: no point of checking res->iov

[Gerd Hoffmann]

From: Dongwon Kim <dongwon.kim@intel.com>

The code should check the opposite condition of res->iov because it will be null
if virtio_gpu_create_mapping_iov fails and actually this checking is not even
required because checking on ret covers all failing cases.

Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
Message-Id: <20210830175033.29233-1-dongwon.kim@intel.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 990e71fd4062..72da5bf5002c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -362,7 +362,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
     ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
                                         cmd, &res->addrs, &res->iov,
                                         &res->iov_cnt);
-    if (ret != 0 || res->iov) {
+    if (ret != 0) {
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         g_free(res);
         return;
-- 
2.31.1

[PULL 2/6] hw/display: Restrict virtio-gpu-udmabuf stubs to !Linux

[Gerd Hoffmann]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

When using qemu configured with --enabled-modules, the
generic stubs are used instead of the module symbols:

  qemu-system-x86_64: -device virtio-vga,blob=on: cannot enable blob resources without udmabuf

Restrict the stubs to Linux and only link them when
CONFIG_VIRTIO_GPU is disabled (only the modularized
version is available when it is enabled).

Reported-by: Maxim R. <mrom06@ya.ru>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/553
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210823100454.615816-2-philmd@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .../display/virtio-gpu-udmabuf-stubs.c                         | 0
 hw/display/meson.build                                         | 3 ++-
 stubs/meson.build                                              | 1 -
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename stubs/virtio-gpu-udmabuf.c => hw/display/virtio-gpu-udmabuf-stubs.c (100%)

diff --git a/stubs/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf-stubs.c
similarity index 100%
rename from stubs/virtio-gpu-udmabuf.c
rename to hw/display/virtio-gpu-udmabuf-stubs.c
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 1e6b707d3c0a..861c43ff9847 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -56,7 +56,8 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
   virtio_gpu_ss = ss.source_set()
   virtio_gpu_ss.add(when: 'CONFIG_VIRTIO_GPU',
                     if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman])
-  virtio_gpu_ss.add(when: 'CONFIG_LINUX', if_true: files('virtio-gpu-udmabuf.c'))
+  virtio_gpu_ss.add(when: 'CONFIG_LINUX', if_true: files('virtio-gpu-udmabuf.c'),
+                                          if_false: files('virtio-gpu-udmabuf-stubs.c'))
   virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: files('vhost-user-gpu.c'))
   hw_display_modules += {'virtio-gpu': virtio_gpu_ss}
 
diff --git a/stubs/meson.build b/stubs/meson.build
index 717bfa9a9997..275ac89c1600 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -52,7 +52,6 @@ if have_system
   stub_ss.add(files('semihost.c'))
   stub_ss.add(files('usb-dev-stub.c'))
   stub_ss.add(files('xen-hw-stub.c'))
-  stub_ss.add(files('virtio-gpu-udmabuf.c'))
 else
   stub_ss.add(files('qdev.c'))
 endif
-- 
2.31.1

[PULL 3/6] ui/console: Restrict udmabuf_fd() to Linux

[Gerd Hoffmann]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210823100454.615816-3-philmd@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/console.h |  2 ++
 ui/udmabuf.c         | 11 -----------
 ui/meson.build       |  6 ++++--
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index b30b63976a5a..3be21497a2e8 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -471,7 +471,9 @@ bool vnc_display_reload_certs(const char *id,  Error **errp);
 /* input.c */
 int index_from_key(const char *key, size_t key_length);
 
+#ifdef CONFIG_LINUX
 /* udmabuf.c */
 int udmabuf_fd(void);
+#endif
 
 #endif
diff --git a/ui/udmabuf.c b/ui/udmabuf.c
index 23abe1e7eb91..cebceb261001 100644
--- a/ui/udmabuf.c
+++ b/ui/udmabuf.c
@@ -8,8 +8,6 @@
 #include "qapi/error.h"
 #include "ui/console.h"
 
-#ifdef CONFIG_LINUX
-
 #include <fcntl.h>
 #include <sys/ioctl.h>
 
@@ -29,12 +27,3 @@ int udmabuf_fd(void)
     }
     return udmabuf;
 }
-
-#else
-
-int udmabuf_fd(void)
-{
-    return -1;
-}
-
-#endif
diff --git a/ui/meson.build b/ui/meson.build
index a3a187d633ab..7d25c1b95b5f 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -12,12 +12,14 @@ softmmu_ss.add(files(
   'kbd-state.c',
   'keymaps.c',
   'qemu-pixman.c',
-  'udmabuf.c',
 ))
 softmmu_ss.add([spice_headers, files('spice-module.c')])
 softmmu_ss.add(when: spice_protocol, if_true: files('vdagent.c'))
 
-softmmu_ss.add(when: 'CONFIG_LINUX', if_true: files('input-linux.c'))
+softmmu_ss.add(when: 'CONFIG_LINUX', if_true: files(
+  'input-linux.c',
+  'udmabuf.c',
+))
 softmmu_ss.add(when: cocoa, if_true: files('cocoa.m'))
 
 vnc_ss = ss.source_set()
-- 
2.31.1

[PULL 4/6] vga: don't abort when adding a duplicate isa-vga device

[Gerd Hoffmann]

From: "Jose R. Ziviani" <jziviani@suse.de>

If users try to add an isa-vga device that was already registered,
still in command line, qemu will crash:

$ qemu-system-mips64el -M pica61 -device isa-vga
RAMBlock "vga.vram" already registered, abort!
Aborted (core dumped)

That particular board registers the device automaticaly, so it's
not obvious that a VGA device already exists. This patch changes
this behavior by displaying a message and exiting without crashing.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/44
Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210817192629.12755-1-jziviani@suse.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/vga-isa.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index 90851e730bcd..8cea84f2bea6 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -33,6 +33,7 @@
 #include "hw/loader.h"
 #include "hw/qdev-properties.h"
 #include "qom/object.h"
+#include "qapi/error.h"
 
 #define TYPE_ISA_VGA "isa-vga"
 OBJECT_DECLARE_SIMPLE_TYPE(ISAVGAState, ISA_VGA)
@@ -61,6 +62,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
     MemoryRegion *vga_io_memory;
     const MemoryRegionPortio *vga_ports, *vbe_ports;
 
+    /*
+     * make sure this device is not being added twice, if so
+     * exit without crashing qemu
+     */
+    if (object_resolve_path_type("", TYPE_ISA_VGA, NULL)) {
+        error_setg(errp, "at most one %s device is permitted", TYPE_ISA_VGA);
+        return;
+    }
+
     s->global_vmstate = true;
     vga_common_init(s, OBJECT(dev));
     s->legacy_address_space = isa_address_space(isadev);
-- 
2.31.1

[PULL 5/6] hw/display/xlnx_dp: fix an out-of-bounds read in xlnx_dp_read

[Gerd Hoffmann]

From: Qiang Liu <cyruscyliu@gmail.com>

xlnx_dp_read allows an out-of-bounds read at its default branch because
of an improper index.

According to
https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html
(DP Module), registers 0x3A4/0x3A4/0x3AC are allowed.

DP_INT_MASK     0x000003A4      32      mixed   0xFFFFF03F      Interrupt Mask Register for intrN.
DP_INT_EN       0x000003A8      32      mixed   0x00000000      Interrupt Enable Register.
DP_INT_DS       0x000003AC      32      mixed   0x00000000      Interrupt Disable Register.

In xlnx_dp_write, when the offset is 0x3A8 and 0x3AC, the virtual device
will write s->core_registers[0x3A4
>> 2]. That is to say, the maxize of s->core_registers could be ((0x3A4
>> 2) + 1). However, the current size of s->core_registers is (0x3AF >>
>> 2), that is ((0x3A4 >> 2) + 2), which is out of the range.
In xlxn_dp_read, the access to offset 0x3A8 or 0x3AC will be directed to
the offset 0x3A8 (incorrect functionality) or 0x3AC (out-of-bounds read)
rather than 0x3A4.

This patch enforces the read access to offset 0x3A8 and 0x3AC to 0x3A4,
but does not adjust the size of s->core_registers to avoid breaking
migration.

Fixes: 58ac482a66de ("introduce xlnx-dp")
Signed-off-by: Qiang Liu <cyruscyliu@gmail.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Acked-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <1628059910-12060-1-git-send-email-cyruscyliu@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/xlnx_dp.c            |  6 +++++-
 tests/qtest/fuzz-xlnx-dp-test.c | 33 +++++++++++++++++++++++++++++++++
 tests/qtest/meson.build         |  1 +
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 tests/qtest/fuzz-xlnx-dp-test.c

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 2bb7a5441ad1..9bb781e31254 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -714,7 +714,11 @@ static uint64_t xlnx_dp_read(void *opaque, hwaddr offset, unsigned size)
         break;
     default:
         assert(offset <= (0x3AC >> 2));
-        ret = s->core_registers[offset];
+        if (offset == (0x3A8 >> 2) || offset == (0x3AC >> 2)) {
+            ret = s->core_registers[DP_INT_MASK];
+        } else {
+            ret = s->core_registers[offset];
+        }
         break;
     }
 
diff --git a/tests/qtest/fuzz-xlnx-dp-test.c b/tests/qtest/fuzz-xlnx-dp-test.c
new file mode 100644
index 000000000000..69eb6c0eb104
--- /dev/null
+++ b/tests/qtest/fuzz-xlnx-dp-test.c
@@ -0,0 +1,33 @@
+/*
+ * QTest fuzzer-generated testcase for xlnx-dp display device
+ *
+ * Copyright (c) 2021 Qiang Liu <cyruscyliu@gmail.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqos/libqtest.h"
+
+/*
+ * This used to trigger the out-of-bounds read in xlnx_dp_read
+ */
+static void test_fuzz_xlnx_dp_0x3ac(void)
+{
+    QTestState *s = qtest_init("-M xlnx-zcu102 -display none ");
+    qtest_readl(s, 0xfd4a03ac);
+    qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+    const char *arch = qtest_get_arch();
+
+    g_test_init(&argc, &argv, NULL);
+
+   if (strcmp(arch, "aarch64") == 0) {
+        qtest_add_func("fuzz/test_fuzz_xlnx_dp/3ac", test_fuzz_xlnx_dp_0x3ac);
+   }
+
+   return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 2bc3efd49f94..757bb8499a23 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -187,6 +187,7 @@ qtests_aarch64 = \
    'numa-test',
    'boot-serial-test',
    'xlnx-can-test',
+   'fuzz-xlnx-dp-test',
    'migration-test']
 
 qtests_s390x = \
-- 
2.31.1

[PULL 6/6] hw/display/artist: Fix bug in coordinate extraction in artist_vram_read() and artist_vram_write()

[Gerd Hoffmann]

From: Helge Deller <deller@gmx.de>

The CDE desktop on HP-UX 10 shows wrongly rendered pixels when the local screen
menu is closed. This bug was introduced by commit c7050f3f167b
("hw/display/artist: Refactor x/y coordination extraction") which converted the
coordinate extraction in artist_vram_read() and artist_vram_write() to use the
ADDR_TO_X and ADDR_TO_Y macros, but forgot to right-shift the address by 2 as
it was done before.

Signed-off-by: Helge Deller <deller@gmx.de>
Fixes: c7050f3f167b ("hw/display/artist: Refactor x/y coordination extraction")
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Sven Schnelle <svens@stackframe.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <YK1aPb8keur9W7h2@ls3530>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/artist.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/display/artist.c b/hw/display/artist.c
index aa7bd594aac0..21b7fd1b440e 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -1170,8 +1170,8 @@ static void artist_vram_write(void *opaque, hwaddr addr, uint64_t val,
     }
 
     buf = vram_write_buffer(s);
-    posy = ADDR_TO_Y(addr);
-    posx = ADDR_TO_X(addr);
+    posy = ADDR_TO_Y(addr >> 2);
+    posx = ADDR_TO_X(addr >> 2);
 
     if (!buf->size) {
         return;
@@ -1232,8 +1232,8 @@ static uint64_t artist_vram_read(void *opaque, hwaddr addr, unsigned size)
         return 0;
     }
 
-    posy = ADDR_TO_Y(addr);
-    posx = ADDR_TO_X(addr);
+    posy = ADDR_TO_Y(addr >> 2);
+    posx = ADDR_TO_X(addr >> 2);
 
     if (posy > buf->height || posx > buf->width) {
         return 0;
-- 
2.31.1

Re: [PULL 0/6] Vga 20210901 patches

[Peter Maydell]

On Wed, 1 Sept 2021 at 05:19, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit ad22d0583300df420819e6c89b1c022b998fac8a:
>
>   Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.2-20210827' into staging (2021-08-27 11:34:12 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/vga-20210901-pull-request
>
> for you to fetch changes up to 01f750f5fef1afd8f6abc0548910f87d473e26d5:
>
>   hw/display/artist: Fix bug in coordinate extraction in artist_vram_read() and artist_vram_write() (2021-08-31 14:38:53 +0200)
>
> ----------------------------------------------------------------
> vga: misc fixes and cleanups.
>
> ----------------------------------------------------------------


Applied, thanks.

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

-- PMM