Fix: metadata stream leak, missing list removal and locking

The metadata stream is part of a list of metadata streams in the
metadata cache. Its addition to the list should be protected by
the metadata cache lock. It needs to be paired with protection
of list iteration with the same lock.

Removal from the list is entirely missing, and should be added
to lttng_metadata_ring_buffer_release (with proper locking).

This missing list removal was probably not causing issues because the
metadata stream structure was leaked: a kfree() is missing from
lttng_metadata_ring_buffer_release as well.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/src/lttng-abi.c b/src/lttng-abi.c
index 312372fcb9bbc5d83bcc59011ad5834e6131d626..4dd6e7e5e2fff77290f493664b543e66471b0a96 100644 (file)
--- a/src/lttng-abi.c
+++ b/src/lttng-abi.c
@@ -1092,8 +1092,12 @@ int lttng_metadata_ring_buffer_release(struct inode *inode, struct file *file)
        struct lttng_metadata_stream *stream = file->private_data;
        struct lib_ring_buffer *buf = stream->priv;
 
+       mutex_lock(&stream->metadata_cache->lock);
+       list_del(&stream->list);
+       mutex_unlock(&stream->metadata_cache->lock);
        kref_put(&stream->metadata_cache->refcount, metadata_cache_destroy);
        module_put(stream->transport->owner);
+       kfree(stream);
        return lib_ring_buffer_release(inode, file, buf);
 }
 
@@ -1245,8 +1249,10 @@ int lttng_abi_open_metadata_stream(struct file *channel_file)
        if (ret < 0)
                goto fd_error;
 
+       mutex_lock(&session->metadata_cache->lock);
        list_add(&metadata_stream->list,
                &session->metadata_cache->metadata_stream);
+       mutex_unlock(&session->metadata_cache->lock);
        return ret;
 
 fd_error:

diff --git a/src/lttng-events.c b/src/lttng-events.c
index 2b72621cf2d6fccacaed54e3c8dde1159500ddb5..11b0ed5396e68adc1e2aea733318082002333f96 100644 (file)
--- a/src/lttng-events.c
+++ b/src/lttng-events.c
@@ -1700,9 +1700,9 @@ void lttng_metadata_end(struct lttng_session *session)
        if (atomic_dec_return(&session->metadata_cache->producing) == 0) {
                struct lttng_metadata_stream *stream;
 
-               mutex_unlock(&session->metadata_cache->lock);
                list_for_each_entry(stream, &session->metadata_cache->metadata_stream, list)
                        wake_up_interruptible(&stream->read_wait);
+               mutex_unlock(&session->metadata_cache->lock);
        }
 }