server/snd_worker.c: add reference counting to SndChannel

Fixes a valgrind discovered possible bug in spice-server - valgrind on
test_playback saw it, didn't see it happen with qemu.

The problem is that the frames buffers returned by spice_server_playback_get_buffer
are part of the malloc'ed SndChannel, whose lifetime is smaller then that of SndWorker.
As a result a pointer to a previously returned spice_server_playback_get_buffer could
remain and be used after SndChannel has been freed because the client disconnected.

1 files changed, 28 insertions, 3 deletions

diff --git a/server/snd_worker.c b/server/snd_worker.c
index 08947859..98f3cd12 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -84,6 +84,7 @@ struct SndChannel {
     RedsStream *stream;
     SndWorker *worker;
     spice_parse_channel_func_t parser;
+    int refs;
 
     RedChannelClient *channel_client;
 
@@ -207,6 +208,23 @@ static int check_cap(uint32_t *caps, int num_caps, uint32_t cap)
     return caps[i] & cap;
 }
 
+static SndChannel *snd_channel_get(SndChannel *channel)
+{
+    channel->refs++;
+    return channel;
+}
+
+static SndChannel *snd_channel_put(SndChannel *channel)
+{
+    if (!--channel->refs) {
+        channel->worker->connection = NULL;
+        free(channel);
+        red_printf("sound channel freed\n");
+        return NULL;
+    }
+    return channel;
+}
+
 static void snd_disconnect_channel(SndChannel *channel)
 {
     SndWorker *worker;
@@ -217,13 +235,12 @@ static void snd_disconnect_channel(SndChannel *channel)
     channel->cleanup(channel);
     worker = channel->worker;
     red_channel_client_destroy_dummy(worker->connection->channel_client);
-    worker->connection = NULL;
     core->watch_remove(channel->stream->watch);
     channel->stream->watch = NULL;
     reds_stream_free(channel->stream);
     spice_marshaller_destroy(channel->send_data.marshaller);
     free(channel->caps);
-    free(channel);
+    snd_channel_put(channel);
 }
 
 static void snd_playback_free_frame(PlaybackChannel *playback_channel, AudioFrame *frame)
@@ -903,6 +920,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
 
     ASSERT(size >= sizeof(*channel));
     channel = spice_malloc0(size);
+    channel->refs = 1;
     channel->parser = spice_get_client_channel_parser(channel_id, NULL);
     channel->stream = stream;
     channel->worker = worker;
@@ -949,6 +967,7 @@ static void snd_disconnect_channel_client(RedChannelClient *rcc)
 
     ASSERT(worker->connection->channel_client == rcc);
     snd_disconnect_channel(worker->connection);
+    ASSERT(worker->connection == NULL);
 }
 
 static void snd_set_command(SndChannel *channel, uint32_t command)
@@ -1049,6 +1068,7 @@ SPICE_GNUC_VISIBLE void spice_server_playback_get_buffer(SpicePlaybackInstance *
         return;
     }
     ASSERT(playback_channel->base.active);
+    snd_channel_get(channel);
 
     *frame = playback_channel->free_frames->samples;
     playback_channel->free_frames = playback_channel->free_frames->next;
@@ -1061,8 +1081,13 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
     PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
     AudioFrame *frame;
 
-    if (!channel)
+    if (!channel) {
         return;
+    }
+    if (!snd_channel_put(channel)) {
+        /* lost last reference, channel has been destroyed previously */
+        return;
+    }
     ASSERT(playback_channel->base.active);
 
     if (playback_channel->pending_frame) {