gstpad: Avoid race in (un)setting EOS flag on sinkpads

The scenario is the following:

* Thread 1 is pushing an EOS event on a sinkpad
* Thread 2 is pushing a STREAM_START event on the same sinkpad before Thread 1
returns. Note : It starts pushing the event after Thread 1 took the object lock.

There is a potential race between:

* The moment Thread 1 sets the EOS flag once it has finished sending the
event (via store_sticky_event). When it does that it has both the STREAM and
OBJECT lock

* The moment Thread 2 sends the STREAM_START event (Which should release that
EOS status), but removing the EOS flag is only done while holding the OBJECT
lock and not the STREAM_LOCK, which means it could be re-set by Thread 1 before
it then checks again the EOS flag (without the STREAM lock taken).

The EOS flag unsetting by STREAM_START should be done with the STREAM lock
taken, otherwise it will be racy.

Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1452

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3320>
This commit is contained in:
Edward Hervey 2022-09-21 10:05:05 +02:00 committed by Edward Hervey
parent e81e212610
commit 605cb6a4d4

View file

@ -5850,6 +5850,17 @@ gst_pad_send_event_unchecked (GstPad * pad, GstEvent * event,
switch (event_type) { switch (event_type) {
case GST_EVENT_STREAM_START: case GST_EVENT_STREAM_START:
/* Take the stream lock to unset the EOS status. This is to ensure
* there isn't any other serialized event passing through while this
* EOS status is being unset */
/* lock order: STREAM_LOCK, LOCK, recheck flushing. */
GST_OBJECT_UNLOCK (pad);
GST_PAD_STREAM_LOCK (pad);
need_unlock = TRUE;
GST_OBJECT_LOCK (pad);
if (G_UNLIKELY (GST_PAD_IS_FLUSHING (pad)))
goto flushing;
/* Remove sticky EOS events */ /* Remove sticky EOS events */
GST_LOG_OBJECT (pad, "Removing pending EOS events"); GST_LOG_OBJECT (pad, "Removing pending EOS events");
remove_event_by_type (pad, GST_EVENT_EOS); remove_event_by_type (pad, GST_EVENT_EOS);
@ -5858,13 +5869,9 @@ gst_pad_send_event_unchecked (GstPad * pad, GstEvent * event,
GST_OBJECT_FLAG_UNSET (pad, GST_PAD_FLAG_EOS); GST_OBJECT_FLAG_UNSET (pad, GST_PAD_FLAG_EOS);
break; break;
default: default:
break;
}
if (serialized) { if (serialized) {
if (G_UNLIKELY (GST_PAD_IS_EOS (pad))) /* Take the stream lock to check the EOS status and drop the event
goto eos; * if that is the case. */
/* lock order: STREAM_LOCK, LOCK, recheck flushing. */ /* lock order: STREAM_LOCK, LOCK, recheck flushing. */
GST_OBJECT_UNLOCK (pad); GST_OBJECT_UNLOCK (pad);
GST_PAD_STREAM_LOCK (pad); GST_PAD_STREAM_LOCK (pad);
@ -5878,6 +5885,8 @@ gst_pad_send_event_unchecked (GstPad * pad, GstEvent * event,
} }
break; break;
} }
break;
}
/* now do the probe */ /* now do the probe */
PROBE_PUSH (pad, PROBE_PUSH (pad,