From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Mon, 2 Feb 2015 12:35:59 +0200 Subject: [PATCH] char-device: spice_char_device_write_to_device: protect against recursion This fixes Spice's smart card support and is related to commit 697f3214fd16adcd524456003619f7f44ddd031b. Reported-by: Swapna Krishnan Recursion is now possible starting with spice_char_device_write_to_device going through spice_char_device_wakeup (after going through qemu), calling again to spice_char_device_write_to_device. The protecting code is the same as the one protecting the read path. This function call loop makes the program to abort with the following messages: usb-ccid: chardev: unexpected message of type 3000000 qemu: qemu_mutex_lock: Resource deadlock avoided Backtrace: (gdb) bt * #0 0x00007ffff3fc78c7 in raise () from /lib64/libc.so.6 * #1 0x00007ffff3fc952a in abort () from /lib64/libc.so.6 * #2 0x0000555555969a95 in error_exit (err=35, * msg=0x5555559f8c90 <__func__.5119> "qemu_mutex_lock") * at util/qemu-thread-posix.c:48 * #3 0x0000555555969b82 in qemu_mutex_lock (mutex=0x5555562c4d60) * at util/qemu-thread-posix.c:79 * #4 0x0000555555714771 in qemu_chr_fe_write (s=0x5555562c4d60, * buf=0x7fffffffd2a0 "", len=12) at qemu-char.c:219 * #5 0x000055555586be49 in ccid_card_vscard_send_msg (s=0x5555565c5f80, * type=VSC_Error, reader_id=0, payload=0x7fffffffd2e0 "", length=4) * at hw/usb/ccid-card-passthru.c:75 * #6 0x000055555586bf00 in ccid_card_vscard_send_error (s=0x5555565c5f80, * reader_id=0, code=VSC_GENERAL_ERROR) at * hw/usb/ccid-card-passthru.c:91 * #7 0x000055555586c559 in ccid_card_vscard_handle_message ( * card=0x5555565c5f80, scr_msg_header=0x5555565c6008) * at hw/usb/ccid-card-passthru.c:254 * #8 0x000055555586c72f in ccid_card_vscard_read (opaque=0x5555565c5f80, * buf=0x5555565034b0 "", size=12) at hw/usb/ccid-card-passthru.c:289 * #9 0x00005555557149db in qemu_chr_be_write (s=0x5555562c4d60, * buf=0x5555565034b0 "", len=12) at qemu-char.c:305 * #10 0x000055555571cde5 in vmc_write (sin=0x5555562c4e78, * buf=0x5555565034b0 "", len=12) at spice-qemu-char.c:41 * #11 0x00007ffff4fa86aa in spice_char_device_write_to_device ( * dev=0x55555657f210) at char_device.c:462 * #12 0x00007ffff4fa9b48 in spice_char_device_wakeup (dev=0x55555657f210) * at char_device.c:862 * #13 0x00007ffff4ff7658 in spice_server_char_device_wakeup * (sin=0x5555562c4e78) at reds.c:2955 * #14 0x000055555571d1d2 in spice_chr_write (chr=0x5555562c4d60, * buf=0x7fffffffd560 "", len=12) at spice-qemu-char.c:189 * #15 0x0000555555714789 in qemu_chr_fe_write (s=0x5555562c4d60, * buf=0x7fffffffd560 "", len=12) at qemu-char.c:220 * #16 0x000055555586be49 in ccid_card_vscard_send_msg (s=0x5555565c5f80, * type=VSC_Error, reader_id=0, payload=0x7fffffffd5a0 "", length=4) * at hw/usb/ccid-card-passthru.c:75 * #17 0x000055555586bf00 in ccid_card_vscard_send_error * (s=0x5555565c5f80, * reader_id=0, code=VSC_SUCCESS) at hw/usb/ccid-card-passthru.c:91 * #18 0x000055555586c4fc in ccid_card_vscard_handle_message ( * card=0x5555565c5f80, scr_msg_header=0x5555565c6008) * at hw/usb/ccid-card-passthru.c:242 * #19 0x000055555586c72f in ccid_card_vscard_read (opaque=0x5555565c5f80, * buf=0x5555565034b0 "", size=12) at hw/usb/ccid-card-passthru.c:289 * #20 0x00005555557149db in qemu_chr_be_write (s=0x5555562c4d60, * buf=0x5555565034b0 "", len=12) at qemu-char.c:305 * #21 0x000055555571cde5 in vmc_write (sin=0x5555562c4e78, * buf=0x5555565034b0 "", len=12) at spice-qemu-char.c:41 * #22 0x00007ffff4fa86aa in spice_char_device_write_to_device ( * dev=0x55555657f210) at char_device.c:462 * #23 0x00007ffff4fa8d37 in spice_char_device_write_buffer_add ( * dev=0x55555657f210, write_buf=0x555556501f70) at char_device.c:597 * #24 0x00007ffff501142d in smartcard_channel_write_to_reader ( * write_buf=0x555556501f70) at smartcard.c:669 * #25 0x00007ffff501034c in smartcard_char_device_notify_reader_add ( * st=0x55555657ef00) at smartcard.c:335 * #26 0x00007ffff50112b3 in smartcard_add_reader (scc=0x555556493ee0, * name=0x5555565023cc "E-Gate 0 0") at smartcard.c:642 * #27 0x00007ffff50118d2 in smartcard_channel_handle_message ( * rcc=0x555556493ee0, type=101, size=22, msg=0x5555565023c0 "\003") * at smartcard.c:757 * #28 0x00007ffff4fbc168 in red_peer_handle_incoming * (stream=0x555556588250, handler=0x555556497ff0) at red_channel.c:308 * #29 0x00007ffff4fbc231 in red_channel_client_receive * (rcc=0x555556493ee0) at red_channel.c:326 * #30 0x00007ffff4fc0019 in red_channel_client_event (fd=59, event=1, * data=0x555556493ee0) at red_channel.c:1574 * #31 0x00005555558b6076 in watch_read (opaque=0x5555565002f0) * at ui/spice-core.c:101 * #32 0x00005555558e8d48 in qemu_iohandler_poll (pollfds=0x5555562b7630, * ret=2) at iohandler.c:143 * #33 0x00005555558e89a4 in main_loop_wait (nonblocking=0) at * main-loop.c:495 * #34 0x00005555557219b0 in main_loop () at vl.c:1794 * #35 0x0000555555729257 in main (argc=40, argv=0x7fffffffddc8, * envp=0x7fffffffdf10) at vl.c:4350 (cherry picked from commit 0c1f5b00e7907aefee13f86a234558f00cd6c7ef) --- server/char_device.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/char_device.c b/server/char_device.c index 285299c..e5d7c69 100644 --- a/server/char_device.c +++ b/server/char_device.c @@ -64,6 +64,7 @@ struct SpiceCharDeviceState { SpiceCharDeviceInstance *sin; int during_read_from_device; + int during_write_to_device; SpiceCharDeviceCallbacks cbs; void *opaque; @@ -441,6 +442,11 @@ static int spice_char_device_write_to_device(SpiceCharDeviceState *dev) return 0; } + /* protect against recursion with spice_char_device_wakeup */ + if (dev->during_write_to_device++ > 0) { + return 0; + } + spice_char_device_state_ref(dev); if (dev->write_to_dev_timer) { @@ -465,6 +471,11 @@ static int spice_char_device_write_to_device(SpiceCharDeviceState *dev) dev->cur_write_buf_pos; n = sif->write(dev->sin, dev->cur_write_buf_pos, write_len); if (n <= 0) { + if (dev->during_write_to_device > 1) { + dev->during_write_to_device = 1; + continue; /* a wakeup might have been called during the write - + make sure it doesn't get lost */ + } break; } total += n; @@ -489,6 +500,7 @@ static int spice_char_device_write_to_device(SpiceCharDeviceState *dev) } dev->active = dev->active || total; } + dev->during_write_to_device = 0; spice_char_device_state_unref(dev); return total; }