On Mon, Sep 11, 2017 at 11:15:43AM +0100, Frediano Ziglio wrote: > Code tested for the presence of a stream while initializing > RedChannelClient. "The code tests for the presence of RedChannelClient::stream while initializing RedChannelClient" > However the check was done too late possibly using the invalid > pointer in red_channel_client_config_socket. "However, the check was done too late, and a RedChannelClient::config_socket implementation (for example snd_channel_client_config_socket) could have tried to use it before the check that it's not NULL." Interestingly, this was introduced in 654dfa4c "red-channel-client: Change initialization order" The commit log does not say much, but this apparently was only changed to be closer to older code, not to fix an actual issue ( https://lists.freedesktop.org/archives/spice-devel/2016-October/033161.html ) Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > --- > Can the stream be NULL? By the way, I think this version > does not introduce possible regressions. > --- > server/red-channel-client.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/server/red-channel-client.c b/server/red-channel-client.c > index 9a47b7e6f..34202c492 100644 > --- a/server/red-channel-client.c > +++ b/server/red-channel-client.c > @@ -922,6 +922,14 @@ static gboolean red_channel_client_initable_init(GInitable *initable, > SpiceCoreInterfaceInternal *core; > RedChannelClient *self = RED_CHANNEL_CLIENT(initable); > > + if (!self->priv->stream) { > + g_set_error_literal(&local_error, > + SPICE_SERVER_ERROR, > + SPICE_SERVER_ERROR_FAILED, > + "Socket not available"); > + goto cleanup; > + } > + > if (!red_channel_client_config_socket(self)) { > g_set_error_literal(&local_error, > SPICE_SERVER_ERROR, > @@ -931,14 +939,12 @@ static gboolean red_channel_client_initable_init(GInitable *initable, > } > > core = red_channel_get_core_interface(self->priv->channel); > - if (self->priv->stream) { > - reds_stream_set_core_interface(self->priv->stream, core); > - self->priv->stream->watch = > - core->watch_add(core, self->priv->stream->socket, > - SPICE_WATCH_EVENT_READ, > - red_channel_client_event, > - self); > - } > + reds_stream_set_core_interface(self->priv->stream, core); > + self->priv->stream->watch = > + core->watch_add(core, self->priv->stream->socket, > + SPICE_WATCH_EVENT_READ, > + red_channel_client_event, > + self); > > if (self->priv->monitor_latency > && reds_stream_get_family(self->priv->stream) != AF_UNIX) { > -- > 2.13.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