[MERGED] libosmo-sccp[master]: sigtran: fix various memory leaks (msgb and xua_msg)

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/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Mon Apr 10 11:27:01 UTC 2017


Harald Welte has submitted this change and it was merged.

Change subject: sigtran: fix various memory leaks (msgb and xua_msg)
......................................................................


sigtran: fix various memory leaks (msgb and xua_msg)

The general rule for 'struct xua_msg' is now that it is free'd by the
function that also allocates it in the first place.  Any downstream
consumer of the xua_msg may interpret it, but not hold any references or
free() it.

Change-Id: I708505d129da5824c69b31a13a9c93201929bada
---
M examples/sccp_test_server.c
M examples/sccp_test_vty.c
M src/m3ua.c
M src/osmo_ss7_hmrt.c
M src/sccp_sclc.c
M src/sccp_scoc.c
M src/sccp_scrc.c
M src/sccp_user.c
M src/sua.c
9 files changed, 33 insertions(+), 17 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/examples/sccp_test_server.c b/examples/sccp_test_server.c
index c3c658f..71bed96 100644
--- a/examples/sccp_test_server.c
+++ b/examples/sccp_test_server.c
@@ -34,6 +34,7 @@
 			oph->primitive, oph->operation);
 		break;
 	}
+	msgb_free(oph->msg);
 	return 0;
 }
 
@@ -71,6 +72,7 @@
 			oph->primitive, oph->operation);
 		break;
 	}
+	msgb_free(oph->msg);
 	return 0;
 }
 
@@ -102,6 +104,7 @@
 			oph->primitive, oph->operation);
 		break;
 	}
+	msgb_free(oph->msg);
 	return 0;
 }
 
diff --git a/examples/sccp_test_vty.c b/examples/sccp_test_vty.c
index 1134d57..ddfbb27 100644
--- a/examples/sccp_test_vty.c
+++ b/examples/sccp_test_vty.c
@@ -125,6 +125,7 @@
 	default:
 		break;
 	}
+	msgb_free(oph->msg);
 	return 0;
 }
 
diff --git a/src/m3ua.c b/src/m3ua.c
index 7437b6e..4f20c6e 100644
--- a/src/m3ua.c
+++ b/src/m3ua.c
@@ -420,13 +420,12 @@
  * Transmitting M3UA messsages to SCTP
  ***********************************************************************/
 
+/* transmit given xua_msg via given ASP. callee takes xua ownership */
 static int m3ua_tx_xua_asp(struct osmo_ss7_asp *asp, struct xua_msg *xua)
 {
 	struct msgb *msg = xua_to_msg(M3UA_VERSION, xua);
 
 	OSMO_ASSERT(asp->cfg.proto == OSMO_SS7_ASP_PROT_M3UA);
-
-	xua_msg_free(xua);
 
 	if (!msg) {
 		LOGP(DLM3UA, LOGL_ERROR, "Error encoding M3UA Msg\n");
@@ -461,7 +460,6 @@
 	}
 	if (!asp) {
 		LOGP(DLM3UA, LOGL_ERROR, "No ASP entroy in AS, dropping message\n");
-		xua_msg_free(xua);
 		return -ENODEV;
 	}
 
diff --git a/src/osmo_ss7_hmrt.c b/src/osmo_ss7_hmrt.c
index bc2b8e5..105a542 100644
--- a/src/osmo_ss7_hmrt.c
+++ b/src/osmo_ss7_hmrt.c
@@ -195,6 +195,7 @@
 				struct osmo_mtp_prim *omp)
 {
 	struct xua_msg *xua;
+	int rc;
 
 	OSMO_ASSERT(omp->oph.sap == MTP_SAP_USER);
 
@@ -210,10 +211,15 @@
 		 * is a very useful feature (aka "loopback device" in
 		 * IPv4). So we call m3ua_hmdc_rx_from_l2() just like
 		 * the MTP-TRANSFER had been received from L2. */
-		return m3ua_hmdc_rx_from_l2(inst, xua);
+		rc = m3ua_hmdc_rx_from_l2(inst, xua);
+		xua_msg_free(xua);
+		break;
 	default:
 		LOGP(DLSS7, LOGL_ERROR, "Ignoring unknown primitive %u:%u\n",
 			omp->oph.primitive, omp->oph.operation);
-		return -1;
+		rc = -1;
 	}
+
+	msgb_free(omp->oph.msg);
+	return rc;
 }
diff --git a/src/sccp_sclc.c b/src/sccp_sclc.c
index dae2c36..7cdfac5 100644
--- a/src/sccp_sclc.c
+++ b/src/sccp_sclc.c
@@ -102,12 +102,15 @@
 				   struct osmo_scu_prim *prim, int msg_type)
 {
 	struct xua_msg *xua;
+	int rc;
 
 	xua = xua_gen_msg_cl(event, prim, msg_type);
 	if (!xua)
 		return -1;
 
-	return sccp_scrc_rx_sclc_msg(scu->inst, xua);
+	rc = sccp_scrc_rx_sclc_msg(scu->inst, xua);
+	xua_msg_free(xua);
+	return rc;
 }
 
 /*! \brief Main entrance function for primitives from SCCP User
@@ -327,8 +330,8 @@
 			/* local originator: send N-NOTICE to user */
 			/* TODO: N-NOTICE.ind SCLC -> SCU */
 			sclc_rx_cldr(inst, xua_out);
-			xua_msg_free(xua_out);
 		}
+		xua_msg_free(xua_out);
 		break;
 	case SUA_CL_CLDR:
 		/* do nothing */
diff --git a/src/sccp_scoc.c b/src/sccp_scoc.c
index f881872..2585c9f 100644
--- a/src/sccp_scoc.c
+++ b/src/sccp_scoc.c
@@ -459,7 +459,9 @@
 	/* amend this with point code information; The SUA RELRE
 	 * includes neither called nor calling party address! */
 	xua->mtp.dpc = conn->remote_pc;
-	return sccp_scrc_rx_scoc_conn_msg(conn->inst, xua);
+	sccp_scrc_rx_scoc_conn_msg(conn->inst, xua);
+	xua_msg_free(xua);
+	return 0;
 }
 
 /* generate a 'struct xua_msg' of requested type from connection +
@@ -593,7 +595,9 @@
 	/* amend this with point code information; Many CO msgs
 	 * includes neither called nor calling party address! */
 	xua->mtp.dpc = conn->remote_pc;
-	return sccp_scrc_rx_scoc_conn_msg(conn->inst, xua);
+	sccp_scrc_rx_scoc_conn_msg(conn->inst, xua);
+	xua_msg_free(xua);
+	return 0;
 }
 
 /* allocate a SCU primitive to be sent to the user */
diff --git a/src/sccp_scrc.c b/src/sccp_scrc.c
index 0ab25cb..f9df075 100644
--- a/src/sccp_scrc.c
+++ b/src/sccp_scrc.c
@@ -384,8 +384,9 @@
 		return scrc_node_2(inst, xua, &called);
 
 	/* TOOD: Coupling performed (not supported) */
-	if (0)
+	if (0) {
 		return scrc_node_2(inst, xua, &called);
+	}
 
 	return scrc_local_out_common(inst, xua, &called);
 }
@@ -412,6 +413,7 @@
 			return scrc_node_12(inst, xua, &called);
 		}
 	}
+
 	return scrc_local_out_common(inst, xua, &called);
 }
 
@@ -450,8 +452,7 @@
 		if (hop_counter <= 1) {
 			/* Error: hop-counter violation */
 			/* node 4 */
-			return scrc_node_4(inst, xua,
-					   SCCP_RETURN_CAUSE_HOP_COUNTER_VIOLATION);
+			return scrc_node_4(inst, xua, SCCP_RETURN_CAUSE_HOP_COUNTER_VIOLATION);
 		}
 		/* Decrement hop-counter */
 		hop_counter--;
diff --git a/src/sccp_user.c b/src/sccp_user.c
index bc03f4e..297d939 100644
--- a/src/sccp_user.c
+++ b/src/sccp_user.c
@@ -157,6 +157,7 @@
 	struct osmo_sccp_instance *inst = ctx;
 	struct osmo_mtp_prim *omp = (struct osmo_mtp_prim *)oph;
 	struct xua_msg *xua;
+	int rc;
 
 	OSMO_ASSERT(oph->sap == MTP_SAP_USER);
 
@@ -166,12 +167,14 @@
 		xua = osmo_sccp_to_xua(oph->msg);
 		xua->mtp = omp->u.transfer;
 		/* hand this primitive into SCCP via the SCRC code */
-		return scrc_rx_mtp_xfer_ind_xua(inst, xua);
+		rc = scrc_rx_mtp_xfer_ind_xua(inst, xua);
 	default:
 		LOGP(DLSCCP, LOGL_ERROR, "Unknown primitive %u:%u receivd\n",
 			oph->primitive, oph->operation);
-		return -1;
+		rc = -1;
 	}
+	msgb_free(oph->msg);
+	return rc;
 }
 
 static LLIST_HEAD(sccp_instances);
diff --git a/src/sua.c b/src/sua.c
index b0d5f8a..2f8acf2 100644
--- a/src/sua.c
+++ b/src/sua.c
@@ -258,8 +258,6 @@
 
 	OSMO_ASSERT(asp->cfg.proto == OSMO_SS7_ASP_PROT_SUA);
 
-	xua_msg_free(xua);
-
 	if (!msg) {
 		LOGPASP(asp, DLSUA, LOGL_ERROR, "Error encoding SUA Msg\n");
 		return -1;
@@ -290,7 +288,6 @@
 	}
 	if (!asp) {
 		LOGP(DLSUA, LOGL_ERROR, "No ASP in AS, dropping message\n");
-		xua_msg_free(xua);
 		return -ENODEV;
 	}
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I708505d129da5824c69b31a13a9c93201929bada
Gerrit-PatchSet: 5
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list