dexter has uploaded this change for review.
mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically
The struct mgcp_conn_peer is allocated statically at the moment. This is
a problem because in case new struct members are added, all those
statocally allocated locations in the programs using the mgcp_client_fsm
API may cause memory corruptions. So let's add an alloc function for this
struct that API users can use.
Let's also use that alloc function in mgcp_client_endpoint_fsm.c for
good measure.
Related: OS#6171
Change-Id: I523d0fcb020f7d46323c497a4be9ee00d5f242ba
---
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
M src/libosmo-mgcp-client/mgcp_client_fsm.c
3 files changed, 57 insertions(+), 16 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/04/34404/1
diff --git a/include/osmocom/mgcp_client/mgcp_client_fsm.h b/include/osmocom/mgcp_client/mgcp_client_fsm.h
index 658d5ea..6127286 100644
--- a/include/osmocom/mgcp_client/mgcp_client_fsm.h
+++ b/include/osmocom/mgcp_client/mgcp_client_fsm.h
@@ -12,7 +12,11 @@
* When modifiying a connection, the endpoint and call_id members may be left
* unpopulated. The call_id field is ignored in this case. If an endpoint
* identifier is supplied it is checked against the internal state to make
- * sure it is correct. */
+ * sure it is correct.
+ *
+ * CAUTION: This struct may be subject to changes and new struct members may
+ * be added in the future. To prevent memory conflicts it is strongly advised
+ * to allocate this struct dynamically using mgcp_conn_peer_alloc() */
struct mgcp_conn_peer {
/*! RTP connection IP-Address (optional, string e.g. "127.0.0.1") */
char addr[INET6_ADDRSTRLEN];
@@ -70,6 +74,7 @@
struct mgcp_codec_param2 codecs_param[MGCP_MAX_CODECS];
};
+struct mgcp_conn_peer *mgcp_conn_peer_alloc(void *ctx);
struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
uint32_t parent_evt, struct mgcp_conn_peer *conn_peer);
int mgcp_conn_modify(struct osmo_fsm_inst *fi, uint32_t parent_evt, struct mgcp_conn_peer *conn_peer);
diff --git a/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c b/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
index a01bbdc..769bf37 100644
--- a/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
+++ b/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
@@ -44,9 +44,9 @@
} while(0)
#define LOG_CI_VERB(ci, level, fmt, args...) do { \
- if (ci->verb_info.addr[0]) \
+ if (ci->verb_info->addr[0]) \
LOG_CI(ci, level, "%s %s:%u: " fmt, \
- osmo_mgcp_verb_name(ci->verb), ci->verb_info.addr, ci->verb_info.port, \
+ osmo_mgcp_verb_name(ci->verb), ci->verb_info->addr, ci->verb_info->port, \
## args); \
else \
LOG_CI(ci, level, "%s: " fmt, \
@@ -95,11 +95,11 @@
bool pending;
bool sent;
enum mgcp_verb verb;
- struct mgcp_conn_peer verb_info;
+ struct mgcp_conn_peer *verb_info;
struct fsm_notify notify;
bool got_port_info;
- struct mgcp_conn_peer rtp_info;
+ struct mgcp_conn_peer *rtp_info;
char mgcp_ci_str[MGCP_CONN_ID_LENGTH];
};
@@ -297,6 +297,7 @@
struct osmo_fsm_inst *fi;
struct osmo_mgcpc_ep *ep;
int rc;
+ size_t i;
if (!mgcp_client)
return NULL;
@@ -309,6 +310,11 @@
ep = talloc_zero(fi, struct osmo_mgcpc_ep);
OSMO_ASSERT(ep);
+ for (i = 0; i < ARRAY_SIZE(ep->ci); i++) {
+ ep->ci[i].verb_info = mgcp_conn_peer_alloc(ep);
+ ep->ci[i].rtp_info = mgcp_conn_peer_alloc(ep);
+ }
+
*ep = (struct osmo_mgcpc_ep){
.mgcp_client = mgcp_client,
.fi = fi,
@@ -460,7 +466,7 @@
continue;
if (other_ci->sent)
continue;
- OSMO_STRLCPY_ARRAY(other_ci->verb_info.endpoint, ep->endpoint);
+ OSMO_STRLCPY_ARRAY(other_ci->verb_info->endpoint, ep->endpoint);
}
return 0;
}
@@ -496,7 +502,7 @@
* may provide new one after remote end params changed */
if (rtp_info) {
ci->got_port_info = true;
- ci->rtp_info = *rtp_info;
+ *ci->rtp_info = *rtp_info;
}
break;
@@ -506,7 +512,7 @@
LOG_CI(ci, LOGL_DEBUG, "received successful response to %s: RTP=%s%s\n",
osmo_mgcp_verb_name(ci->verb),
- mgcp_conn_peer_name(ci->got_port_info? &ci->rtp_info : NULL),
+ mgcp_conn_peer_name(ci->got_port_info? ci->rtp_info : NULL),
ci->notify.fi ? "" : " (not sending a notification)");
if (ci->notify.fi)
@@ -524,7 +530,7 @@
return NULL;
if (!ci->got_port_info)
return NULL;
- return &ci->rtp_info;
+ return ci->rtp_info;
}
/*! Return the MGW's remote RTP port information for this connection, i.e. the remote RTP port that the MGW is sending
@@ -534,7 +540,7 @@
ci = osmo_mgcpc_ep_check_ci((struct osmo_mgcpc_ep_ci*)ci);
if (!ci)
return NULL;
- return &ci->verb_info;
+ return ci->verb_info;
}
/*! Return the MGW's RTP port information for this connection, as returned by the last CRCX/MDCX OK message. */
@@ -665,15 +671,15 @@
LOG_CI_VERB(ci, LOGL_DEBUG, "notify=%s\n", osmo_fsm_inst_name(ci->notify.fi));
if (verb_info)
- ci->verb_info = *verb_info;
+ *ci->verb_info = *verb_info;
if (ep->endpoint[0]) {
- if (ci->verb_info.endpoint[0] && strcmp(ci->verb_info.endpoint, ep->endpoint))
+ if (ci->verb_info->endpoint[0] && strcmp(ci->verb_info->endpoint, ep->endpoint))
LOG_CI(ci, LOGL_ERROR,
"Warning: Requested %s on endpoint %s, but this CI is on endpoint %s."
" Using the proper endpoint instead.\n",
- osmo_mgcp_verb_name(verb), ci->verb_info.endpoint, ep->endpoint);
- osmo_strlcpy(ci->verb_info.endpoint, ep->endpoint, sizeof(ci->verb_info.endpoint));
+ osmo_mgcp_verb_name(verb), ci->verb_info->endpoint, ep->endpoint);
+ osmo_strlcpy(ci->verb_info->endpoint, ep->endpoint, sizeof(ci->verb_info->endpoint));
}
switch (ci->verb) {
@@ -764,7 +770,7 @@
LOG_CI_VERB(ci, LOGL_DEBUG, "Sending\n");
ci->mgcp_client_fi = mgcp_conn_create(ep->mgcp_client, ep->fi,
CI_EV_FAILURE(ci), CI_EV_SUCCESS(ci),
- &ci->verb_info);
+ ci->verb_info);
ci->sent = true;
if (!ci->mgcp_client_fi){
LOG_CI_VERB(ci, LOGL_ERROR, "Cannot send\n");
@@ -777,7 +783,7 @@
case MGCP_VERB_MDCX:
OSMO_ASSERT(ci->mgcp_client_fi);
LOG_CI_VERB(ci, LOGL_DEBUG, "Sending\n");
- rc = mgcp_conn_modify(ci->mgcp_client_fi, CI_EV_SUCCESS(ci), &ci->verb_info);
+ rc = mgcp_conn_modify(ci->mgcp_client_fi, CI_EV_SUCCESS(ci), ci->verb_info);
ci->sent = true;
if (rc) {
LOG_CI_VERB(ci, LOGL_ERROR, "Cannot send (rc=%d %s)\n", rc, strerror(-rc));
diff --git a/src/libosmo-mgcp-client/mgcp_client_fsm.c b/src/libosmo-mgcp-client/mgcp_client_fsm.c
index 1d50b3f..01a18cc 100644
--- a/src/libosmo-mgcp-client/mgcp_client_fsm.c
+++ b/src/libosmo-mgcp-client/mgcp_client_fsm.c
@@ -621,6 +621,17 @@
.log_subsys = DLMGCP,
};
+/*! allocate struct to hold the description of an MGCP connection peer.
+ * \param[in] ctx talloc context.
+ * \returns newly-allocated and initialized struct mgcp_conn_peer. */
+struct mgcp_conn_peer *mgcp_conn_peer_alloc(void *ctx)
+{
+ struct mgcp_conn_peer *peer;
+ peer = talloc_zero(ctx, struct mgcp_conn_peer);
+ OSMO_ASSERT(peer);
+ return peer;
+}
+
/*! allocate FSM, and create a new connection on the MGW.
* \param[in] mgcp MGCP client descriptor.
* \param[in] parent_fi Parent FSM instance.
To view, visit change 34404. To unsubscribe, or for help writing mail filters, visit settings.