Hi,
On Thu, Dec 01, 2016 at 01:29:50PM +0100, Christophe Fergeau wrote:
> On Wed, Nov 30, 2016 at 06:36:32PM +0100, Victor Toso wrote:
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > SpiceSession does not initialize its SpiceUsbDeviceManager object on
> > startup that could lead to a race condition where channel-usbredir is
> > requested to flush data while it is uninitialized.
> >
> > In a few places, spice_usb_device_manager_get() is called as in
> > usb-device-widget.c and spice-gtk-session.c but not used in
> > spicy-stats, making the tool to crash on startup.
>
> Just running spicy-stats when there is a usbredir channel is going to
> cause a crash?
Yes
> Isn't this avoided by your next patch as well
Yes
> which makes sure host is not NULL before trying to flush?
I see this as two different problems, really.
1-) chanel-usbredir should take in consideration that it might not be
initialized (second patch)
2-) SpiceSession should initialize SpiceUsbDeviceManager (this patch)
otherwise, situations like (1) could happen;
spice_usb_device_manager_initable_init() could fail here so, the
situation in (1) might as well happen.
>
> Christophe
>
> >
> > #0 in usbredirhost_write_guest_data (host=0x0) at usbredir/usbredirhost/usbredirhost.c:876
> > #1 in spice_usbredir_channel_up (c=0x643830) at channel-usbredir.c:821
> > #2 in spice_channel_up (channel=0x643830) at spice-channel.c:1238
> > #3 in spice_channel_recv_auth (channel=0x643830) at spice-channel.c:1225
> > #4 in spice_channel_coroutine (data=0x643830) at spice-channel.c:2580
> > #5 in coroutine_trampoline (cc=0x642ec0) at coroutine_ucontext.c:63
> > #6 in continuation_trampoline (i0=6565568, i1=0) at continuation.c:55
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1399838
> >
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > Reported-by: Michael Cullen <michael@xxxxxxxxxxxxxxxxx>
> > ---
> > src/spice-session.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/src/spice-session.c b/src/spice-session.c
> > index f900bd1..91e4f97 100644
> > --- a/src/spice-session.c
> > +++ b/src/spice-session.c
> > @@ -281,6 +281,7 @@ static void spice_session_init(SpiceSession *session)
> > {
> > SpiceSessionPrivate *s;
> > gchar *channels;
> > + GError *err = NULL;
> >
> > SPICE_DEBUG("New session (compiled from package " PACKAGE_STRING ")");
> > s = session->priv = SPICE_SESSION_GET_PRIVATE(session);
> > @@ -293,6 +294,12 @@ static void spice_session_init(SpiceSession *session)
> > s->images = cache_image_new((GDestroyNotify)pixman_image_unref);
> > s->glz_window = glz_decoder_window_new();
> > update_proxy(session, NULL);
> > +
> > + spice_usb_device_manager_get(session, &err);
> > + if (err != NULL) {
> > + SPICE_DEBUG("Could not initialize SpiceUsbDeviceManager - %s", err->message);
> > + g_clear_error(&err);
> > + }
> > }
> >
> > static void
> > --
> > 2.9.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel