From: David Goulet Date: Wed, 2 Apr 2014 14:31:34 +0000 (-0400) Subject: Fix: use after free of a relayd stream X-Git-Tag: v2.4.1~2 X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=aeb8d6a340e0f991d61c19c00d636d1ba10d283e;p=lttng-tools.git Fix: use after free of a relayd stream A race could occur with a stream destruction and a control connection being destroyed emptying its recv_list. A freed stream could still be in the list thus having a use after free during the connection destroy. That was triggering undefined behavior from infinite looping to segmentation faults. We've observed this issue on high load stress test. A relayd received all the stream but NOT the streams sent command which empty the list. This can happen if a start tracing never occured or failed on the application side thus the close stream command is sent to the relayd freeing the stream before it is removed from that list. Signed-off-by: David Goulet --- diff --git a/src/bin/lttng-relayd/connection.h b/src/bin/lttng-relayd/connection.h index eeb1d545b..fc4a59075 100644 --- a/src/bin/lttng-relayd/connection.h +++ b/src/bin/lttng-relayd/connection.h @@ -54,6 +54,13 @@ struct relay_connection { uint32_t major; uint32_t minor; uint64_t session_id; + + /* + * This contains streams that are received on that connection. It's used to + * store them until we get the streams sent command where they are removed + * and flagged ready for the viewer. This is ONLY used by the control + * thread thus any action on it should happen in that thread. + */ struct cds_list_head recv_head; unsigned int version_check_done:1; diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c index aeb061330..89454b1e3 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -1204,6 +1204,18 @@ 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);