<p>Neels Hofmeyr <strong>uploaded patch set #2</strong> to this change.</p><p><a href="https://gerrit.osmocom.org/13277">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">add caller-owns-msgb variant osmo_sccp_user_sap_down_nofree()<br><br>Add osmo_sccp_user_sap_down_nofree(), which is identical to<br>osmo_sccp_user_sap_down(), but doesn't imply a msgb_free().<br><br>To implement that, sccp_sclc_user_sap_down_nofree() with the same msgb<br>semantics is required.<br><br>Rationale:<br><br>Avoiding msgb leaks is easiest if the caller retains ownership of the msgb.<br>Take this hypothetical chain where leaks are obviously avoided:<br><br>  void send()<br>  {<br>       msg = msgb_alloc();<br>   dispatch(msg);<br>        msgb_free(msg);<br>  }<br><br>  void dispatch(msg)<br>  {<br>     osmo_fsm_inst_dispatch(fi, msg);<br>  }<br><br>  void fi_on_event(fi, data)<br>  {<br>    if (socket_is_ok)<br>             socket_write((struct msgb*)data);<br>  }<br><br>  void socket_write(msgb)<br>  {<br>      if (!ok1)<br>             return;<br>       if (ok2) {<br>            if (!ok3)<br>                     return;<br>               write(sock, msg->data);<br>    }<br>  }<br><br>However, if the caller passes ownership down to the msgb consumer, things<br>become nightmarishly complex:<br><br>  void send()<br>  {<br>      msg = msgb_alloc();<br>   rc = dispatch(msg);<br>   /* dispatching event failed? */<br>       if (rc)<br>               msgb_free(msg);<br>  }<br><br>  int dispatch(msg)<br>  {<br>      if (osmo_fsm_inst_dispatch(fi, msg))<br>          return -1;<br>    if (something_else())<br>         return -1; // <-- double free!<br>  }<br><br>  void fi_on_event(fi, data)<br>  {<br>   if (socket_is_ok) {<br>           socket_write((struct msgb*)data);<br>     else<br>          /* socket didn't consume? */<br>              msgb_free(data);<br>  }<br><br>  int socket_write(msgb)<br>  {<br>        if (!ok1)<br>             return -1; // <-- leak!<br>    if (ok2) {<br>            if (!ok3)<br>                     goto out;<br>             write(sock, msg->data);<br>    }<br>  out:<br>        msgb_free(msg);<br>    return -2;<br>  }<br><br>If any link in this call chain fails to be aware of the importance to return a<br>failed RC or to free a msgb if the chain is broken, or to not return a failed<br>RC if the msgb is consumed, we have a hidden msgb leak or double free.<br><br>This is the case with osmo_sccp_user_sap_down(). In new osmo-msc, passing data<br>through various FSM instances, there is high potential for leak/double-free<br>bugs. A very large brain is required to track down every msgb path.<br><br>osmo_sccp_user_sap_down_nofree() makes this problem trivial to solve even for<br>humans.<br><br>Change-Id: Ic818efa78b90f727e1a94c18b60d9a306644f340<br>---<br>M include/osmocom/sigtran/sccp_sap.h<br>M src/sccp_internal.h<br>M src/sccp_sclc.c<br>M src/sccp_scoc.c<br>4 files changed, 38 insertions(+), 23 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/77/13277/2</pre><p>To view, visit <a href="https://gerrit.osmocom.org/13277">change 13277</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.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.osmocom.org/13277"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmo-sccp </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newpatchset </div>
<div style="display:none"> Gerrit-Change-Id: Ic818efa78b90f727e1a94c18b60d9a306644f340 </div>
<div style="display:none"> Gerrit-Change-Number: 13277 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Harald Welte <laforge@gnumonks.org> </div>