<p>Kevin Harwell <strong>uploaded patch set #3</strong> to this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/11697">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">srtp: Fix possible race condition, and add NULL checks<br><br>Somehow it's possible for the srtp session object to be NULL even though the<br>Asterisk srtp object itself is valid. When this happened it would cause a<br>crash down in the srtp code when attempting to protect or unprotect data.<br><br>After looking at the code there is at least one spot that makes this situation<br>possible. If Asterisk fails to unprotect the data, and after several retries<br>it still can't then the srtp->session gets freed, and set to NULL while still<br>leaving the Asterisk srtp object around. However, according to the original<br>issue reporter this does not appear to be their situation since they found<br>no errors logged stating the above happened (which Asterisk does for that<br>situation).<br><br>An issue was found however, where a possible race condition could occur between<br>the pjsip incoming negotiation, and the receiving of RTP packets. Both places<br>could attempt to create/setup srtp for the same rtp instance at the same time.<br>This potentially could be the cause of the problem as well.<br><br>Given the above this patch adds locking around srtp setup for a given rtp, or<br>rtcp instance. NULL checks for the session have also been added within the<br>protect and unprotect functions as a precaution. These checks should at least<br>stop Asterisk from crashing if it gets in this situation again.<br><br>This patch also fixes one other issue noticed during investigation. When doing<br>a replace the old object was freed before creating the replacement. If the new<br>replacement object failed to create then the rtp/rtcp instance would now point<br>to freed srtp data which could potentially cause a crash as well when the next<br>attempt to reference it was made. This is now fixed so the old srtp object is<br>kept upon replacement failure.<br><br>Lastly, more logging has been added to help diagnose future issues.<br><br>ASTERISK-28472<br><br>Change-Id: I240e11cbb1e9ea8083d59d50db069891228fe5cc<br>---<br>M main/rtp_engine.c<br>M res/res_pjsip_sdp_rtp.c<br>M res/res_srtp.c<br>3 files changed, 33 insertions(+), 5 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/97/11697/3</pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/11697">change 11697</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/11697"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I240e11cbb1e9ea8083d59d50db069891228fe5cc </div>
<div style="display:none"> Gerrit-Change-Number: 11697 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: newpatchset </div>