qxl: check release info object

Commit Message

Prasad Pandit April 25, 2019, 6:35 a.m. UTC

From: Prasad J Pandit <[e-mail]>

When releasing spice resources in release_resource() routine,
if release info object 'ext.info' is null, it leads to null
pointer dereference. Add check to avoid it.

Reported-by: Bugs SysSec <[e-mail]>
Signed-off-by: Prasad J Pandit <[e-mail]>
---
 hw/display/qxl.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Prasad Pandit May 2, 2019, 7:28 a.m. UTC | #1

+-- On Thu, 25 Apr 2019, P J P wrote --+
| When releasing spice resources in release_resource() routine,
| if release info object 'ext.info' is null, it leads to null
| pointer dereference. Add check to avoid it.
| 
| diff --git a/hw/display/qxl.c b/hw/display/qxl.c
| index c8ce5781e0..632923add2 100644
| --- a/hw/display/qxl.c
| +++ b/hw/display/qxl.c
| @@ -777,6 +777,9 @@ static void interface_release_resource(QXLInstance *sin,
|      QXLReleaseRing *ring;
|      uint64_t *item, id;
|  
| +    if (!ext.info) {
| +        return;
| +    }
|      if (ext.group_id == MEMSLOT_GROUP_HOST) {
|          /* host group -> vga mode update request */
|          QXLCommandExt *cmdext = (void *)(intptr_t)(ext.info->id);
| 

Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Gerd Hoffmann May 7, 2019, 7:09 a.m. UTC | #2

On Thu, Apr 25, 2019 at 12:05:34PM +0530, P J P wrote:
> From: Prasad J Pandit <[e-mail]>
> 
> When releasing spice resources in release_resource() routine,
> if release info object 'ext.info' is null, it leads to null
> pointer dereference. Add check to avoid it.

Added to vga patch queue.

thanks,
  Gerd

Patch

===
(process:30785): Spice-WARNING **: 11:43:59.284: memslot.c:68:memslot_validate_virt: virtual address out of range
    virt=0x555556d247e0+0xbf slot_id=0 group_id=0
    slot=0x0-0x0 delta=0x0

Thread 5 "SPICE Worker" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffdb7ff700 (LWP 30792)]
interface_release_resource (sin=0x555556d12738, ext=...) at hw/display/qxl.c:785
785	        QXLCommandExt *cmdext = (void *)(intptr_t)(ext.info->id);
(gdb) bt
#0  0x0000555555adca68 in interface_release_resource (sin=0x555556d12738, ext=...) at hw/display/qxl.c:785
#1  0x00007ffff74991d5 in red_drawable_unref (red_drawable=0x7fffd402a520) at red-worker.c:100
#2  0x00007ffff749941c in red_drawable_unref (red_drawable=<optimized out>) at red-worker.c:229
#3  0x00007ffff749941c in red_process_display (worker=worker@entry=0x555556e2f050, ring_is_empty=ring_is_empty@entry=0x7fffdb7fe854) at red-worker.c:229
#4  0x00007ffff74995f2 in worker_source_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at red-worker.c:1265
#5  0x00007ffff7ec906d in g_main_dispatch (context=0x555556e38fc0) at gmain.c:3182
#6  0x00007ffff7ec906d in g_main_context_dispatch (context=context@entry=0x555556e38fc0) at gmain.c:3847
#7  0x00007ffff7ec9438 in g_main_context_iterate (context=0x555556e38fc0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3920
#8  0x00007ffff7ec9762 in g_main_loop_run (loop=0x7fffd4002100) at gmain.c:4116
#9  0x00007ffff7498dde in red_worker_main (arg=0x555556e2f050) at red-worker.c:1369
#10 0x00007ffff70e458e in start_thread () at /lib64/libpthread.so.0
#11 0x00007ffff7013683 in clone () at /lib64/libc.so.6
(gdb)
===

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index c8ce5781e0..632923add2 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -777,6 +777,9 @@  static void interface_release_resource(QXLInstance *sin,
     QXLReleaseRing *ring;
     uint64_t *item, id;
 
+    if (!ext.info) {
+        return;
+    }
     if (ext.group_id == MEMSLOT_GROUP_HOST) {
         /* host group -> vga mode update request */
         QXLCommandExt *cmdext = (void *)(intptr_t)(ext.info->id);