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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
