Re: [PATCH 1/2 v3] remove dangling pointer for RedCharDeviceVDIPort

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, May 06, 2016 at 05:58:19AM -0400, Frediano Ziglio wrote:
> > 
> > Hey,
> > 
> > On Thu, May 05, 2016 at 05:03:58PM +0100, Frediano Ziglio wrote:
> > > 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
> > 
> > Not sure if this is quite fixed yet. From latest upstream (with this patch),
> > I
> > can manage to crash the guest with:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00007fdfb4fed556 in vdi_port_read_one_msg_from_device (sin=0x5603d2a36c68,
> > opaque=<optimized out>) at reds.c:840
> > 840         g_assert(RED_CHAR_DEVICE(reds->agent_dev) == sin->st);
> > (gdb) bt
> > #0  0x00007fdfb4fed556 in vdi_port_read_one_msg_from_device
> > (sin=0x5603d2a36c68, opaque=<optimized out>) at reds.c:840
> > #1  0x00007fdfb4faad41 in red_char_device_read_from_device
> > (dev=0x5603d2a3b150) at char-device.c:103
> > #2  0x00007fdfb4faad41 in red_char_device_read_from_device
> > (dev=0x5603d2a3b150) at char-device.c:347
> > #3  0x00007fdfb4fad67c in red_char_device_start (dev=0x5603d2a3b150) at
> > char-device.c:810
> > #4  0x00007fdfb4fe9d38 in spice_server_vm_start (reds=0x5603d2af7300) at
> > reds.c:4071
> > #5  0x00005603cfc22a49 in qdev_reset_one ()
> > #6  0x00005603cfc22358 in qbus_walk_children ()
> > #7  0x00005603cfc22288 in qdev_walk_children ()
> > #8  0x00005603cfc22358 in qbus_walk_children ()
> > #9  0x00005603cfbb9a9a in qemu_system_reset ()
> > #10 0x00005603cfaaf264 in main ()
> > 
> > ...
> > 
> > The agent restart also crashes the client which is what I'm focusing in fix
> > at
> > the moment... Happy crash Friday!
> > 
> > Cheers,
> >   toso
> > 
> 
> Did you apply both patches? Looks like you encountered issue fixed by second one.

No, I'm running from git master. I'll apply the patch and see if still crashes.

> Are you able to easily reproduce it?

I'm not 100% sure that this crash is the same that I was having a few minutes
ago. Guest and Client were crashing after a drag-and-drop operation. But when I
attached gdb to qemu, it stopped to happened. This backtrace came after I reboot
the guest vm.

> Even with v0.13.1 and this patch?

I did not try that. I'll check.

  toso

>
> Frediano
> 
> > >
> > > 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
> > 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]