When a client disconnects remove it from the list of clients connected to the spice char-device. This was caused by commit 1cec1c5118b65124de6bc6f984f376ff4e297bfb ("reds: Make VDIPortState a GObject") as the lifespan of RedCharDevice was changed. This could be reproduced with: - start rhel7 machine - connect remote viewer (RV) - RV: login - connect ssh - SSH: stop agent - disconnect RV - SSH: start agent - connect to RV and caused (using address sanitizer): main_channel_handle_parsed: agent start ================================================================= ==29592==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c00001cff0 at pc 0x7fa85b6e8595 bp 0x7ffde3801940 sp 0x7ffde3801930 READ of size 8 at 0x60c00001cff0 thread T0 #0 0x7fa85b6e8594 in red_client_get_main /home/freddy/work/spice-server/server/red-channel.c:2190 #1 0x7fa85b7311e6 in vdi_port_send_msg_to_client /home/freddy/work/spice-server/server/reds.c:880 #2 0x7fa85b69383e in red_char_device_send_msg_to_client /home/freddy/work/spice-server/server/char-device.c:138 #3 0x7fa85b69383e in red_char_device_send_msg_to_clients /home/freddy/work/spice-server/server/char-device.c:356 #4 0x7fa85b69383e in red_char_device_read_from_device /home/freddy/work/spice-server/server/char-device.c:403 #5 0x55a2633b81c1 (/usr/bin/qemu-system-x86_64+0x5561c1) #6 0x55a2633afe7a (/usr/bin/qemu-system-x86_64+0x54de7a) #7 0x55a2634cb7b1 (/usr/bin/qemu-system-x86_64+0x6697b1) #8 0x55a2632078d0 (/usr/bin/qemu-system-x86_64+0x3a58d0) #9 0x55a26379b2e8 (/usr/bin/qemu-system-x86_64+0x9392e8) #10 0x55a26379a7a0 (/usr/bin/qemu-system-x86_64+0x9387a0) #11 0x55a26313fb78 in main (/usr/bin/qemu-system-x86_64+0x2ddb78) #12 0x7fa85a3cc57f in __libc_start_main (/lib64/libc.so.6+0x2057f) #13 0x55a26314b0c8 (/usr/bin/qemu-system-x86_64+0x2e90c8) 0x60c00001cff0 is located 48 bytes inside of 128-byte region [0x60c00001cfc0,0x60c00001d040) freed by thread T0 here: #0 0x7fa869e3667a in __interceptor_free (/lib64/libasan.so.2+0x9867a) #1 0x7fa85b6d75f7 in red_client_unref /home/freddy/work/spice-server/server/red-channel.c:2076 #2 0x7fa85b6ead74 in dispatcher_handle_single_read /home/freddy/work/spice-server/server/dispatcher.c:291 #3 0x7fa85b6ead74 in dispatcher_handle_recv_read /home/freddy/work/spice-server/server/dispatcher.c:314 #4 0x55a26379b2e8 (/usr/bin/qemu-system-x86_64+0x9392e8) #5 0x55a26379a7a0 (/usr/bin/qemu-system-x86_64+0x9387a0) #6 0x55a26313fb78 in main (/usr/bin/qemu-system-x86_64+0x2ddb78) #7 0x7fa85a3cc57f in __libc_start_main (/lib64/libc.so.6+0x2057f) previously allocated by thread T0 here: #0 0x7fa869e36b19 in __interceptor_calloc (/lib64/libasan.so.2+0x98b19) #1 0x7fa85b7d6858 in spice_malloc0 /home/freddy/work/spice-server/spice-common/common/mem.c:109 #2 0x7fa85b6e760c in red_client_new /home/freddy/work/spice-server/server/red-channel.c:2053 #3 0x7fa85b7449e4 in reds_handle_main_link /home/freddy/work/spice-server/server/reds.c:1762 #4 0x7fa85b7449e4 in reds_handle_link /home/freddy/work/spice-server/server/reds.c:2002 #5 0x7fa85b745d3a in reds_handle_ticket /home/freddy/work/spice-server/server/reds.c:2056 #6 0x55a26379b2e8 (/usr/bin/qemu-system-x86_64+0x9392e8) #7 0x55a26379a7a0 (/usr/bin/qemu-system-x86_64+0x9387a0) #8 0x55a26313fb78 in main (/usr/bin/qemu-system-x86_64+0x2ddb78) #9 0x7fa85a3cc57f in __libc_start_main (/lib64/libc.so.6+0x2057f) SUMMARY: AddressSanitizer: heap-use-after-free /home/freddy/work/spice-server/server/red-channel.c:2190 red_client_get_main Shadow bytes around the buggy address: 0x0c187fffb9a0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c187fffb9b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c187fffb9c0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c187fffb9d0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c187fffb9e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x0c187fffb9f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd[fd]fd 0x0c187fffba00: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c187fffba10: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c187fffba20: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c187fffba30: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c187fffba40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- server/reds.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) Changes from v2: - added a comment from Uri Lublin. diff --git a/server/reds.c b/server/reds.c index efd1429..67c262a 100644 --- a/server/reds.c +++ b/server/reds.c @@ -603,12 +603,10 @@ void reds_client_disconnect(RedsState *reds, RedClient *client) reds_mig_remove_wait_disconnect_client(reds, client); } - if (reds->agent_dev->priv->agent_attached) { - /* note that vdagent might be NULL, if the vdagent was once - * up and than was removed */ - if (red_char_device_client_exists(RED_CHAR_DEVICE(reds->agent_dev), client)) { - red_char_device_client_remove(RED_CHAR_DEVICE(reds->agent_dev), client); - } + /* note that client might be NULL, if the vdagent was once + * up and than was removed */ + if (red_char_device_client_exists(RED_CHAR_DEVICE(reds->agent_dev), client)) { + red_char_device_client_remove(RED_CHAR_DEVICE(reds->agent_dev), client); } ring_remove(&client->link); -- 2.5.5 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel