Fix: action-executor: leak of `work_item::subitems` field

[lttng-tools.git] / src / bin / lttng-sessiond / action-executor.c

diff --git a/src/bin/lttng-sessiond/action-executor.c b/src/bin/lttng-sessiond/action-executor.c
index 9a89865a930c142e395f134d6fb23303b3d3508d..853146d3b8b3bb6f77c3788cecfb2fa280f0409a 100644 (file)
--- a/src/bin/lttng-sessiond/action-executor.c
+++ b/src/bin/lttng-sessiond/action-executor.c
@@ -135,7 +135,7 @@ static int action_executor_snapshot_session_handler(
                struct action_executor *executor,
                const struct action_work_item *,
                struct action_work_subitem *);
-static int action_executor_group_handler(struct action_executor *executor,
+static int action_executor_list_handler(struct action_executor *executor,
                const struct action_work_item *,
                struct action_work_subitem *);
 static int action_executor_generic_handler(struct action_executor *executor,
@@ -148,7 +148,7 @@ static const action_executor_handler action_executors[] = {
        [LTTNG_ACTION_TYPE_STOP_SESSION] = action_executor_stop_session_handler,
        [LTTNG_ACTION_TYPE_ROTATE_SESSION] = action_executor_rotate_session_handler,
        [LTTNG_ACTION_TYPE_SNAPSHOT_SESSION] = action_executor_snapshot_session_handler,
-       [LTTNG_ACTION_TYPE_GROUP] = action_executor_group_handler,
+       [LTTNG_ACTION_TYPE_LIST] = action_executor_list_handler,
 };
 
 /* Forward declaration */
@@ -660,11 +660,11 @@ end:
        return ret;
 }
 
-static int action_executor_group_handler(struct action_executor *executor,
+static int action_executor_list_handler(struct action_executor *executor,
                const struct action_work_item *work_item,
                struct action_work_subitem *item)
 {
-       ERR("Execution of a group action by the action executor should never occur");
+       ERR("Execution of a list action by the action executor should never occur");
        abort();
 }
 
@@ -730,6 +730,7 @@ static void action_work_item_destroy(struct action_work_item *work_item)
        lttng_evaluation_destroy(work_item->evaluation);
        notification_client_list_put(work_item->client_list);
        lttng_dynamic_array_reset(work_item->subitems);
+       free(work_item->subitems);
        free(work_item);
 }
 
@@ -1010,7 +1011,7 @@ static int add_action_to_subitem_array(struct lttng_action *action,
        assert(action);
        assert(subitems);
 
-       if (type == LTTNG_ACTION_TYPE_GROUP) {
+       if (type == LTTNG_ACTION_TYPE_LIST) {
                unsigned int count, i;
 
                status = lttng_action_list_get_count(action, &count);
@@ -1031,7 +1032,7 @@ static int add_action_to_subitem_array(struct lttng_action *action,
 
                /*
                 * Go directly to the end since there is no need to add the
-                * group action by itself to the subitems array.
+                * list action by itself to the subitems array.
                 */
                goto end;
        }
@@ -1060,7 +1061,7 @@ static int add_action_to_subitem_array(struct lttng_action *action,
                                action, &session_name);
                assert(status == LTTNG_ACTION_STATUS_OK);
                break;
-       case LTTNG_ACTION_TYPE_GROUP:
+       case LTTNG_ACTION_TYPE_LIST:
        case LTTNG_ACTION_TYPE_UNKNOWN:
                /* Fallthrough */
        default:
@@ -1076,17 +1077,26 @@ static int add_action_to_subitem_array(struct lttng_action *action,
         * simplicity and consistency.
         */
        if (session_name != NULL) {
-               struct ltt_session *session = NULL;
+               uint64_t session_id;
 
-               session_lock_list();
-               session = session_find_by_name(session_name);
-               if (session) {
+               /*
+                * Instantaneous sampling of the session id if present.
+                *
+                * This method is preferred over `sessiond_find_by_name` then
+                * fetching the session'd id since `sessiond_find_by_name`
+                * requires the session list lock to be taken.
+                *
+                * Taking the session list lock can lead to a deadlock
+                * between the action executor and the notification thread
+                * (caller of add_action_to_subitem_array). It is okay if the
+                * session state changes between the enqueuing time and the
+                * execution time. The execution context is validated at
+                * execution time.
+                */
+               if (sample_session_id_by_name(session_name, &session_id)) {
                        LTTNG_OPTIONAL_SET(&subitem.context.session_id,
-                                       session->id);
-                       session_put(session);
+                                       session_id);
                }
-
-               session_unlock_list();
        }
 
        /* Get a reference to the action. */