From: Jan Alexander Steffens (heftig) <jan.steffens@ltnglobal.com>
Date: Mon, 7 Dec 2020 13:54:28 +0000 (+0100)
Subject: srt: Don't take object lock calling gst_srt_object_get_stats
X-Git-Tag: 1.19.3~507^2~975
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=470e6989d27b7a103cce096956c5721640d11950;p=platform%2Fupstream%2Fgstreamer.git

srt: Don't take object lock calling gst_srt_object_get_stats

This function takes the sock lock. This can result in a deadlock when
another thread holding the sock lock is trying to take the object lock.

Thread A (Holds object lock, wants sock lock):

    #2  gst_srt_object_get_stats at gst-plugins-bad/ext/srt/gstsrtobject.c:1753
    #3  gst_srt_object_get_property_helper at gst-plugins-bad/ext/srt/gstsrtobject.c:409
    #4  gst_srt_sink_get_property at gst-plugins-bad/ext/srt/gstsrtsink.c:95
    #5  g_object_get_property from libgobject-2.0.so.0

Thread B (Holds sock lock, wants object lock):

    #2  gst_element_post_message_default at gstreamer/gst/gstelement.c:2069
    #3  gst_element_post_message at gstreamer/gst/gstelement.c:2123
    #4  gst_element_message_full_with_details at gstreamer/gst/gstelement.c:2259
    #5  gst_element_message_full at gstreamer/gst/gstelement.c:2298
    #6  gst_srt_object_send_headers at gst-plugins-bad/ext/srt/gstsrtobject.c:1407
    #7  gst_srt_object_send_headers at gst-plugins-bad/ext/srt/gstsrtobject.c:1444
    #8  gst_srt_object_write_to_callers at gst-plugins-bad/ext/srt/gstsrtobject.c:1444
    #9  gst_srt_object_write at gst-plugins-bad/ext/srt/gstsrtobject.c:1598
    #10 gst_srt_sink_render at gst-plugins-bad/ext/srt/gstsrtsink.c:179

Fixes d2d00e07acc2b1ab1ae5a728ef5dc33c9dee7869.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1861>
---

diff --git a/ext/srt/gstsrtobject.c b/ext/srt/gstsrtobject.c
index 86dcf1edde..016870468b 100644
--- a/ext/srt/gstsrtobject.c
+++ b/ext/srt/gstsrtobject.c
@@ -362,84 +362,99 @@ gboolean
 gst_srt_object_get_property_helper (GstSRTObject * srtobject,
     guint prop_id, GValue * value, GParamSpec * pspec)
 {
-  GST_OBJECT_LOCK (srtobject->element);
-
   switch (prop_id) {
     case PROP_URI:
+      GST_OBJECT_LOCK (srtobject->element);
       g_value_take_string (value, gst_uri_to_string (srtobject->uri));
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
     case PROP_MODE:{
       GstSRTConnectionMode v;
+
+      GST_OBJECT_LOCK (srtobject->element);
       if (!gst_structure_get_enum (srtobject->parameters, "mode",
               GST_TYPE_SRT_CONNECTION_MODE, (gint *) & v)) {
         GST_WARNING_OBJECT (srtobject->element, "Failed to get 'mode'");
         v = GST_SRT_CONNECTION_MODE_NONE;
       }
       g_value_set_enum (value, v);
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
     }
     case PROP_LOCALADDRESS:
+      GST_OBJECT_LOCK (srtobject->element);
       g_value_set_string (value,
           gst_structure_get_string (srtobject->parameters, "localaddress"));
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
     case PROP_LOCALPORT:{
       guint v;
+
+      GST_OBJECT_LOCK (srtobject->element);
       if (!gst_structure_get_uint (srtobject->parameters, "localport", &v)) {
         GST_WARNING_OBJECT (srtobject->element, "Failed to get 'localport'");
         v = GST_SRT_DEFAULT_PORT;
       }
       g_value_set_uint (value, v);
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
     }
     case PROP_PBKEYLEN:{
       GstSRTKeyLength v;
+
+      GST_OBJECT_LOCK (srtobject->element);
       if (!gst_structure_get_enum (srtobject->parameters, "pbkeylen",
               GST_TYPE_SRT_KEY_LENGTH, (gint *) & v)) {
         GST_WARNING_OBJECT (srtobject->element, "Failed to get 'pbkeylen'");
         v = GST_SRT_KEY_LENGTH_NO_KEY;
       }
       g_value_set_enum (value, v);
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
     }
     case PROP_POLL_TIMEOUT:{
       gint v;
+
+      GST_OBJECT_LOCK (srtobject->element);
       if (!gst_structure_get_int (srtobject->parameters, "poll-timeout", &v)) {
         GST_WARNING_OBJECT (srtobject->element, "Failed to get 'poll-timeout'");
         v = GST_SRT_DEFAULT_POLL_TIMEOUT;
       }
       g_value_set_int (value, v);
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
     }
     case PROP_LATENCY:{
       gint v;
+
+      GST_OBJECT_LOCK (srtobject->element);
       if (!gst_structure_get_int (srtobject->parameters, "latency", &v)) {
         GST_WARNING_OBJECT (srtobject->element, "Failed to get 'latency'");
         v = GST_SRT_DEFAULT_LATENCY;
       }
       g_value_set_int (value, v);
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
     }
     case PROP_STATS:
       g_value_take_boxed (value, gst_srt_object_get_stats (srtobject));
       break;
     case PROP_WAIT_FOR_CONNECTION:
+      GST_OBJECT_LOCK (srtobject->element);
       g_value_set_boolean (value, srtobject->wait_for_connection);
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
-    case PROP_STREAMID:{
+    case PROP_STREAMID:
+      GST_OBJECT_LOCK (srtobject->element);
       g_value_set_string (value,
           gst_structure_get_string (srtobject->parameters, "streamid"));
+      GST_OBJECT_UNLOCK (srtobject->element);
       break;
-    }
     default:
-      goto err;
+      return FALSE;
   }
 
-  GST_OBJECT_UNLOCK (srtobject->element);
   return TRUE;
-
-err:
-  GST_OBJECT_UNLOCK (srtobject->element);
-  return FALSE;
 }
 
 void