[Spice-devel] [PATCH] char-device: spice_char_device_write_to_device: protect against recursion

Marc-André Lureau mlureau at redhat.com
Tue Feb 3 05:34:58 PST 2015



----- Original Message -----
> On Mon, 2015-02-02 at 11:37 -0500, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> > > This fixes Spice's smart card support and is related to
> > > commit 697f3214fd16adcd524456003619f7f44ddd031b.
> > > 
> > > Reported-by: Swapna Krishnan <skrishna at redhat.com>
> > > 
> > > Recursion is now possible starting with spice_char_device_wakeup calling
> > >  spice_char_device_write_to_device that later (after going through qemu)
> > > calls spice_char_device_wakeup.
> 
> Actually, it's the other way:
> spice_char_device_write_to_device -> (calling)
>   (other functions) ->
>     spice_char_device_wakeup  ->
>       spice_char_device_write_to_device
> 
> > > 
> > > The protecting code is the same to the one in 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
> > > ---
> > > 
> > > Should I attach the backtrace in the commit message ?
> > 
> > Yes, please.
> 
> I'll post it here for now, and later add it to the commit message.

Thanks. Spice server is try to push pending messages to device after qemu calls wakeup to notify of a reply... I wish sometime the qemu chr device abstraction would simply be a pipe (system or userspace pipe with notify fds), so both ends could poll and I think that would simplify the code.

Given that the write_to_device will loop until the write queue is empty, and the solution is similar to read path, I think the patch looks good. ack 


> 
> (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
> (gdb)
> 
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 


More information about the Spice-devel mailing list