[MERGED] osmo-msc[master]: a_iface: fix memory leaks

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Mon Sep 11 12:44:52 UTC 2017


Neels Hofmeyr has submitted this change and it was merged.

Change subject: a_iface: fix memory leaks
......................................................................


a_iface: fix memory leaks

Fix multiple memory leaske in A/BSSMAP code

Change-Id: I90703c96e6a266a1cfa60b184139375aeb9ae32d
---
M include/osmocom/msc/a_iface.h
M src/libmsc/a_iface.c
M src/libmsc/a_iface_bssap.c
M src/libmsc/msc_ifaces.c
4 files changed, 21 insertions(+), 10 deletions(-)

Approvals:
  Neels Hofmeyr: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/msc/a_iface.h b/include/osmocom/msc/a_iface.h
index a49ede2..32003eb 100644
--- a/include/osmocom/msc/a_iface.h
+++ b/include/osmocom/msc/a_iface.h
@@ -51,7 +51,7 @@
 /* Initalize A interface connection between to MSC and BSC */
 int a_init(struct osmo_sccp_instance *sccp, struct gsm_network *network);
 
-/* Send DTAP message via A-interface */
+/* Send DTAP message via A-interface, take ownership of msg */
 int a_iface_tx_dtap(struct msgb *msg);
 
 /* Send Cipher mode command via A-interface */
diff --git a/src/libmsc/a_iface.c b/src/libmsc/a_iface.c
index 3d2013e..8db6173 100644
--- a/src/libmsc/a_iface.c
+++ b/src/libmsc/a_iface.c
@@ -127,7 +127,7 @@
 	return NULL;
 }
 
-/* Send DTAP message via A-interface */
+/* Send DTAP message via A-interface, take ownership of msg */
 int a_iface_tx_dtap(struct msgb *msg)
 {
 	struct gsm_subscriber_connection *conn;
@@ -144,6 +144,11 @@
 
 	msg->l3h = msg->data;
 	msg_resp = gsm0808_create_dtap(msg, link_id);
+
+	/* gsm0808_create_dtap() has copied the data to msg_resp,
+	 * so msg has served its purpose now */
+	msgb_free(msg);
+
 	if (!msg_resp) {
 		LOGP(DMSC, LOGL_ERROR, "Unable to generate BSSMAP DTAP message!\n");
 		return -EINVAL;
@@ -151,6 +156,7 @@
 		LOGP(DMSC, LOGL_DEBUG, "Massage will be sent as BSSMAP DTAP message!\n");
 
 	LOGP(DMSC, LOGL_DEBUG, "N-DATA.req(%u, %s)\n", conn->a.conn_id, osmo_hexdump(msg_resp->data, msg_resp->len));
+	/* osmo_sccp_tx_data_msg() takes ownership of msg_resp */
 	return osmo_sccp_tx_data_msg(conn->a.scu, conn->a.conn_id, msg_resp);
 }
 
diff --git a/src/libmsc/a_iface_bssap.c b/src/libmsc/a_iface_bssap.c
index 34d03e3..3d5755a 100644
--- a/src/libmsc/a_iface_bssap.c
+++ b/src/libmsc/a_iface_bssap.c
@@ -150,6 +150,7 @@
 
 	if (msgb_l3len(msg) < 1) {
 		LOGP(DMSC, LOGL_NOTICE, "Error: No data received -- discarding message!\n");
+		msgb_free(msg);
 		return;
 	}
 
@@ -327,9 +328,9 @@
 	conn = subscr_conn_allocate_a(a_conn_info, network, lac, scu, a_conn_info->conn_id);
 
 	/* Handover location update to the MSC code */
-	/* msc_compl_l3() takes ownership of dtap_msg
-	 * message buffer */
 	rc = msc_compl_l3(conn, msg, 0);
+	msgb_free(msg);
+
 	if (rc == MSC_CONN_ACCEPT) {
 		LOGP(DMSC, LOGL_NOTICE, "User has been accepted by MSC.\n");
 		return 0;
@@ -423,9 +424,9 @@
 		msg = NULL;
 	}
 
-	/* Hand over cipher mode complete message to the MSC,
-	 * msc_cipher_mode_compl() takes ownership for msg */
+	/* Hand over cipher mode complete message to the MSC */
 	msc_cipher_mode_compl(conn, msg, alg_id);
+	msgb_free(msg);
 
 	return 0;
 fail:
@@ -677,9 +678,9 @@
 	/* msc_dtap expects the dtap payload in l3h */
 	msg->l3h = msg->l2h + 3;
 
-	/* Forward dtap payload into the msc,
-	 * msc_dtap() takes ownership for msg */
+	/* Forward dtap payload into the msc */
 	msc_dtap(conn, conn->a.conn_id, msg);
+	msgb_free(msg);
 
 	return 0;
 }
@@ -696,6 +697,7 @@
 	if (msgb_l2len(msg) < sizeof(struct bssmap_header)) {
 		LOGP(DMSC, LOGL_NOTICE, "The header is too short -- discarding message!\n");
 		msgb_free(msg);
+		return -EINVAL;
 	}
 
 	switch (msg->l2h[0]) {
diff --git a/src/libmsc/msc_ifaces.c b/src/libmsc/msc_ifaces.c
index 7a04153..f54426e 100644
--- a/src/libmsc/msc_ifaces.c
+++ b/src/libmsc/msc_ifaces.c
@@ -44,10 +44,12 @@
 
 static int msc_tx(struct gsm_subscriber_connection *conn, struct msgb *msg)
 {
-	if (!conn)
-		return -EINVAL;
 	if (!msg)
 		return -EINVAL;
+	if (!conn) {
+		msgb_free(msg);
+		return -EINVAL;
+	}
 
 	DEBUGP(DMSC, "msc_tx %u bytes to %s via %s\n",
 	       msg->len, vlr_subscr_name(conn->vsub),
@@ -65,6 +67,7 @@
 		LOGP(DMSC, LOGL_ERROR,
 		     "msc_tx(): conn->via_ran invalid (%d)\n",
 		     conn->via_ran);
+		msgb_free(msg);
 		return -1;
 	}
 }

-- 
To view, visit https://gerrit.osmocom.org/3889
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I90703c96e6a266a1cfa60b184139375aeb9ae32d
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list