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.

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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I523d0fcb020f7d46323c497a4be9ee00d5f242ba
Gerrit-Change-Number: 34404
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <[email protected]>
Gerrit-MessageType: newchange

Reply via email to