> > Currently, calling spice_server_remove_interface() twice in a row with > the same SPICE_INTERFACE_CHAR_DEVICE is going to cause a crash when > calling red_char_device_get_server(char_device->st); because > char_device->st will have been set to NULL by the first call. > > This commit adds a few sanity checks before trying to use the various > 'st' members of the interfaces. > > This should avoid the crash described in > https://bugzilla.redhat.com/show_bug.cgi?id=1411194 even though it's not > clear how we got in that situation. Yes, there is no description on what he was doing. Maybe migration with device closed ? > --- > server/reds.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/server/reds.c b/server/reds.c > index 29485a8..90d126d 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -3307,8 +3307,11 @@ SPICE_GNUC_VISIBLE int > spice_server_remove_interface(SpiceBaseInstance *sin) > RedsState *reds; > const SpiceBaseInterface *interface = sin->sif; > > + g_return_val_if_fail(sin != NULL, -2); > + sin is already used, should be const SpiceBaseInterface *interface; g_return_val_if_fail(sin != NULL, -2); interface = sin->sif; or C99: g_return_val_if_fail(sin != NULL, -2); const SpiceBaseInterface *interface = sin->sif; > if (strcmp(interface->type, SPICE_INTERFACE_TABLET) == 0) { > SpiceTabletInstance *tablet = SPICE_CONTAINEROF(sin, > SpiceTabletInstance, base); > + g_return_val_if_fail(tablet->st != NULL, -2); > reds = spice_tablet_state_get_server(tablet->st); > spice_info("remove SPICE_INTERFACE_TABLET"); > inputs_channel_detach_tablet(reds->inputs_channel, tablet); > @@ -3321,12 +3324,14 @@ SPICE_GNUC_VISIBLE int > spice_server_remove_interface(SpiceBaseInstance *sin) > snd_detach_record(SPICE_CONTAINEROF(sin, SpiceRecordInstance, > base)); > } else if (strcmp(interface->type, SPICE_INTERFACE_CHAR_DEVICE) == 0) { > SpiceCharDeviceInstance *char_device = SPICE_CONTAINEROF(sin, > SpiceCharDeviceInstance, base); > + g_return_val_if_fail(char_device->st != NULL, -2); > reds = red_char_device_get_server(char_device->st); > spice_server_char_device_remove_interface(reds, sin); > } else if (strcmp(interface->type, SPICE_INTERFACE_QXL) == 0) { > QXLInstance *qxl; > > qxl = SPICE_CONTAINEROF(sin, QXLInstance, base); > + g_return_val_if_fail(qxl->st != NULL, -2); > reds = red_qxl_get_server(qxl->st); > reds->qxl_instances = g_list_remove(reds->qxl_instances, qxl); > red_qxl_destroy(qxl); Why -2 ? Beside these small comments patch looks fine. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel