mgcp_client: add transaction cleanup
So far, if an MGCP message is sent, the transaction gets enqueued, but there is
no way to end the transaction other than receiving a valid reply. So, if the
caller decides that the transaction timed out and tears down the priv pointer
passed to mgcp_client_tx, and if then a late reply arrives, the callback will
dereference the invalid priv pointer and cause a segfault. Hence it is possible
to crash an mgcp_client program by sending a late response.
Furthermore, if no reply ever arrives, we would keep the pending response in
the list forever, amounting to a "memory leak".
Add mgcp_client_cancel() to discard a pending transaction. The caller can now
decide to discard a pending response when it sees fit (e.g. the caller's
timeout expired). This needs to be added to OsmoMSC and OsmoBSC.
Add mgcp_msg_trans_id() to provide an obvious way to obtain the transaction id
from a generated MGCP message.
No public API is broken; but refine the negative return code from
mgcp_client_rx(): return -ENOENT if no such transaction ID is found, and still
-1 if decoding failed. This is mainly for mgcp_client_test.
Implement a test for mgcp_client_cancel() in mgcp_client_test.c.
Tweak internal mgcp_client_response_pending_get() to take only the transaction
id as argument instead of the entire mgcp message struct.
Found-by: dexter
Related: OS#2695 OS#2696
Change-Id: I16811e168a46a82a05943252a737b3434143f4bd
diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c
index 37fe0b8..172faac 100644
--- a/tests/mgcp_client/mgcp_client_test.c
+++ b/tests/mgcp_client/mgcp_client_test.c
@@ -24,6 +24,7 @@
#include <osmocom/core/application.h>
#include <osmocom/mgcp_client/mgcp_client.h>
#include <osmocom/mgcp_client/mgcp_client_internal.h>
+#include <errno.h>
void *ctx;
@@ -73,7 +74,7 @@
static struct mgcp_client_conf conf;
struct mgcp_client *mgcp = NULL;
-static void reply_to(mgcp_trans_id_t trans_id, int code, const char *comment,
+static int reply_to(mgcp_trans_id_t trans_id, int code, const char *comment,
int conn_id, const char *params)
{
static char compose[4096 - 128];
@@ -87,7 +88,7 @@
printf("composed response:\n-----\n%s\n-----\n",
compose);
- mgcp_client_rx(mgcp, from_str(compose));
+ return mgcp_client_rx(mgcp, from_str(compose));
}
void test_response_cb(struct mgcp_response *response, void *priv)
@@ -225,6 +226,51 @@
msgb_free(msg);
}
+void test_mgcp_client_cancel()
+{
+ mgcp_trans_id_t trans_id;
+ struct msgb *msg;
+ struct mgcp_msg mgcp_msg = {
+ .verb = MGCP_VERB_CRCX,
+ .audio_ip = "192.168.100.23",
+ .endpoint = "23@mgw",
+ .audio_port = 1234,
+ .call_id = 47,
+ .conn_id = 11,
+ .conn_mode = MGCP_CONN_RECV_SEND,
+ .presence = (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID
+ | MGCP_MSG_PRESENCE_CONN_ID | MGCP_MSG_PRESENCE_CONN_MODE),
+ };
+
+ printf("\n%s():\n", __func__);
+ fprintf(stderr, "\n%s():\n", __func__);
+
+ if (mgcp)
+ talloc_free(mgcp);
+ mgcp = mgcp_client_init(ctx, &conf);
+
+ msg = mgcp_msg_gen(mgcp, &mgcp_msg);
+ trans_id = mgcp_msg_trans_id(msg);
+ fprintf(stderr, "- composed msg with trans_id=%u\n", trans_id);
+
+ fprintf(stderr, "- not in queue yet, cannot cancel yet\n");
+ OSMO_ASSERT(mgcp_client_cancel(mgcp, trans_id) == -ENOENT);
+
+ fprintf(stderr, "- enqueue\n");
+ dummy_mgcp_send(msg);
+
+ fprintf(stderr, "- cancel succeeds\n");
+ OSMO_ASSERT(mgcp_client_cancel(mgcp, trans_id) == 0);
+
+ fprintf(stderr, "- late response gets discarded\n");
+ OSMO_ASSERT(reply_to(trans_id, 200, "OK", 1, "v=0\r\n") == -ENOENT);
+
+ fprintf(stderr, "- canceling again does nothing\n");
+ OSMO_ASSERT(mgcp_client_cancel(mgcp, trans_id) == -ENOENT);
+
+ fprintf(stderr, "%s() done\n", __func__);
+}
+
static const struct log_info_cat log_categories[] = {
};
@@ -244,10 +290,13 @@
log_set_use_color(osmo_stderr_target, 0);
log_set_print_category(osmo_stderr_target, 1);
+ log_set_category_filter(osmo_stderr_target, DLMGCP, 1, LOGL_DEBUG);
+
mgcp_client_conf_init(&conf);
test_crcx();
test_mgcp_msg();
+ test_mgcp_client_cancel();
printf("Done\n");
fprintf(stderr, "Done\n");