<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/19271">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">libosmo-mgcp-client: fix memleak in case if no response is received<br><br>This problem was noticed while running several LCLS test cases from<br>ttcn3-bsc-test. Every test case makes osmo-bsc leak at least two<br>chunks named 'struct mgcp_response_pending'.<br><br>Here is the related osmo-bsc output with additional debug messages:<br><br> DRLL ERROR mgcp_client_fsm.c:525 MGCP_CONN(to-MSC)[0x612000016120]{ST_READY}:<br> MGW/DLCX: abrupt FSM termination with connections still present,<br> sending unconditional DLCX...<br> DLMGCP DEBUG mgcp_client.c:1010 mgcp_client_next_trans_id(id=35): new trans ID<br> DLMGCP DEBUG mgcp_client.c:918 mgcp_client_pending_add(id=35): allocated and queued<br> DLMGCP DEBUG mgcp_client.c:962 Queued 53 bytes for MGCP GW<br> DLMGCP DEBUG mgcp_client.c:725 Tx MGCP: r=127.0.0.1:2427<->l=127.0.0.1:2727:<br> len=53 'DLCX 35 rtpbridge/1@mgw MGCP 1.0\r\nC: 5\r\nI:'...<br> DLMGCP ERROR mgcp_client.c:704 Failed to read: r=127.0.0.1:2427<->l=127.0.0.1:2727:<br> 111='Connection refused'<br><br>The MGCP client FSM enqueues a DLCX from its fsm_cleanup_cb(), and<br>terminates. Thus if the remote MGCP peer becomes unavailable (e.g.<br>due to a network failure), we would never get a response, and since<br>the FSM is already terminated, nobody would pop and free() the<br>response handler from the queue (mgcp->responses_pending).<br><br>As a simple workaround, let's avoid allocating dummy entries of<br>'struct mgcp_response_pending' without a response handler. The<br>only case where an MGCP message is sent without a handler is<br>exactly during the FSM termination.<br><br>Change-Id: I83938ff47fa8570b8d9dc810a184864a0c0b58aa<br>Related: OS#4619<br>---<br>M src/libosmo-mgcp-client/mgcp_client.c<br>1 file changed, 10 insertions(+), 5 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c</span><br><span>index 1ca7483..ffba074 100644</span><br><span>--- a/src/libosmo-mgcp-client/mgcp_client.c</span><br><span>+++ b/src/libosmo-mgcp-client/mgcp_client.c</span><br><span>@@ -985,7 +985,7 @@</span><br><span> int mgcp_client_tx(struct mgcp_client *mgcp, struct msgb *msg,</span><br><span> mgcp_response_cb_t response_cb, void *priv)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct mgcp_response_pending *pending;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct mgcp_response_pending *pending = NULL;</span><br><span> mgcp_trans_id_t trans_id;</span><br><span> int rc;</span><br><span> </span><br><span>@@ -997,10 +997,13 @@</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- pending = mgcp_client_pending_add(mgcp, trans_id, response_cb, priv);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!pending) {</span><br><span style="color: hsl(0, 100%, 40%);">- talloc_free(msg);</span><br><span style="color: hsl(0, 100%, 40%);">- return -ENOMEM;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Do not allocate a dummy 'mgcp_response_pending' entry */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (response_cb != NULL) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pending = mgcp_client_pending_add(mgcp, trans_id, response_cb, priv);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!pending) {</span><br><span style="color: hsl(120, 100%, 40%);">+ talloc_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -ENOMEM;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> if (msgb_l2len(msg) > 4096) {</span><br><span>@@ -1023,6 +1026,8 @@</span><br><span> return 0;</span><br><span> </span><br><span> mgcp_tx_error:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!pending)</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span> /* Dequeue pending response, it's going to be free()d */</span><br><span> llist_del(&pending->entry);</span><br><span> /* Pass NULL to response cb to indicate an error */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/19271">change 19271</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-mgw/+/19271"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I83938ff47fa8570b8d9dc810a184864a0c0b58aa </div>
<div style="display:none"> Gerrit-Change-Number: 19271 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Assignee: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>