From d0b2e6cb6867f044a1fc4093fb429f2df854b45f Mon Sep 17 00:00:00 2001 From: Thibault Saunier <tsaunier@igalia.com> Date: Mon, 31 Mar 2025 11:10:12 -0300 Subject: [PATCH] validate: scenario: Fix race condition when ignoring EOS The part of the code that is commented with: ``` /* gst_validate_action_set_done() does not finish the action * immediately. Instead, it posts a task to the main thread to do most * of the work in _action_set_done(). * * While the EOS handling lock guarantees that if an action had to call * gst_validate_action_set_done() it has done so, it does not guarantee * that _action_set_done() has been called. * * Is it possible that this handler is run before _action_set_done(), so * we check at this point for actions that have a pending_set_done and * call it before continuing. */ ``` was not being executed in the case where the scenario was 'ignoring EOS' while it was also required. Also fix potential use after free in that specific code path. Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/9014> --- .../gst/validate/gst-validate-scenario.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/subprojects/gst-devtools/validate/gst/validate/gst-validate-scenario.c b/subprojects/gst-devtools/validate/gst/validate/gst-validate-scenario.c index 0d1b07a8ff..6ef5fe009a 100644 --- a/subprojects/gst-devtools/validate/gst/validate/gst-validate-scenario.c +++ b/subprojects/gst-devtools/validate/gst/validate/gst-validate-scenario.c @@ -5199,11 +5199,6 @@ handle_bus_message (MessageData * d) GstValidateActionType *stop_action_type; GstStructure *s; - if (!is_error && priv->ignore_eos) { - GST_INFO_OBJECT (scenario, "Got EOS but ignoring it!"); - goto done; - } - if (is_error && priv->allow_errors) { #ifndef GST_DISABLE_GST_DEBUG GError *err = NULL; @@ -5254,17 +5249,25 @@ handle_bus_message (MessageData * d) * Is it possible that this handler is run before _action_set_done(), so * we check at this point for actions that have a pending_set_done and * call it before continuing. */ - GList *actions = g_list_copy (priv->actions); + GList *actions = g_list_copy_deep (priv->actions, + (GCopyFunc) (gst_validate_action_ref), NULL); GList *i; for (i = actions; i; i = i->next) { GstValidateAction *action = (GstValidateAction *) i->data; if (action->priv->pending_set_done) _action_set_done (action); } - g_list_free (actions); + g_list_free_full (actions, (GDestroyNotify) gst_validate_action_unref); } if (!is_error) { + if (priv->ignore_eos) { + GST_INFO_OBJECT (scenario, + "Got EOS but ignoring it, executing next action?"); + GST_VALIDATE_SCENARIO_EOS_HANDLING_UNLOCK (scenario); + goto done; + } + priv->got_eos = TRUE; if (priv->wait_message_action) {