Fix: session lock use after free

The session lock is broken in that it does not handle teardown correctly
(use after free). Surround each usage by the session list lock for now
to fix this issue, and don't unlock the session lock after free. Since
each session lock usage is surrounded by session list lock, no other
thread will be left waiting on this lock when the session destroy is
performed.

This effectively renders useless the per-session lock. Leave it there
for now to minimize code change before 2.0 final.

This locking scheme will be revisited for lttng 2.1.

Acked-by: David Goulet <dgoulet@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 6880973a10a08a08d2ec6475021aef8e7f3cf760..4d8851ef813168a8445ad77ef12c1fdc838101d9 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -3303,9 +3303,13 @@ static int process_client_msg(struct command_ctx *cmd_ctx)
                break;
        default:
                DBG("Getting session %s by name", cmd_ctx->lsm->session.name);
+               /*
+                * We keep the session list lock across _all_ commands
+                * for now, because the per-session lock does not
+                * handle teardown properly.
+                */
                session_lock_list();
                cmd_ctx->session = session_find_by_name(cmd_ctx->lsm->session.name);
-               session_unlock_list();
                if (cmd_ctx->session == NULL) {
                        if (cmd_ctx->lsm->session.name != NULL) {
                                ret = LTTCOMM_SESS_NOT_FOUND;
@@ -3535,6 +3539,11 @@ skip_domain:
        {
                ret = cmd_destroy_session(cmd_ctx->session,
                                cmd_ctx->lsm->session.name);
+               /*
+                * Set session to NULL so we do not unlock it after
+                * free.
+                */
+               cmd_ctx->session = NULL;
                break;
        }
        case LTTNG_LIST_DOMAINS:
@@ -3669,6 +3678,9 @@ setup_error:
        if (cmd_ctx->session) {
                session_unlock(cmd_ctx->session);
        }
+       if (need_tracing_session) {
+               session_unlock_list();
+       }
 init_setup_error:
        return ret;
 }