>
> Hey,
>
> Does this make sense to have this in the 0.12 branch too?
>
> Christophe
>
I remember I was able to reproduce... I don't remember how.
Something like adding a line somewhere.
Perhaps shutting down the socket before handling SPICE_MSGC_ACK?
I think the patch is safe and fix a real problem.
Frediano
> On Tue, Jan 19, 2016 at 10:36:58AM +0000, Frediano Ziglio wrote:
> > This fixes a crash if red_channel_client disconnect is called
> > handling a message.
> > This can happen for instance handling SPICE_MSGC_ACK which calls
> > red_channel_client_push which try to write items to socket detecting
> > writing errors (for instance socket disconnection).
> > Messages are read in a loop and the red_channel_client_disconnect
> > would cause rcc->stream to be NULL which will turn in a use-after-free
> > problem (stream in red_peer_handle_incoming will use cached stream value).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/red-channel.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > Changes from v1:
> > - reduce diff size removing style change.
> >
> > diff --git a/server/red-channel.c b/server/red-channel.c
> > index 306c87d..efe889a 100644
> > --- a/server/red-channel.c
> > +++ b/server/red-channel.c
> > @@ -1233,6 +1233,10 @@ void red_channel_client_unref(RedChannelClient *rcc)
> > {
> > if (!--rcc->refs) {
> > spice_debug("destroy rcc=%p", rcc);
> > +
> > + reds_stream_free(rcc->stream);
> > + rcc->stream = NULL;
> > +
> > if (rcc->send_data.main.marshaller) {
> > spice_marshaller_destroy(rcc->send_data.main.marshaller);
> > }
> > @@ -1749,7 +1753,7 @@ void red_channel_pipes_add_empty_msg(RedChannel
> > *channel, int msg_type)
> > int red_channel_client_is_connected(RedChannelClient *rcc)
> > {
> > if (!rcc->dummy) {
> > - return rcc->stream != NULL;
> > + return ring_item_is_linked(&rcc->channel_link);
> > } else {
> > return rcc->dummy_connected;
> > }
> > @@ -1804,6 +1808,8 @@ static void
> > red_channel_remove_client(RedChannelClient *rcc)
> > rcc->channel->type, rcc->channel->id,
> > rcc->channel->thread_id, pthread_self());
> > }
> > + spice_return_if_fail(ring_item_is_linked(&rcc->channel_link));
> > +
> > ring_remove(&rcc->channel_link);
> > spice_assert(rcc->channel->clients_num > 0);
> > rcc->channel->clients_num--;
> > @@ -1845,8 +1851,6 @@ void red_channel_client_disconnect(RedChannelClient
> > *rcc)
> > rcc->channel->core->watch_remove(rcc->stream->watch);
> > rcc->stream->watch = NULL;
> > }
> > - reds_stream_free(rcc->stream);
> > - rcc->stream = NULL;
> > if (rcc->latency_monitor.timer) {
> > rcc->channel->core->timer_remove(rcc->latency_monitor.timer);
> > rcc->latency_monitor.timer = NULL;
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel