Fix: don't delete stream from connection recv list

We don't need to delete them from the list during a connection destroy
because it's only a reference to the stream that might be valid or not
during the connection destroy. There is no need at all to access the
stream's pointer at that point.

Commit cd2ef1ef1d54ced9e4d0d03b865bb7fc6a905f80 did not fixed the full
issue so this should fix the use after free race for good.

Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/bin/lttng-relayd/connection.c b/src/bin/lttng-relayd/connection.c
index f8315449a31052264847a608a5ef1a399c4ddf12..4cf87ad3471d87e21fb31943ec082cc00b597196 100644 (file)
--- a/src/bin/lttng-relayd/connection.c
+++ b/src/bin/lttng-relayd/connection.c
@@ -67,16 +67,8 @@ void connection_delete(struct lttng_ht *ht, struct relay_connection *conn)
 
 void connection_destroy(struct relay_connection *conn)
 {
-       struct relay_stream *stream, *tmp_stream;
-
        assert(conn);
 
-       /* Clean up recv list of this connection if any. */
-       cds_list_for_each_entry_safe(stream, tmp_stream, &conn->recv_head,
-                       recv_list) {
-               cds_list_del(&stream->recv_list);
-       }
-
        call_rcu(&conn->rcu_node, rcu_free_connection);
 }
 

diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
index d82a3412d7b8578be8f5b39abf0a9fd949ff07bc..a93151ac47f560875f07c9cbf113225f9dd892cc 100644 (file)
--- a/src/bin/lttng-relayd/main.c
+++ b/src/bin/lttng-relayd/main.c
@@ -1340,18 +1340,6 @@ int relay_close_stream(struct lttcomm_relayd_hdr *recv_hdr,
        session->stream_count--;
        assert(session->stream_count >= 0);
 
-       /*
-        * Remove the stream from the connection recv list since we are about to
-        * flag it invalid and thus might be freed. This has to be done here since
-        * only the control thread can do actions on that list.
-        *
-        * Note that this stream might NOT be in the list but we have to try to
-        * remove it here else this can race with the stream destruction freeing
-        * the object and the connection destroy doing a use after free when
-        * deleting the remaining nodes in this list.
-        */
-       cds_list_del(&stream->recv_list);
-
        /* Check if we can close it or else the data will do it. */
        try_close_stream(session, stream);