When the metadata cache is expanded (reallocated) by
lttng_metadata_printf(), the metadata cache reader
(lttng_metadata_output_channel()) may use freed memory, because the
metadata cache is not protected from concurrent read accesses. The
metadata cache updates are protected from each other by the sessions
mutex, but metadata cache reads do not hold the sessions mutex.
Actually, the comment on top of lttng_metadata_output_channel() stating
"We have exclusive access to our metadata buffer (protected by the
sessions_mutex)" is simply wrong, because this mutex is never held when
calling lttng_metadata_output_channel().
Promote the per-stream lock to the metadata cache used by each of those
metadata streams, thus ensuring mutual exclusion between metadata cache
reallocation and readers.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
if (finalized)
mask |= POLLHUP;
if (finalized)
mask |= POLLHUP;
+ mutex_lock(&stream->metadata_cache->lock);
if (stream->metadata_cache->metadata_written >
stream->metadata_out)
mask |= POLLIN;
if (stream->metadata_cache->metadata_written >
stream->metadata_out)
mask |= POLLIN;
+ mutex_unlock(&stream->metadata_cache->lock);
metadata_stream->priv = buf;
stream_priv = metadata_stream;
metadata_stream->transport = channel->transport;
metadata_stream->priv = buf;
stream_priv = metadata_stream;
metadata_stream->transport = channel->transport;
- mutex_init(&metadata_stream->lock);
/*
* Since life-time of metadata cache differs from that of
/*
* Since life-time of metadata cache differs from that of
goto err_free_cache;
metadata_cache->cache_alloc = METADATA_CACHE_DEFAULT_SIZE;
kref_init(&metadata_cache->refcount);
goto err_free_cache;
metadata_cache->cache_alloc = METADATA_CACHE_DEFAULT_SIZE;
kref_init(&metadata_cache->refcount);
+ mutex_init(&metadata_cache->lock);
session->metadata_cache = metadata_cache;
INIT_LIST_HEAD(&metadata_cache->metadata_stream);
memcpy(&metadata_cache->uuid, &session->uuid,
session->metadata_cache = metadata_cache;
INIT_LIST_HEAD(&metadata_cache->metadata_stream);
memcpy(&metadata_cache->uuid, &session->uuid,
/*
* Serialize at most one packet worth of metadata into a metadata
* channel.
/*
* Serialize at most one packet worth of metadata into a metadata
* channel.
- * We have exclusive access to our metadata buffer (protected by the
- * sessions_mutex), so we can do racy operations such as looking for
- * remaining space left in packet and write, since mutual exclusion
- * protects us from concurrent writes.
+ * We grab the metadata cache mutex to get exclusive access to our metadata
+ * buffer and to the metadata cache. Exclusive access to the metadata buffer
+ * allows us to do racy operations such as looking for remaining space left in
+ * packet and write, since mutual exclusion protects us from concurrent writes.
+ * Mutual exclusion on the metadata cache allow us to read the cache content
+ * without racing against reallocation of the cache by updates.
* Returns the number of bytes written in the channel, 0 if no data
* was written and a negative value on error.
*/
* Returns the number of bytes written in the channel, 0 if no data
* was written and a negative value on error.
*/
size_t len, reserve_len;
/*
size_t len, reserve_len;
/*
- * Ensure we support mutiple get_next / put sequences followed
- * by put_next. The metadata stream lock internally protects
- * reading the metadata cache. It can indeed be read
- * concurrently by "get_next_subbuf" and "flush" operations on
- * the buffer invoked by different processes.
+ * Ensure we support mutiple get_next / put sequences followed by
+ * put_next. The metadata cache lock protects reading the metadata
+ * cache. It can indeed be read concurrently by "get_next_subbuf" and
+ * "flush" operations on the buffer invoked by different processes.
+ * Moreover, since the metadata cache memory can be reallocated, we
+ * need to have exclusive access against updates even though we only
+ * read it.
- mutex_lock(&stream->lock);
+ mutex_lock(&stream->metadata_cache->lock);
WARN_ON(stream->metadata_in < stream->metadata_out);
if (stream->metadata_in != stream->metadata_out)
goto end;
WARN_ON(stream->metadata_in < stream->metadata_out);
if (stream->metadata_in != stream->metadata_out)
goto end;
- mutex_unlock(&stream->lock);
+ mutex_unlock(&stream->metadata_cache->lock);
return ret;
}
/*
* Write the metadata to the metadata cache.
* Must be called with sessions_mutex held.
return ret;
}
/*
* Write the metadata to the metadata cache.
* Must be called with sessions_mutex held.
+ * The metadata cache lock protects us from concurrent read access from
+ * thread outputting metadata content to ring buffer.
*/
int lttng_metadata_printf(struct lttng_session *session,
const char *fmt, ...)
*/
int lttng_metadata_printf(struct lttng_session *session,
const char *fmt, ...)
return -ENOMEM;
len = strlen(str);
return -ENOMEM;
len = strlen(str);
+ mutex_lock(&session->metadata_cache->lock);
if (session->metadata_cache->metadata_written + len >
session->metadata_cache->cache_alloc) {
char *tmp_cache_realloc;
if (session->metadata_cache->metadata_written + len >
session->metadata_cache->cache_alloc) {
char *tmp_cache_realloc;
session->metadata_cache->metadata_written,
str, len);
session->metadata_cache->metadata_written += len;
session->metadata_cache->metadata_written,
str, len);
session->metadata_cache->metadata_written += len;
+ mutex_unlock(&session->metadata_cache->lock);
kfree(str);
list_for_each_entry(stream, &session->metadata_cache->metadata_stream, list)
kfree(str);
list_for_each_entry(stream, &session->metadata_cache->metadata_stream, list)
+ mutex_unlock(&session->metadata_cache->lock);
kfree(str);
return -ENOMEM;
}
kfree(str);
return -ENOMEM;
}
wait_queue_head_t read_wait; /* Reader buffer-level wait queue */
struct list_head list; /* Stream list */
struct lttng_transport *transport;
wait_queue_head_t read_wait; /* Reader buffer-level wait queue */
struct list_head list; /* Stream list */
struct lttng_transport *transport;
};
struct lttng_session {
};
struct lttng_session {
struct kref refcount; /* Metadata cache usage */
struct list_head metadata_stream; /* Metadata stream list */
uuid_le uuid; /* Trace session unique ID (copy) */
struct kref refcount; /* Metadata cache usage */
struct list_head metadata_stream; /* Metadata stream list */
uuid_le uuid; /* Trace session unique ID (copy) */
};
struct lttng_session *lttng_session_create(void);
};
struct lttng_session *lttng_session_create(void);