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