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@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
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 */