vnc: fix qemu crash because of SIGSEGV

The backtrace is:

0x00007f0b75cdf880 in pixman_image_get_stride () from /lib64/libpixman-1.so.0
0x00007f0b77bcb3cf in vnc_server_fb_stride (vd=0x7f0b7a1a2bb0) at ui/vnc.c:680
vnc_dpy_copy (dcl=0x7f0b7a1a2c00, src_x=224, src_y=263, dst_x=319, dst_y=363, w=1, h=1) at ui/vnc.c:915
0x00007f0b77bbcc35 in dpy_gfx_copy (con=0x7f0b7a146210, src_x=src_x@entry=224, src_y=src_y@entry=263, dst_x=dst_x@entry=319,
dst_y=dst_y@entry=363, w=1, h=1) at ui/console.c:1575
0x00007f0b77bbda4e in qemu_console_copy (con=<optimized out>, src_x=src_x@entry=224, src_y=src_y@entry=263, dst_x=dst_x@entry=319,
dst_y=dst_y@entry=363, w=<optimized out>, h=<optimized out>) at ui/console.c:2111
0x00007f0b77ac0980 in cirrus_do_copy (h=<optimized out>, w=<optimized out>, src=<optimized out>, dst=<optimized out>, s=0x7f0b7b086090) at hw/display/cirrus_vga.c:774
cirrus_bitblt_videotovideo_copy (s=0x7f0b7b086090) at hw/display/cirrus_vga.c:793
cirrus_bitblt_videotovideo (s=0x7f0b7b086090) at hw/display/cirrus_vga.c:915
cirrus_bitblt_start (s=0x7f0b7b086090) at hw/display/cirrus_vga.c:1056
0x00007f0b77965cfb in memory_region_write_accessor (mr=0x7f0b7b096e40, addr=320, value=<optimized out>, size=1, shift=<optimized out>,mask=<optimized out>, attrs=...) at /root/rpmbuild/BUILD/master/qemu/memory.c:525
0x00007f0b77963f59 in access_with_adjusted_size (addr=addr@entry=320, value=value@entry=0x7f0b69a268d8, size=size@entry=4,
access_size_min=<optimized out>, access_size_max=<optimized out>, access=access@entry=0x7f0b77965c80 <memory_region_write_accessor>,
mr=mr@entry=0x7f0b7b096e40, attrs=attrs@entry=...) at /root/rpmbuild/BUILD/master/qemu/memory.c:591
0x00007f0b77968315 in memory_region_dispatch_write (mr=mr@entry=0x7f0b7b096e40, addr=addr@entry=320, data=18446744073709551362,
size=size@entry=4, attrs=attrs@entry=...) at /root/rpmbuild/BUILD/master/qemu/memory.c:1262
0x00007f0b779256a9 in address_space_write_continue (mr=0x7f0b7b096e40, l=4, addr1=320, len=4, buf=0x7f0b77713028 "\002\377\377\377",
attrs=..., addr=4273930560, as=0x7f0b7827d280 <address_space_memory>) at /root/rpmbuild/BUILD/master/qemu/exec.c:2544
address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=<optimized out>, len=<optimized out>) at /root/rpmbuild/BUILD/master/qemu/exec.c:2601
0x00007f0b77925c1d in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., attrs@entry=...,
buf=buf@entry=0x7f0b77713028 "\002\377\377\377", len=<optimized out>, is_write=<optimized out>) at /root/rpmbuild/BUILD/master/qemu/exec.c:2703
0x00007f0b77962f53 in kvm_cpu_exec (cpu=cpu@entry=0x7f0b79fcc2d0) at /root/rpmbuild/BUILD/master/qemu/kvm-all.c:1965
0x00007f0b77950cc6 in qemu_kvm_cpu_thread_fn (arg=0x7f0b79fcc2d0) at /root/rpmbuild/BUILD/master/qemu/cpus.c:1078
0x00007f0b744b3dc5 in start_thread (arg=0x7f0b69a27700) at pthread_create.c:308
0x00007f0b70d3d66d in clone () from /lib64/libc.so.6

The code path while meeting segfault:
 vnc_dpy_copy
   vnc_update_client
     vnc_disconnect_finish [while vnc_disconnect_start() is invoked because somethins wrong]
       vnc_update_server_surface
         vd->server = NULL;
   vnc_server_fb_stride
     pixman_image_get_stride(vd->server)

Let's add a non-NULL check before calling vnc_server_fb_stride() to avoid segmentation fault.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrange <berrange@redhat.com>
Reported-by: Yanying Zhuang <ann.zhuangyanying@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 1472788698-120964-1-git-send-email-arei.gonglei@huawei.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

hw/usb/dev-mtp.c

@@ -48,9 +48,6 @@ enum mtp_code {
     CMD_GET_OBJECT_INFO            = 0x1008,
     CMD_GET_OBJECT                 = 0x1009,
     CMD_GET_PARTIAL_OBJECT         = 0x101b,
-    CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801,
-    CMD_GET_OBJECT_PROP_DESC       = 0x9802,
-    CMD_GET_OBJECT_PROP_VALUE      = 0x9803,
 
     /* response codes */
     RES_OK                         = 0x2001,
@@ -62,12 +59,10 @@ enum mtp_code {
     RES_INCOMPLETE_TRANSFER        = 0x2007,
     RES_INVALID_STORAGE_ID         = 0x2008,
     RES_INVALID_OBJECT_HANDLE      = 0x2009,
-    RES_INVALID_OBJECT_FORMAT_CODE = 0x200b,
     RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
     RES_INVALID_PARENT_OBJECT      = 0x201a,
     RES_INVALID_PARAMETER          = 0x201d,
     RES_SESSION_ALREADY_OPEN       = 0x201e,
-    RES_INVALID_OBJECT_PROP_CODE   = 0xA801,
 
     /* format codes */
     FMT_UNDEFINED_OBJECT           = 0x3000,
@@ -77,22 +72,6 @@ enum mtp_code {
     EVT_OBJ_ADDED                  = 0x4002,
     EVT_OBJ_REMOVED                = 0x4003,
     EVT_OBJ_INFO_CHANGED           = 0x4007,
-
-    /* object properties */
-    PROP_STORAGE_ID                = 0xDC01,
-    PROP_OBJECT_FORMAT             = 0xDC02,
-    PROP_OBJECT_COMPRESSED_SIZE    = 0xDC04,
-    PROP_PARENT_OBJECT             = 0xDC0B,
-    PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER = 0xDC41,
-    PROP_NAME                      = 0xDC44,
-};
-
-enum mtp_data_type {
-    DATA_TYPE_UINT16  = 0x0004,
-    DATA_TYPE_UINT32  = 0x0006,
-    DATA_TYPE_UINT64  = 0x0008,
-    DATA_TYPE_UINT128 = 0x000a,
-    DATA_TYPE_STRING  = 0xffff,
 };
 
 typedef struct {
@@ -136,8 +115,8 @@ struct MTPControl {
 struct MTPData {
     uint16_t     code;
     uint32_t     trans;
-    uint64_t     offset;
-    uint64_t     length;
+    uint32_t     offset;
+    uint32_t     length;
     uint32_t     alloc;
     uint8_t      *data;
     bool         first;
@@ -799,9 +778,6 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, MTPControl *c)
         CMD_GET_OBJECT_INFO,
         CMD_GET_OBJECT,
         CMD_GET_PARTIAL_OBJECT,
-        CMD_GET_OBJECT_PROPS_SUPPORTED,
-        CMD_GET_OBJECT_PROP_DESC,
-        CMD_GET_OBJECT_PROP_VALUE,
     };
     static const uint16_t fmt[] = {
         FMT_UNDEFINED_OBJECT,
@@ -907,12 +883,7 @@ static MTPData *usb_mtp_get_object_info(MTPState *s, MTPControl *c,
     usb_mtp_add_u32(d, QEMU_STORAGE_ID);
     usb_mtp_add_u16(d, o->format);
     usb_mtp_add_u16(d, 0);
-
-    if (o->stat.st_size > 0xFFFFFFFF) {
-        usb_mtp_add_u32(d, 0xFFFFFFFF);
-    } else {
-        usb_mtp_add_u32(d, o->stat.st_size);
-    }
+    usb_mtp_add_u32(d, o->stat.st_size);
 
     usb_mtp_add_u16(d, 0);
     usb_mtp_add_u32(d, 0);
@@ -995,122 +966,6 @@ static MTPData *usb_mtp_get_partial_object(MTPState *s, MTPControl *c,
     return d;
 }
 
-static MTPData *usb_mtp_get_object_props_supported(MTPState *s, MTPControl *c)
-{
-    static const uint16_t props[] = {
-        PROP_STORAGE_ID,
-        PROP_OBJECT_FORMAT,
-        PROP_OBJECT_COMPRESSED_SIZE,
-        PROP_PARENT_OBJECT,
-        PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER,
-        PROP_NAME,
-    };
-    MTPData *d = usb_mtp_data_alloc(c);
-    usb_mtp_add_u16_array(d, ARRAY_SIZE(props), props);
-
-    return d;
-}
-
-static MTPData *usb_mtp_get_object_prop_desc(MTPState *s, MTPControl *c)
-{
-    MTPData *d = usb_mtp_data_alloc(c);
-    switch (c->argv[0]) {
-    case PROP_STORAGE_ID:
-        usb_mtp_add_u16(d, PROP_STORAGE_ID);
-        usb_mtp_add_u16(d, DATA_TYPE_UINT32);
-        usb_mtp_add_u8(d, 0x00);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u8(d, 0x00);
-        break;
-    case PROP_OBJECT_FORMAT:
-        usb_mtp_add_u16(d, PROP_OBJECT_FORMAT);
-        usb_mtp_add_u16(d, DATA_TYPE_UINT16);
-        usb_mtp_add_u8(d, 0x00);
-        usb_mtp_add_u16(d, 0x0000);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u8(d, 0x00);
-        break;
-    case PROP_OBJECT_COMPRESSED_SIZE:
-        usb_mtp_add_u16(d, PROP_OBJECT_COMPRESSED_SIZE);
-        usb_mtp_add_u16(d, DATA_TYPE_UINT64);
-        usb_mtp_add_u8(d, 0x00);
-        usb_mtp_add_u64(d, 0x0000000000000000);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u8(d, 0x00);
-        break;
-    case PROP_PARENT_OBJECT:
-        usb_mtp_add_u16(d, PROP_PARENT_OBJECT);
-        usb_mtp_add_u16(d, DATA_TYPE_UINT32);
-        usb_mtp_add_u8(d, 0x00);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u8(d, 0x00);
-        break;
-    case PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER:
-        usb_mtp_add_u16(d, PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER);
-        usb_mtp_add_u16(d, DATA_TYPE_UINT128);
-        usb_mtp_add_u8(d, 0x00);
-        usb_mtp_add_u64(d, 0x0000000000000000);
-        usb_mtp_add_u64(d, 0x0000000000000000);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u8(d, 0x00);
-        break;
-    case PROP_NAME:
-        usb_mtp_add_u16(d, PROP_NAME);
-        usb_mtp_add_u16(d, DATA_TYPE_STRING);
-        usb_mtp_add_u8(d, 0x00);
-        usb_mtp_add_u8(d, 0x00);
-        usb_mtp_add_u32(d, 0x00000000);
-        usb_mtp_add_u8(d, 0x00);
-        break;
-    default:
-        usb_mtp_data_free(d);
-        return NULL;
-    }
-
-    return d;
-}
-
-static MTPData *usb_mtp_get_object_prop_value(MTPState *s, MTPControl *c,
-                                              MTPObject *o)
-{
-    MTPData *d = usb_mtp_data_alloc(c);
-    switch (c->argv[1]) {
-    case PROP_STORAGE_ID:
-        usb_mtp_add_u32(d, QEMU_STORAGE_ID);
-        break;
-    case PROP_OBJECT_FORMAT:
-        usb_mtp_add_u16(d, o->format);
-        break;
-    case PROP_OBJECT_COMPRESSED_SIZE:
-        usb_mtp_add_u64(d, o->stat.st_size);
-        break;
-    case PROP_PARENT_OBJECT:
-        if (o->parent == NULL) {
-            usb_mtp_add_u32(d, 0x00000000);
-        } else {
-            usb_mtp_add_u32(d, o->parent->handle);
-        }
-        break;
-    case PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER:
-        /* Should be persistant between sessions,
-         * but using our objedt ID is "good enough"
-         * for now */
-        usb_mtp_add_u64(d, 0x0000000000000000);
-        usb_mtp_add_u64(d, o->handle);
-        break;
-    case PROP_NAME:
-        usb_mtp_add_str(d, o->name);
-        break;
-    default:
-        usb_mtp_data_free(d);
-        return NULL;
-    }
-
-    return d;
-}
-
 static void usb_mtp_command(MTPState *s, MTPControl *c)
 {
     MTPData *data_in = NULL;
@@ -1258,43 +1113,6 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
         nres = 1;
         res0 = data_in->length;
         break;
-    case CMD_GET_OBJECT_PROPS_SUPPORTED:
-        if (c->argv[0] != FMT_UNDEFINED_OBJECT &&
-            c->argv[0] != FMT_ASSOCIATION) {
-            usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
-                                 c->trans, 0, 0, 0);
-            return;
-        }
-        data_in = usb_mtp_get_object_props_supported(s, c);
-        break;
-    case CMD_GET_OBJECT_PROP_DESC:
-        if (c->argv[1] != FMT_UNDEFINED_OBJECT &&
-            c->argv[1] != FMT_ASSOCIATION) {
-            usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
-                                 c->trans, 0, 0, 0);
-            return;
-        }
-        data_in = usb_mtp_get_object_prop_desc(s, c);
-        if (data_in == NULL) {
-            usb_mtp_queue_result(s, RES_INVALID_OBJECT_PROP_CODE,
-                                 c->trans, 0, 0, 0);
-            return;
-        }
-        break;
-    case CMD_GET_OBJECT_PROP_VALUE:
-        o = usb_mtp_object_lookup(s, c->argv[0]);
-        if (o == NULL) {
-            usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
-                                 c->trans, 0, 0, 0);
-            return;
-        }
-        data_in = usb_mtp_get_object_prop_value(s, c, o);
-        if (data_in == NULL) {
-            usb_mtp_queue_result(s, RES_INVALID_OBJECT_PROP_CODE,
-                                 c->trans, 0, 0, 0);
-            return;
-        }
-        break;
     default:
         trace_usb_mtp_op_unknown(s->dev.addr, c->code);
         usb_mtp_queue_result(s, RES_OPERATION_NOT_SUPPORTED,
@@ -1375,15 +1193,10 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p)
         }
         if (s->data_in !=  NULL) {
             MTPData *d = s->data_in;
-            uint64_t dlen = d->length - d->offset;
+            int dlen = d->length - d->offset;
             if (d->first) {
                 trace_usb_mtp_data_in(s->dev.addr, d->trans, d->length);
-                if (d->length + sizeof(container) > 0xFFFFFFFF) {
-                    container.length = cpu_to_le32(0xFFFFFFFF);
-                } else {
-                    container.length =
-                        cpu_to_le32(d->length + sizeof(container));
-                }
+                container.length = cpu_to_le32(d->length + sizeof(container));
                 container.type   = cpu_to_le16(TYPE_DATA);
                 container.code   = cpu_to_le16(d->code);
                 container.trans  = cpu_to_le32(d->trans);

hw/usb/hcd-xhci.c

@@ -1753,12 +1753,6 @@ static void xhci_xfer_report(XHCITransfer *xfer)
         unsigned int chunk = 0;
 
         switch (TRB_TYPE(*trb)) {
-        case TR_SETUP:
-            chunk = trb->status & 0x1ffff;
-            if (chunk > 8) {
-                chunk = 8;
-            }
-            break;
         case TR_DATA:
         case TR_NORMAL:
         case TR_ISOCH:
@@ -3715,7 +3709,8 @@ static void usb_xhci_exit(PCIDevice *dev)
     /* destroy msix memory region */
     if (dev->msix_table && dev->msix_pba
         && dev->msix_entry_used) {
-        msix_uninit(dev, &xhci->mem, &xhci->mem);
+        memory_region_del_subregion(&xhci->mem, &dev->msix_table_mmio);
+        memory_region_del_subregion(&xhci->mem, &dev->msix_pba_mmio);
     }
 
     usb_bus_release(&xhci->bus);

hw/usb/host-libusb.c

@@ -743,13 +743,10 @@ static void usb_host_speed_compat(USBHostDevice *s)
                         rc = libusb_get_ss_endpoint_companion_descriptor
                             (ctx, endp, &endp_ss_comp);
                         if (rc == LIBUSB_SUCCESS) {
-                            int streams = endp_ss_comp->bmAttributes & 0x1f;
-                            if (streams) {
-                                compat_full = false;
-                                compat_high = false;
-                            }
                             libusb_free_ss_endpoint_companion_descriptor
                                 (endp_ss_comp);
+                            compat_full = false;
+                            compat_high = false;
                         }
 #endif
                         break;

qemu-options.hx

@@ -1145,7 +1145,7 @@ Configure wan image compression (lossy for slow links).
 Default is auto.
 
 @item streaming-video=[off|all|filter]
-Configure video stream detection.  Default is filter.
+Configure video stream detection.  Default is off.
 
 @item agent-mouse=[on|off]
 Enable/disable passing mouse events via vdagent.  Default is on.

ui/curses.c

@@ -181,7 +181,7 @@ static kbd_layout_t *kbd_layout = NULL;
 
 static void curses_refresh(DisplayChangeListener *dcl)
 {
-    int chr, nextchr, keysym, keycode, keycode_alt;
+    int chr, keysym, keycode, keycode_alt;
 
     curses_winch_check();
 
@@ -195,15 +195,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
 
     graphic_hw_text_update(NULL, screen);
 
-    nextchr = ERR;
     while (1) {
         /* while there are any pending key strokes to process */
-        if (nextchr == ERR)
-            chr = getch();
-        else {
-            chr = nextchr;
-            nextchr = ERR;
-        }
+        chr = getch();
 
         if (chr == ERR)
             break;
@@ -224,13 +218,12 @@ static void curses_refresh(DisplayChangeListener *dcl)
 
         /* alt key */
         if (keycode == 1) {
-            nextchr = getch();
+            int nextchr = getch();
 
             if (nextchr != ERR) {
                 chr = nextchr;
                 keycode_alt = ALT;
-                keycode = curses2keycode[nextchr];
-                nextchr = ERR;
+                keycode = curses2keycode[chr];
 
                 if (keycode != -1) {
                     keycode |= ALT;
@@ -317,7 +310,10 @@ static void curses_refresh(DisplayChangeListener *dcl)
                 qemu_input_event_send_key_delay(0);
             }
         } else {
-            keysym = curses2qemu[chr];
+            keysym = -1;
+            if (chr < CURSES_KEYS) {
+                keysym = curses2qemu[chr];
+            }
             if (keysym == -1)
                 keysym = chr;
 

ui/vnc.c

@@ -911,6 +911,10 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl,
         }
     }
 
+    if (!vd->server) {
+        /* no client connected */
+        return;
+    }
     /* do bitblit op on the local surface too */
     pitch = vnc_server_fb_stride(vd);
     src_row = vnc_server_fb_ptr(vd, src_x, src_y);