Change in osmo-mgw[master]: libosmo-mgcp-client: fix memleak in case if no response is received

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/19271 )

Change subject: libosmo-mgcp-client: fix memleak in case if no response is received
......................................................................

libosmo-mgcp-client: fix memleak in case if no response is received

This problem was noticed while running several LCLS test cases from
ttcn3-bsc-test.  Every test case makes osmo-bsc leak at least two
chunks named 'struct mgcp_response_pending'.

Here is the related osmo-bsc output with additional debug messages:

  DRLL ERROR mgcp_client_fsm.c:525 MGCP_CONN(to-MSC)[0x612000016120]{ST_READY}:
             MGW/DLCX: abrupt FSM termination with connections still present,
             sending unconditional DLCX...
  DLMGCP DEBUG mgcp_client.c:1010 mgcp_client_next_trans_id(id=35): new trans ID
  DLMGCP DEBUG mgcp_client.c:918 mgcp_client_pending_add(id=35): allocated and queued
  DLMGCP DEBUG mgcp_client.c:962 Queued 53 bytes for MGCP GW
  DLMGCP DEBUG mgcp_client.c:725 Tx MGCP: r=127.0.0.1:2427<->l=127.0.0.1:2727:
               len=53 'DLCX 35 rtpbridge/1 at mgw MGCP 1.0\r\nC: 5\r\nI:'...
  DLMGCP ERROR mgcp_client.c:704 Failed to read: r=127.0.0.1:2427<->l=127.0.0.1:2727:
               111='Connection refused'

The MGCP client FSM enqueues a DLCX from its fsm_cleanup_cb(), and
terminates.  Thus if the remote MGCP peer becomes unavailable (e.g.
due to a network failure), we would never get a response, and since
the FSM is already terminated, nobody would pop and free() the
response handler from the queue (mgcp->responses_pending).

As a simple workaround, let's avoid allocating dummy entries of
'struct mgcp_response_pending' without a response handler.  The
only case where an MGCP message is sent without a handler is
exactly during the FSM termination.

Change-Id: I83938ff47fa8570b8d9dc810a184864a0c0b58aa
Related: OS#4619
---
M src/libosmo-mgcp-client/mgcp_client.c
1 file changed, 10 insertions(+), 5 deletions(-)

Approvals:
  laforge: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index 1ca7483..ffba074 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -985,7 +985,7 @@
 int mgcp_client_tx(struct mgcp_client *mgcp, struct msgb *msg,
 		   mgcp_response_cb_t response_cb, void *priv)
 {
-	struct mgcp_response_pending *pending;
+	struct mgcp_response_pending *pending = NULL;
 	mgcp_trans_id_t trans_id;
 	int rc;
 
@@ -997,10 +997,13 @@
 		return -EINVAL;
 	}
 
-	pending = mgcp_client_pending_add(mgcp, trans_id, response_cb, priv);
-	if (!pending) {
-		talloc_free(msg);
-		return -ENOMEM;
+	/* Do not allocate a dummy 'mgcp_response_pending' entry */
+	if (response_cb != NULL) {
+		pending = mgcp_client_pending_add(mgcp, trans_id, response_cb, priv);
+		if (!pending) {
+			talloc_free(msg);
+			return -ENOMEM;
+		}
 	}
 
 	if (msgb_l2len(msg) > 4096) {
@@ -1023,6 +1026,8 @@
 	return 0;
 
 mgcp_tx_error:
+	if (!pending)
+		return -1;
 	/* Dequeue pending response, it's going to be free()d */
 	llist_del(&pending->entry);
 	/* Pass NULL to response cb to indicate an error */

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/19271
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I83938ff47fa8570b8d9dc810a184864a0c0b58aa
Gerrit-Change-Number: 19271
Gerrit-PatchSet: 3
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Assignee: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: fixeria <axilirator at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200731/32ce5e3e/attachment.htm>