[Spice-devel] [PATCH spice] server: fix crash when restarting VM with old client

[Christophe Fergeau]

Hey,

I'd tend to add a similar check to spice_char_device_read_from_device()
as it's called from spice_server_vm_start() as well, and it's the only
other place accessing dev->sin.
Looks good otherwise.

Christophe

On Thu, Oct 09, 2014 at 05:58:14PM +0200, Marc-André Lureau wrote:
> The server will reset the vdagent char device when the client does not
> implement SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS. This will nullify
> dev->sin and the following crash will be reached on restart:
> 
>  #0  0x00007fb05aa264a1 in spice_char_device_write_to_device (dev=dev at entry=0x7fb066ae5d30) at char_device.c:443
>  #1  0x00007fb05aa27137 in spice_char_device_write_to_device (dev=0x7fb066ae5d30) at char_device.c:436
>  #2  spice_char_device_start (dev=0x7fb066ae5d30) at char_device.c:798
>  #3  0x00007fb05aa6a981 in spice_server_vm_start (s=<optimized out>) at reds.c:3795
>  #4  0x00007fb0644b7f89 in qdev_reset_one (dev=<optimized out>, opaque=<optimized out>) at hw/core/qdev.c:241
>  #5  0x00007fb0644b7918 in qbus_walk_children (bus=0x7fb06661e870, pre_devfn=0x0, pre_busfn=0x0,
>      post_devfn=0x7fb0644b7f80 <qdev_reset_one>, post_busfn=0x7fb0644b6350 <qbus_reset_one>, opaque=0x0)
>      at hw/core/qdev.c:422
>  #6  0x00007fb0644b7848 in qdev_walk_children (dev=0x7fb0665f47a0, pre_devfn=0x0, pre_busfn=0x0,
>      post_devfn=0x7fb0644b7f80 <qdev_reset_one>, post_busfn=0x7fb0644b6350 <qbus_reset_one>, opaque=0x0)
>      at hw/core/qdev.c:456
>  #7  0x00007fb0644b7918 in qbus_walk_children (bus=0x7fb06647cde0, pre_devfn=0x0, pre_busfn=0x0,
>      post_devfn=0x7fb0644b7f80 <qdev_reset_one>, post_busfn=0x7fb0644b6350 <qbus_reset_one>, opaque=0x0)
>      at hw/core/qdev.c:422
>  #8  0x00007fb0644399fd in qemu_devices_reset () at vl.c:1830
> 
> After restart, qemu will reset the device instance (sin) when virtio
> port is opened:
> 
>  #0  spice_char_device_state_reset_dev_instance (state=0x7fe4873876d0, sin=sin at entry=0x7fe486fb0c68)
>      at char_device.c:667
>  #1  0x00007fe47b277516 in attach_to_red_agent (sin=0x7fe486fb0c68) at reds.c:2838
>  #2  spice_server_char_device_add_interface (sin=0x7fe486fb0c68, s=0x7fe486fb2e60) at reds.c:2962
>  #3  spice_server_add_interface (s=0x7fe486fb2e60, sin=0x7fe486fb0c68) at reds.c:3104
>  #4  0x00007fe484c69e57 in vmc_register_interface (scd=0x7fe486fb0c60) at spice-qemu-char.c:123
>  #5  0x00007fe484ce96b4 in set_guest_connected (port=<optimized out>, guest_connected=1)
>      at hw/char/virtio-console.c:89
>  #6  0x00007fe484ba70ed in handle_control_message (len=8, buf=0x7fe486fbdf70, vser=0x7fe48739ae98)
>      at /usr/src/debug/qemu-2.1.0/hw/char/virtio-serial-bus.c:382
> 
> Let's ignore the call to spice_char_device_write_to_device() when
> dev->sin is NULL, similary to other conditions, such as dev->running.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1145919
> ---
>  server/char_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/server/char_device.c b/server/char_device.c
> index 660a788..e4759f9 100644
> --- a/server/char_device.c
> +++ b/server/char_device.c
> @@ -433,7 +433,7 @@ static int spice_char_device_write_to_device(SpiceCharDeviceState *dev)
>      int total = 0;
>      int n;
>  
> -    if (!dev->running || dev->wait_for_migrate_data) {
> +    if (!dev->running || dev->wait_for_migrate_data || !dev->sin) {
>          return 0;
>      }
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20141017/d1a72c7e/attachment.sig>