Summary:	ASTERISK-24650: [patch] Fix access check to structures in rtp_engine.c
Reporter:	Badalian Vyacheslav (slavon)	Labels:
Date Opened:	2014-12-30 04:17:42.000-0600	Date Closed:	2016-01-28 11:41:46.000-0600
Priority:	Major	Regression?
Status:	Closed/Complete	Components:
Versions:	11.15.0	Frequency of Occurrence
Related Issues:	is related to ASTERISK-24651 [patch] Fix race condition in DTLS
Environment:		Attachments:	( 0) fix_rtp_engine_c_access_checks.diff
Description:	Patch to be more accurate in access to structures in INSTANCE. Its Needed to next patch (that fixes race conditions in DTLS) that i post later (then it will success run all our tests).
Comments:	By: Badalian Vyacheslav (slavon) 2014-12-30 04:19:34.954-0600 This code is safe to add becouse its simple will add additional checks for  access to structures from another modules. By: Rusty Newton (rnewton) 2015-01-02 12:04:31.368-0600 The next part of the process is to get your code up on Reviewboard. You can follow the [code review process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. By: Matt Jordan (mjordan) 2015-01-19 20:03:53.843-0600 Again, like your patch with extra locking, you aren't providing sufficient reason for the {{instance}} pointer checks for others to understand why it was necessary. Passing a {{NULL}} instance pointer into the RTP engine is an error. Users of the RTP engine shouldn't be doing that. Populating the code with those pointer checks is actually masking a problem elsewhere in the code. Do you have a backtrace showing the original problem you were attempting to solve? By: Badalian Vyacheslav (slavon) 2015-02-17 09:03:54.140-0600 If this can never be, what you are afraid of? These are the rules of good programming. Do not refer to elements until provrish that struct is alive. Due to the flow of the race conditions in SSL it is that it causes SIG FAULT. After my patch asterisk an error, closes the channel, but does not fall. By: Matt Jordan (mjordan) 2015-02-17 10:56:02.490-0600 [~slavon]: Because throwing a lot of NULL checks in can mask the root cause of a problem. Instead, you end up with strange failure conditions that are difficult to diagnose and troubleshoot. If the contract of an API is to not be handed a NULL pointer, then the caller should not be doing that. I'd like to understand *why* something is occurring, and not just slap bandaids on something that is having an issue. You have not yet shown what the actual problem is, and so it is incredibly difficult to know if your solution is the correct one, or is just masking a problem. We don't try mask problems, we try to fix them correctly. By: Corey Farrell (coreyfarrell) 2016-01-25 11:49:37.510-0600 [~slavon]: You claimed this causes a seg fault, we need to see the [backtrace|https://wiki.asterisk.org/wiki/display/AST/Getting+a+Backtrace].  As [~mjordan] pointed out if NULL is being passed to these procedures, you have found a bug in the caller, not in RTP. By: Badalian Vyacheslav (slavon) 2016-01-28 11:38:54.521-0600 i think you may close this bugreport. in asterisk 13 you have more accurate lock in dtls and test looks good. We now test chan_sip and chan_pjsip for havy dtls load.