a_iface: fix memory leaks

Fix multiple memory leaske in A/BSSMAP code

Change-Id: I90703c96e6a266a1cfa60b184139375aeb9ae32d

4 files changed, 21 insertions, 10 deletions

diff --git a/include/osmocom/msc/a_iface.h b/include/osmocom/msc/a_iface.h
index a49ede25e..32003ebdb 100644
--- a/include/osmocom/msc/a_iface.h
+++ b/include/osmocom/msc/a_iface.h
@@ -51,7 +51,7 @@ struct bsc_context {
 /* 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 3d2013e62..8db6173e3 100644
--- a/src/libmsc/a_iface.c
+++ b/src/libmsc/a_iface.c
@@ -127,7 +127,7 @@ static struct a_reset_ctx *get_reset_ctx_by_sccp_addr(const struct osmo_sccp_add
 	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 @@ int a_iface_tx_dtap(struct msgb *msg)
 
 	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 @@ int a_iface_tx_dtap(struct msgb *msg)
 		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 34d03e300..3d5755a4c 100644
--- a/src/libmsc/a_iface_bssap.c
+++ b/src/libmsc/a_iface_bssap.c
@@ -150,6 +150,7 @@ static void bssmap_rcvmsg_udt(struct osmo_sccp_user *scu, const struct a_conn_in
 
 	if (msgb_l3len(msg) < 1) {
 		LOGP(DMSC, LOGL_NOTICE, "Error: No data received -- discarding message!\n");
+		msgb_free(msg);
 		return;
 	}
 
@@ -327,9 +328,9 @@ static int bssmap_rx_l3_compl(struct osmo_sccp_user *scu, const struct a_conn_in
 	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 @@ static int bssmap_rx_ciph_compl(const struct osmo_sccp_user *scu, const struct a
 		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 @@ static int rx_dtap(const struct osmo_sccp_user *scu, const struct a_conn_info *a
 	/* 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 @@ int sccp_rx_dt(struct osmo_sccp_user *scu, const struct a_conn_info *a_conn_info
 	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 7a04153a6..f54426e18 100644
--- a/src/libmsc/msc_ifaces.c
+++ b/src/libmsc/msc_ifaces.c
@@ -44,10 +44,12 @@ extern struct msgb *ranap_new_msg_rab_assign_voice(uint8_t rab_id,
 
 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 @@ static int msc_tx(struct gsm_subscriber_connection *conn, struct msgb *msg)
 		LOGP(DMSC, LOGL_ERROR,
 		     "msc_tx(): conn->via_ran invalid (%d)\n",
 		     conn->via_ran);
+		msgb_free(msg);
 		return -1;
 	}
 }