<p>fixeria has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-msc/+/26460">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">libmsc: fix memory leak (struct msgb) in msc_i_ran_enc()<br><br>Function msc_i_ran_enc() calls msc_role_ran_encode(), but unlike the<br>other callers of this function it does not free() the encoded message.<br><br>A simple solution would be to call msgb_free(), like it's done in<br>the other places.  But a more elegant solution is to modify function<br>msc_role_ran_encode(), so that it attaches the msgb to OTC_SELECT.<br>This way there is no need to call msgb_free() here and there.<br><br>This change fixes a memleak observed while running ttcn3-msc-test.<br><br>Change-Id: I741e082badc32ba9a97c1495c894e1d22e122e3a<br>Related: OS#5340<br>---<br>M src/libmsc/msc_a.c<br>M src/libmsc/msc_a_remote.c<br>M src/libmsc/msc_t.c<br>M src/libmsc/msub.c<br>4 files changed, 7 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/60/26460/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/libmsc/msc_a.c b/src/libmsc/msc_a.c</span><br><span>index 74721d2..c9b0572 100644</span><br><span>--- a/src/libmsc/msc_a.c</span><br><span>+++ b/src/libmsc/msc_a.c</span><br><span>@@ -1659,12 +1659,9 @@</span><br><span>            .an_proto = msc_a->c.ran->an_proto,</span><br><span>            .msg = msc_role_ran_encode(msc_a->c.fi, ran_msg),</span><br><span>         };</span><br><span style="color: hsl(0, 100%, 40%);">-      int rc;</span><br><span>      if (!an_apdu.msg)</span><br><span>            return -EIO;</span><br><span style="color: hsl(0, 100%, 40%);">-    rc = _msub_role_dispatch(msc_a->c.msub, to_role, to_role_event, &an_apdu, file, line);</span><br><span style="color: hsl(0, 100%, 40%);">-   msgb_free(an_apdu.msg);</span><br><span style="color: hsl(0, 100%, 40%);">- return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+    return _msub_role_dispatch(msc_a->c.msub, to_role, to_role_event, &an_apdu, file, line);</span><br><span> }</span><br><span> </span><br><span> int msc_a_tx_dtap_to_i(struct msc_a *msc_a, struct msgb *dtap)</span><br><span>diff --git a/src/libmsc/msc_a_remote.c b/src/libmsc/msc_a_remote.c</span><br><span>index 84eff07..e4474f4 100644</span><br><span>--- a/src/libmsc/msc_a_remote.c</span><br><span>+++ b/src/libmsc/msc_a_remote.c</span><br><span>@@ -179,8 +179,6 @@</span><br><span>            return;</span><br><span> </span><br><span>  msc_a_remote_msg_up_to_remote_msc(msc_a, MSC_ROLE_T, OSMO_GSUP_MSGT_E_PREPARE_HANDOVER_ERROR, &an_apdu);</span><br><span style="color: hsl(0, 100%, 40%);">-    msgb_free(an_apdu.msg);</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span> }</span><br><span> </span><br><span> /*     [MSC-A---------------------]            [MSC-B---------------------]</span><br><span>diff --git a/src/libmsc/msc_t.c b/src/libmsc/msc_t.c</span><br><span>index af0ddaa..43bc74e 100644</span><br><span>--- a/src/libmsc/msc_t.c</span><br><span>+++ b/src/libmsc/msc_t.c</span><br><span>@@ -145,7 +145,6 @@</span><br><span>           return;</span><br><span> </span><br><span>  msub_role_dispatch(msc_t->c.msub, MSC_ROLE_A, MSC_A_EV_FROM_T_PREPARE_HANDOVER_FAILURE, &an_apdu);</span><br><span style="color: hsl(0, 100%, 40%);">-       msgb_free(an_apdu.msg);</span><br><span> }</span><br><span> </span><br><span> static int msc_t_ho_request_decode_and_store_cb(struct osmo_fsm_inst *msc_t_fi, void *data,</span><br><span>@@ -238,7 +237,6 @@</span><br><span> static int msc_t_send_stored_ho_request__decode_cb(struct osmo_fsm_inst *msc_t_fi, void *data,</span><br><span>                                                const struct ran_msg *ran_dec)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-   int rc;</span><br><span>      struct an_apdu an_apdu;</span><br><span>      struct msc_t *msc_t = msc_t_priv(msc_t_fi);</span><br><span>  struct osmo_sockaddr_str *rtp_ran_local = data;</span><br><span>@@ -263,9 +261,7 @@</span><br><span>        };</span><br><span>   if (!an_apdu.msg)</span><br><span>            return -EIO;</span><br><span style="color: hsl(0, 100%, 40%);">-    rc = msc_t_down_l2_co(msc_t, &an_apdu, true);</span><br><span style="color: hsl(0, 100%, 40%);">-       msgb_free(an_apdu.msg);</span><br><span style="color: hsl(0, 100%, 40%);">- return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+    return msc_t_down_l2_co(msc_t, &an_apdu, true);</span><br><span> }</span><br><span> </span><br><span> /* The MGW endpoint is created, we know our AoIP Transport Layer Address and can send the Handover Request to the RAN</span><br><span>@@ -472,9 +468,7 @@</span><br><span>  if (!an_apdu.msg)</span><br><span>            return -EIO;</span><br><span>         /* Send to remote MSC via msc_a_remote role */</span><br><span style="color: hsl(0, 100%, 40%);">-  rc = msub_role_dispatch(msc_t->c.msub, MSC_ROLE_A, MSC_A_EV_FROM_T_PREPARE_HANDOVER_RESPONSE, &an_apdu);</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_free(an_apdu.msg);</span><br><span style="color: hsl(0, 100%, 40%);">- return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+    return msub_role_dispatch(msc_t->c.msub, MSC_ROLE_A, MSC_A_EV_FROM_T_PREPARE_HANDOVER_RESPONSE, &an_apdu);</span><br><span> }</span><br><span> </span><br><span> static int msc_t_wait_ho_request_ack_decode_cb(struct osmo_fsm_inst *msc_t_fi, void *data,</span><br><span>diff --git a/src/libmsc/msub.c b/src/libmsc/msub.c</span><br><span>index 112703a..e4dd332 100644</span><br><span>--- a/src/libmsc/msub.c</span><br><span>+++ b/src/libmsc/msub.c</span><br><span>@@ -544,6 +544,8 @@</span><br><span>      *conn_p = NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* NOTE: the resulting message buffer will be attached to OTC_SELECT, so its lifetime</span><br><span style="color: hsl(120, 100%, 40%);">+ * is limited by the current select() loop iteration.  Use talloc_steal() to avoid this. */</span><br><span> struct msgb *msc_role_ran_encode(struct osmo_fsm_inst *fi, const struct ran_msg *ran_msg)</span><br><span> {</span><br><span>       struct msc_role_common *c = fi->priv;</span><br><span>@@ -556,6 +558,8 @@</span><br><span>       msg = c->ran->ran_encode(fi, ran_msg);</span><br><span>         if (!msg)</span><br><span>            LOGPFSML(fi, LOGL_ERROR, "Failed to encode %s\n", ran_msg_type_name(ran_msg->msg_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+          talloc_steal(OTC_SELECT, msg);</span><br><span>       return msg;</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-msc/+/26460">change 26460</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/c/osmo-msc/+/26460"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I741e082badc32ba9a97c1495c894e1d22e122e3a </div>
<div style="display:none"> Gerrit-Change-Number: 26460 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>