webrtc/datachannel: fix use-after-free in sctp state notification

g_signal_disconnect*() doesn't stop any existing callbacks from running
which means that if the notify::state callback is in progress in one
thread and the data channel object is finalize()ed in another thread,
then there could be a use-after-free trying lock the data channel
object.

We can't reasonably use a GWeakRef as we don't have a 'parent' object to
free the GWeakRef after the data channel is finalized.  This is also
complicated by the fact that the application can hold a reference to the
data channel object that would live beyond the lifetime of webrtcbin
itself.

We solve this by implementing a ghetto weak-ref solution internally with
a list of outstanding data channels.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1664>
This commit is contained in:
Matthew Waters 2021-11-26 22:11:06 +11:00 committed by GStreamer Marge Bot
parent 831b34fb43
commit 5741ee38e0
2 changed files with 44 additions and 0 deletions

View file

@ -50,6 +50,9 @@ G_DEFINE_TYPE_WITH_CODE (WebRTCDataChannel, webrtc_data_channel,
GST_DEBUG_CATEGORY_INIT (webrtc_data_channel_debug, "webrtcdatachannel", 0,
"webrtcdatachannel"););
G_LOCK_DEFINE_STATIC (outstanding_channels_lock);
static GList *outstanding_channels;
typedef enum
{
DATA_CHANNEL_PPID_WEBRTC_CONTROL = 50,
@ -877,13 +880,37 @@ _on_sctp_notify_state_unlocked (GObject * sctp_transport,
}
}
static WebRTCDataChannel *
ensure_channel_alive (WebRTCDataChannel * channel)
{
/* ghetto impl of, does the channel still exist?.
* Needed because g_signal_handler_disconnect*() will not disconnect any
* running functions and _finalize() implementation can complete and
* invalidate channel */
G_LOCK (outstanding_channels_lock);
if (g_list_find (outstanding_channels, channel)) {
g_object_ref (channel);
} else {
G_UNLOCK (outstanding_channels_lock);
return NULL;
}
G_UNLOCK (outstanding_channels_lock);
return channel;
}
static void
_on_sctp_notify_state (GObject * sctp_transport, GParamSpec * pspec,
WebRTCDataChannel * channel)
{
if (!(channel = ensure_channel_alive (channel)))
return;
GST_WEBRTC_DATA_CHANNEL_LOCK (channel);
_on_sctp_notify_state_unlocked (sctp_transport, channel);
GST_WEBRTC_DATA_CHANNEL_UNLOCK (channel);
g_object_unref (channel);
}
static void
@ -963,6 +990,16 @@ gst_webrtc_data_channel_constructed (GObject * object)
gst_caps_unref (caps);
}
static void
gst_webrtc_data_channel_dispose (GObject * object)
{
G_LOCK (outstanding_channels_lock);
outstanding_channels = g_list_remove (outstanding_channels, object);
G_UNLOCK (outstanding_channels_lock);
G_OBJECT_CLASS (parent_class)->dispose (object);
}
static void
gst_webrtc_data_channel_finalize (GObject * object)
{
@ -993,6 +1030,7 @@ webrtc_data_channel_class_init (WebRTCDataChannelClass * klass)
(GstWebRTCDataChannelClass *) klass;
gobject_class->constructed = gst_webrtc_data_channel_constructed;
gobject_class->dispose = gst_webrtc_data_channel_dispose;
gobject_class->finalize = gst_webrtc_data_channel_finalize;
channel_class->send_data = webrtc_data_channel_send_data;
@ -1003,6 +1041,9 @@ webrtc_data_channel_class_init (WebRTCDataChannelClass * klass)
static void
webrtc_data_channel_init (WebRTCDataChannel * channel)
{
G_LOCK (outstanding_channels_lock);
outstanding_channels = g_list_prepend (outstanding_channels, channel);
G_UNLOCK (outstanding_channels_lock);
}
static void
@ -1015,6 +1056,7 @@ _data_channel_set_sctp_transport (WebRTCDataChannel * channel,
GST_WEBRTC_DATA_CHANNEL_LOCK (channel);
if (channel->sctp_transport)
g_signal_handlers_disconnect_by_data (channel->sctp_transport, channel);
GST_TRACE ("%p set sctp %p", channel, sctp);
gst_object_replace ((GstObject **) & channel->sctp_transport,
GST_OBJECT (sctp));

View file

@ -179,6 +179,8 @@ gst_webrtc_data_channel_finalize (GObject * object)
g_free (channel->protocol);
channel->protocol = NULL;
g_mutex_clear (&channel->lock);
G_OBJECT_CLASS (parent_class)->finalize (object);
}