A failure on lttng_pipe_write() during send_stream_to_thread() leads to
a null-pointer dereference of the stream handle during
consumer_del_channel(). The chain of events leading to the problem
is:
- Failure during lttng_pipe_write() inside send_stream_to_thread().
- Call to consumer_stream_destroy() via consumer_del_stream_for_data()
or consumer_del_stream_for_metadata().
- The stream is monitor and globally visible at this point leading to
performing a call to destroy_close_stream() which performs the first
cleanup of the stream.
Note: At this point the stream is still in the channel local stream
list (stream.send_node).
- The call to unref_channel() returns a reference to a channel for which
a cleanup call must be done.
- The cleanup call for the channel is performed using
consumer_del_channel().
- At this point the stream is still in the channel's local stream list.
This results in a second call to consumer_stream_destroy() via
clean_channel_stream_list(). Which, itself, results in accesses to
freed memory.
The fix consists in:
- Using cds_list_del() inside send_stream_to_thread() after public
exposition of the stream to ensure that the stream ownership/visibility
is clear. A stream cannot be globally visible and local
(stream.send_node) to a channel at the same time.
- Modifying error paths to acknowledge the ownership transfer to
send_stream_to_thread().
Reported-by: Liguang Li <liguang.li@windriver.com>
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
/*
* From this point on, the stream's ownership has been moved away from
/*
* From this point on, the stream's ownership has been moved away from
- * the channel and becomes globally visible.
+ * the channel and it becomes globally visible. Hence, remove it from
+ * the local stream list to prevent the stream from being both local and
+ * global.
*/
stream->globally_visible = 1;
*/
stream->globally_visible = 1;
+ cds_list_del(&stream->send_node);
ret = lttng_pipe_write(stream_pipe, &stream, sizeof(stream));
if (ret < 0) {
ret = lttng_pipe_write(stream_pipe, &stream, sizeof(stream));
if (ret < 0) {
} else {
consumer_del_stream_for_data(stream);
}
} else {
consumer_del_stream_for_data(stream);
}
* If we are unable to send the stream to the thread, there is
* a big problem so just stop everything.
*/
* If we are unable to send the stream to the thread, there is
* a big problem so just stop everything.
*/
- /* Remove node from the channel stream list. */
- cds_list_del(&stream->send_node);
-
- /* Remove node from the channel stream list. */
- cds_list_del(&stream->send_node);
-
+ /*
+ * Ownership of metadata stream is passed along. Freeing is handled by
+ * the callee.
+ */
ret = send_streams_to_thread(metadata, ctx);
if (ret < 0) {
/*
ret = send_streams_to_thread(metadata, ctx);
if (ret < 0) {
/*
* a big problem so just stop everything.
*/
ret = LTTCOMM_CONSUMERD_FATAL;
* a big problem so just stop everything.
*/
ret = LTTCOMM_CONSUMERD_FATAL;
+ goto send_streams_error;
}
/* List MUST be empty after or else it could be reused. */
assert(cds_list_empty(&metadata->streams.head));
}
/* List MUST be empty after or else it could be reused. */
assert(cds_list_empty(&metadata->streams.head));
consumer_stream_destroy(metadata->metadata_stream, NULL);
cds_list_del(&metadata->metadata_stream->send_node);
metadata->metadata_stream = NULL;
consumer_stream_destroy(metadata->metadata_stream, NULL);
cds_list_del(&metadata->metadata_stream->send_node);
metadata->metadata_stream = NULL;
error_no_stream:
end:
return ret;
error_no_stream:
end:
return ret;