laforge has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/21724 )
Change subject: gprs_ns2: improve handling of TLV errors on new nsvcs
......................................................................
gprs_ns2: improve handling of TLV errors on new nsvcs
The specification says the PDU should be ignored if the PDU type is
unknown.
Change-Id: I2992d06b37ed122b7ff315d4852e86acc936800b
---
M src/gb/gprs_ns2.c
1 file changed, 20 insertions(+), 8 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
daniel: Looks good to me, but someone else must approve
diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c
index 9d5a97c..49606d8 100644
--- a/src/gb/gprs_ns2.c
+++ b/src/gb/gprs_ns2.c
@@ -793,19 +793,17 @@
uint16_t nsvci;
uint16_t nsei;
- int rc;
+ int rc, tlv;
if (msg->len < sizeof(struct gprs_ns_hdr))
return GPRS_NS2_CS_ERROR;
- rc = ns2_tlv_parse(&tp, nsh->data,
+ /* parse the tlv early to allow reject status msg to
+ * work with valid tp.
+ * Ignore the return code until the pdu type is parsed because
+ * an unknown pdu type should be ignored */
+ tlv = ns2_tlv_parse(&tp, nsh->data,
msgb_l2len(msg) - sizeof(*nsh), 0, 0);
- if (rc < 0) {
- LOGP(DLNS, LOGL_ERROR, "Rx NS RESET Error %d during "
- "TLV Parse\n", rc);
- /* TODO: send invalid message back */
- return GPRS_NS2_CS_REJECTED;
- }
switch (nsh->pdu_type) {
case NS_PDUT_STATUS:
@@ -849,6 +847,20 @@
return GPRS_NS2_CS_REJECTED;
}
+ if (tlv < 0) {
+ /* TODO: correct behaviour would checking what's wrong.
+ * If it's an essential TLV for the PDU return
NS_CAUSE_INVAL_ESSENT_IE.
+ * Otherwise ignore the non-essential TLV. */
+ LOGP(DLNS, LOGL_ERROR, "Rx NS RESET Error %d during "
+ "TLV Parse\n", tlv);
+ rc = reject_status_msg(msg, &tp, reject,
NS_CAUSE_PROTO_ERR_UNSPEC);
+ if (rc < 0) {
+ LOGP(DLNS, LOGL_ERROR, "Failed to generate reject
message (%d)\n", rc);
+ return rc;
+ }
+ return GPRS_NS2_CS_REJECTED;
+ }
+
if (!TLVP_PRES_LEN(&tp, NS_IE_CAUSE, 1) ||
!TLVP_PRES_LEN(&tp, NS_IE_VCI, 2) || !TLVP_PRES_LEN(&tp,
NS_IE_NSEI, 2)) {
LOGP(DLNS, LOGL_ERROR, "NS RESET Missing mandatory IE\n");
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21724
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2992d06b37ed122b7ff315d4852e86acc936800b
Gerrit-Change-Number: 21724
Gerrit-PatchSet: 3
Gerrit-Owner: lynxis lazus <lyn...@fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillm...@sysmocom.de>
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-MessageType: merged
