From 256a55760810f90bdcbd3d8bc13963677ba49b9d Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 20 Mar 2012 13:39:20 -0400 Subject: [PATCH] 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 Signed-off-by: Mathieu Desnoyers --- src/bin/lttng-sessiond/main.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 6880973a1..4d8851ef8 100644 --- 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; } -- 2.34.1