After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 562319 - [rtpsession] memory corruption
[rtpsession] memory corruption
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 0.10.10
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-11-26 10:10 UTC by Aurelien Grimaud
Modified: 2008-11-26 12:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
valgrind log (13.73 KB, text/plain)
2008-11-26 10:11 UTC, Aurelien Grimaud
Details

Description Aurelien Grimaud 2008-11-26 10:10:17 UTC
Running my application under valgrind, I get the attached output.

In rtp_session_process_rtp, the source have been freed after obtain_source.
I suspect a race condition when releasing the lock while performing call backs (on_xxx).
There is a window during which any thread can take the lock, especially the rtcp_thread which performs the session cleanup.
So, any time a callback is called, the current used source should be considered no more valid, even if it seems to be inside a lock.
In rtp_session_process_rtp, calling on_ssrc_validated or on_new_ssrc enables session_cleanup to be performed, thus invalidating source.
Source should be protected by referencing it whenever it is taken from hash_table.

There may also be a problem with hash_table concurrency.
Because of call backs, the lock can be released while g_hash_table_foreach_remove and another thread may perform g_hash_table_lookup or g_hash_table_insert.
Is this safe ?
Comment 1 Aurelien Grimaud 2008-11-26 10:11:40 UTC
Created attachment 123411 [details]
valgrind log
Comment 2 Wim Taymans 2008-11-26 12:10:32 UTC
What version of -bad is this?
Comment 3 Wim Taymans 2008-11-26 12:15:35 UTC
On closer inspection, there might indeed a race. I'll fix it.
Comment 4 Wim Taymans 2008-11-26 12:40:19 UTC
        * gst/rtpmanager/rtpsession.c: (obtain_source),
        (rtp_session_create_source), (rtp_session_process_rtp),
        (rtp_session_process_sr), (rtp_session_process_rr),
        (rtp_session_process_sdes), (rtp_session_process_bye):
        Make obtain_source return an aditional ref so that we don't lose our ref
        to it when a session cleanup occurs when we are emiting a signal.
        Emit the on_new_ssrc signal for the CSRC, not the SSRC.
        Fixes #562319.