On Thu, Dec 01, 2016 at 02:20:29PM +0100, Victor Toso wrote: > 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 Ok, I'd mention this explicitly "making the tool crash on startup if it encounters an usbredir channel" > > > 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); I know this is going to be redundant, but session->priv->usb_manager = spice_usb_device_manager_get(session, &err); would look a bit better. Either way, Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel