Change in libosmocore[master]: gprs_ns2: fix memory leaks when receiving SNS or invalid packets

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/23484 )

Change subject: gprs_ns2: fix memory leaks when receiving SNS or invalid packets
......................................................................

gprs_ns2: fix memory leaks when receiving SNS or invalid packets

Change-Id: I8834d3f092e6cbe4f527e95e1eebd8133a386207
---
M src/gb/gprs_ns2.c
M src/gb/gprs_ns2_sns.c
M src/gb/gprs_ns2_vc_fsm.c
3 files changed, 25 insertions(+), 21 deletions(-)

Approvals:
  laforge: Looks good to me, approved
  daniel: Looks good to me, approved
  dexter: Looks good to me, but someone else must approve
  pespin: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c
index 507a5ac..1148d6f 100644
--- a/src/gb/gprs_ns2.c
+++ b/src/gb/gprs_ns2.c
@@ -1243,7 +1243,7 @@
 
 /*! Bottom-side entry-point for received NS PDU from the driver/bind
  * \param[in] nsvc NS-VC for which the message was received
- * \param msg the received message. Ownership is trasnferred, caller must not free it!
+ * \param msg the received message. Ownership is transferred, caller must not free it!
  * \return 0 on success; negative on error */
 int ns2_recv_vc(struct gprs_ns2_vc *nsvc,
 		struct msgb *msg)
@@ -1258,8 +1258,10 @@
 	rate_ctr_inc(&nsvc->ctrg->ctr[NS_CTR_PKTS_IN]);
 	rate_ctr_add(&nsvc->ctrg->ctr[NS_CTR_BYTES_IN], msg->len);
 
-	if (msg->len < sizeof(struct gprs_ns_hdr))
-		return -EINVAL;
+	if (msg->len < sizeof(struct gprs_ns_hdr)) {
+		rc = -EINVAL;
+		goto freemsg;
+	}
 
 	if (nsh->pdu_type != NS_PDUT_UNITDATA)
 		LOG_NS_RX_SIGNAL(nsvc, nsh->pdu_type);
@@ -1273,11 +1275,10 @@
 				   msgb_l2len(msg) - sizeof(*nsh)-1, 0, 0);
 		if (rc < 0) {
 			LOGP(DLNS, LOGL_NOTICE, "Error during TLV Parse in %s\n", msgb_hexdump(msg));
-			return rc;
+			goto freemsg;
 		}
 		/* All sub-network service related message types */
-		rc = ns2_sns_rx(nsvc, msg, &tp);
-		break;
+		return ns2_sns_rx(nsvc, msg, &tp);
 	case SNS_PDUT_ACK:
 	case SNS_PDUT_ADD:
 	case SNS_PDUT_CHANGE_WEIGHT:
@@ -1287,14 +1288,13 @@
 				   msgb_l2len(msg) - sizeof(*nsh)-5, 0, 0);
 		if (rc < 0) {
 			LOGP(DLNS, LOGL_NOTICE, "Error during TLV Parse in %s\n", msgb_hexdump(msg));
-			return rc;
+			goto freemsg;
 		}
 		tp.lv[NS_IE_NSEI].val = nsh->data+2;
 		tp.lv[NS_IE_NSEI].len = 2;
 		tp.lv[NS_IE_TRANS_ID].val = nsh->data+4;
 		tp.lv[NS_IE_TRANS_ID].len = 1;
-		rc = ns2_sns_rx(nsvc, msg, &tp);
-		break;
+		return ns2_sns_rx(nsvc, msg, &tp);
 	case SNS_PDUT_CONFIG_ACK:
 	case SNS_PDUT_SIZE:
 	case SNS_PDUT_SIZE_ACK:
@@ -1302,15 +1302,12 @@
 				   msgb_l2len(msg) - sizeof(*nsh), 0, 0);
 		if (rc < 0) {
 			LOGP(DLNS, LOGL_NOTICE, "Error during TLV Parse in %s\n", msgb_hexdump(msg));
-			return rc;
+			goto freemsg;
 		}
 		/* All sub-network service related message types */
-		rc = ns2_sns_rx(nsvc, msg, &tp);
-		break;
-
+		return ns2_sns_rx(nsvc, msg, &tp);
 	case NS_PDUT_UNITDATA:
-		rc = ns2_vc_rx(nsvc, msg, &tp);
-		break;
+		return ns2_vc_rx(nsvc, msg, &tp);
 	default:
 		rc = ns2_tlv_parse(&tp, nsh->data,
 				   msgb_l2len(msg) - sizeof(*nsh), 0, 0);
@@ -1320,9 +1317,10 @@
 				ns2_tx_status(nsvc, NS_CAUSE_PROTO_ERR_UNSPEC, 0, msg);
 			return rc;
 		}
-		rc = ns2_vc_rx(nsvc, msg, &tp);
-		break;
+		return ns2_vc_rx(nsvc, msg, &tp);
 	}
+freemsg:
+	msgb_free(msg);
 
 	return rc;
 }
diff --git a/src/gb/gprs_ns2_sns.c b/src/gb/gprs_ns2_sns.c
index 144ab21..b8c44f1 100644
--- a/src/gb/gprs_ns2_sns.c
+++ b/src/gb/gprs_ns2_sns.c
@@ -1614,11 +1614,13 @@
 	uint16_t nsei = nsvc->nse->nsei;
 	struct ns2_sns_state *gss;
 	struct osmo_fsm_inst *fi;
+	int rc = 0;
 
 	if (!nse->bss_sns_fi) {
 		LOGNSVC(nsvc, LOGL_NOTICE, "Rx %s for NS Instance that has no SNS!\n",
 			get_value_string(gprs_ns_pdu_strings, nsh->pdu_type));
-		return -EINVAL;
+		rc = -EINVAL;
+		goto out;
 	}
 
 	/* FIXME: how to resolve SNS FSM Instance by NSEI (SGSN)? */
@@ -1661,10 +1663,13 @@
 	default:
 		LOGPFSML(fi, LOGL_ERROR, "NSEI=%u Rx unknown SNS PDU type %s\n", nsei,
 			 get_value_string(gprs_ns_pdu_strings, nsh->pdu_type));
-		return -EINVAL;
+		rc = -EINVAL;
 	}
 
-	return 0;
+out:
+	msgb_free(msg);
+
+	return rc;
 }
 
 #include <osmocom/vty/vty.h>
diff --git a/src/gb/gprs_ns2_vc_fsm.c b/src/gb/gprs_ns2_vc_fsm.c
index ad8d4db..a8cb570 100644
--- a/src/gb/gprs_ns2_vc_fsm.c
+++ b/src/gb/gprs_ns2_vc_fsm.c
@@ -883,7 +883,8 @@
 	default:
 		LOGPFSML(fi, LOGL_ERROR, "NSEI=%u Rx unknown NS PDU type %s\n", nsvc->nse->nsei,
 			 get_value_string(gprs_ns_pdu_strings, nsh->pdu_type));
-		return -EINVAL;
+		rc = -EINVAL;
+		break;
 	}
 
 out:

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/23484
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I8834d3f092e6cbe4f527e95e1eebd8133a386207
Gerrit-Change-Number: 23484
Gerrit-PatchSet: 2
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210329/0817d32e/attachment.htm>