dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email )
Change subject: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically ......................................................................
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.