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/.
laforge gerrit-no-reply at lists.osmocom.orglaforge 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 <lynxis at fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann 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/20201220/49ff084f/attachment.htm>