b38b0f
From 60df0d1b59e02c4ef2964473f84b707153ccad58 Mon Sep 17 00:00:00 2001
b38b0f
From: Gerd Hoffmann <kraxel@redhat.com>
b38b0f
Date: Tue, 13 Aug 2019 12:21:56 +0100
b38b0f
Subject: [PATCH 1/3] console: Avoid segfault in screendump
b38b0f
MIME-Version: 1.0
b38b0f
Content-Type: text/plain; charset=UTF-8
b38b0f
Content-Transfer-Encoding: 8bit
b38b0f
b38b0f
RH-Author: Gerd Hoffmann <kraxel@redhat.com>
b38b0f
Message-id: <20190813122156.5609-2-kraxel@redhat.com>
b38b0f
Patchwork-id: 89958
b38b0f
O-Subject: [RHEL-8.1.0 qemu-kvm PATCH 1/1] console: Avoid segfault in screendump
b38b0f
Bugzilla: 1684383
b38b0f
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
b38b0f
RH-Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
b38b0f
RH-Acked-by: John Snow <jsnow@redhat.com>
b38b0f
b38b0f
From: Michal Privoznik <mprivozn@redhat.com>
b38b0f
b38b0f
After f771c5440e04626f1 it is possible to select device and
b38b0f
head which to take screendump from. And even though we check if
b38b0f
provided head number falls within range, it may still happen that
b38b0f
the console has no surface yet leading to SIGSEGV:
b38b0f
b38b0f
  qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 \
b38b0f
    -qmp stdio \
b38b0f
    -device virtio-vga,id=video0,max_outputs=4
b38b0f
b38b0f
  {"execute":"qmp_capabilities"}
b38b0f
  {"execute":"screendump", "arguments":{"filename":"/tmp/screen.ppm", "device":"video0", "head":1}}
b38b0f
  Segmentation fault
b38b0f
b38b0f
 #0  0x00005628249dda88 in ppm_save (filename=0x56282826cbc0 "/tmp/screen.ppm", ds=0x0, errp=0x7fff52a6fae0) at ui/console.c:304
b38b0f
 #1  0x00005628249ddd9b in qmp_screendump (filename=0x56282826cbc0 "/tmp/screen.ppm", has_device=true, device=0x5628276902d0 "video0", has_head=true, head=1, errp=0x7fff52a6fae0) at ui/console.c:375
b38b0f
 #2  0x00005628247740df in qmp_marshal_screendump (args=0x562828265e00, ret=0x7fff52a6fb68, errp=0x7fff52a6fb60) at qapi/qapi-commands-ui.c:110
b38b0f
b38b0f
Here, @ds from frame #0 (or @surface from frame #1) is
b38b0f
dereferenced at the very beginning of ppm_save(). And because
b38b0f
it's NULL crash happens.
b38b0f
b38b0f
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
b38b0f
Reviewed-by: Thomas Huth <thuth@redhat.com>
b38b0f
Message-id: cb05bb1909daa6ba62145c0194aafa05a14ed3d1.1526569138.git.mprivozn@redhat.com
b38b0f
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
b38b0f
(cherry picked from commit 08d9864fa4e0c616e076ca8b225d39a7ecb189af)
b38b0f
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
b38b0f
---
b38b0f
 ui/console.c | 5 +++++
b38b0f
 1 file changed, 5 insertions(+)
b38b0f
b38b0f
diff --git a/ui/console.c b/ui/console.c
b38b0f
index 594ec63..4e4052f 100644
b38b0f
--- a/ui/console.c
b38b0f
+++ b/ui/console.c
b38b0f
@@ -370,6 +370,11 @@ void qmp_screendump(const char *filename, bool has_device, const char *device,
b38b0f
 
b38b0f
     graphic_hw_update(con);
b38b0f
     surface = qemu_console_surface(con);
b38b0f
+    if (!surface) {
b38b0f
+        error_setg(errp, "no surface");
b38b0f
+        return;
b38b0f
+    }
b38b0f
+
b38b0f
     ppm_save(filename, surface, errp);
b38b0f
 }
b38b0f
 
b38b0f
-- 
b38b0f
1.8.3.1
b38b0f