From db142dc59dc8d79d8ee608c9165bc865d240b97d Mon Sep 17 00:00:00 2001 From: Philipp Date: Thu, 22 Dec 2016 14:15:20 +0100 Subject: sndcp: Allow empty SNDCP-XID indications In some rare cases the modem might send a xid indication that does not contain anything except the version number field. The sgsn ignors such SNDCP-XID indications by stripping the entire field from the response. We found a modem in the wild that started to act problematic when the empty SNDCP-XID was missing in the response. This patch changes the XID negotiation behaviour in a way that if a modem should send empty SNDCP-XID indications, the reply will also contain an empty SNDCP-XID indication. Apart from that the SNDCP-XID version number is now parsed and echoed in the response. This ensures that we always reply with the version number that the modem expects. (The version was 0 in all cases we observed so far) Change-Id: I097a770cb4907418f53e620a051ebb8cd110c5f2 Related: OS#1794 --- openbsc/src/gprs/gprs_sndcp.c | 25 ++++++++---------- openbsc/src/gprs/gprs_sndcp_xid.c | 54 ++++++++++++++++++++++++++------------- 2 files changed, 47 insertions(+), 32 deletions(-) (limited to 'openbsc/src') diff --git a/openbsc/src/gprs/gprs_sndcp.c b/openbsc/src/gprs/gprs_sndcp.c index 1cbeede09..60455b518 100644 --- a/openbsc/src/gprs/gprs_sndcp.c +++ b/openbsc/src/gprs/gprs_sndcp.c @@ -960,8 +960,13 @@ static int gprs_llc_gen_sndcp_xid(uint8_t *bytes, int bytes_len, uint8_t nsapi) llist_add(&v42bis_comp_field.list, &comp_fields); } + /* Do not attempt to compile anything if there is no data in the list */ + if (llist_empty(&comp_fields)) + return 0; + /* Compile bytestream */ - return gprs_sndcp_compile_xid(bytes, bytes_len, &comp_fields); + return gprs_sndcp_compile_xid(bytes, bytes_len, &comp_fields, + DEFAULT_SNDCP_VERSION); } /* Set of SNDCP-XID bnegotiation (See also: TS 144 065, @@ -1106,6 +1111,7 @@ int sndcp_sn_xid_ind(struct gprs_llc_xid_field *xid_field_indication, int rc; int compclass; + int version; struct llist_head *comp_fields; struct gprs_sndcp_comp_field *comp_field; @@ -1115,22 +1121,13 @@ int sndcp_sn_xid_ind(struct gprs_llc_xid_field *xid_field_indication, OSMO_ASSERT(lle); /* Parse SNDCP-CID XID-Field */ - comp_fields = gprs_sndcp_parse_xid(lle->llme, + comp_fields = gprs_sndcp_parse_xid(&version, lle->llme, xid_field_indication->data, xid_field_indication->data_len, NULL); if (!comp_fields) return -EINVAL; - /* Don't bother with empty indications */ - if (llist_empty(comp_fields)) { - xid_field_response->data = NULL; - xid_field_response->data_len = 0; - DEBUGP(DSNDCP, - "SNDCP-XID indication did not contain any parameters!\n"); - return 0; - } - /* Handle compression entites */ DEBUGP(DSNDCP, "SNDCP-XID-IND (ms):\n"); gprs_sndcp_dump_comp_fields(comp_fields, LOGL_DEBUG); @@ -1168,7 +1165,7 @@ int sndcp_sn_xid_ind(struct gprs_llc_xid_field *xid_field_indication, /* Compile modified SNDCP-XID bytes */ rc = gprs_sndcp_compile_xid(xid_field_response->data, xid_field_indication->data_len, - comp_fields); + comp_fields, 0); if (rc > 0) xid_field_response->data_len = rc; @@ -1210,7 +1207,7 @@ int sndcp_sn_xid_conf(struct gprs_llc_xid_field *xid_field_conf, OSMO_ASSERT(xid_field_request); /* Parse SNDCP-CID XID-Field */ - comp_fields_req = gprs_sndcp_parse_xid(lle->llme, + comp_fields_req = gprs_sndcp_parse_xid(NULL, lle->llme, xid_field_request->data, xid_field_request->data_len, NULL); @@ -1221,7 +1218,7 @@ int sndcp_sn_xid_conf(struct gprs_llc_xid_field *xid_field_conf, gprs_sndcp_dump_comp_fields(comp_fields_req, LOGL_DEBUG); /* Parse SNDCP-CID XID-Field */ - comp_fields_conf = gprs_sndcp_parse_xid(lle->llme, + comp_fields_conf = gprs_sndcp_parse_xid(NULL, lle->llme, xid_field_conf->data, xid_field_conf->data_len, comp_fields_req); diff --git a/openbsc/src/gprs/gprs_sndcp_xid.c b/openbsc/src/gprs/gprs_sndcp_xid.c index bb43eab68..dfea5febc 100644 --- a/openbsc/src/gprs/gprs_sndcp_xid.c +++ b/openbsc/src/gprs/gprs_sndcp_xid.c @@ -549,26 +549,29 @@ static int gprs_sndcp_pack_fields(const struct llist_head *comp_fields, /* Transform a list with compression fields into an SNDCP-XID message (dst) */ int gprs_sndcp_compile_xid(uint8_t *dst, unsigned int dst_maxlen, - const struct llist_head *comp_fields) + const struct llist_head *comp_fields, int version) { int rc; int byte_counter = 0; uint8_t comp_bytes[512]; - uint8_t xid_version_number[1] = { CURRENT_SNDCP_VERSION }; + uint8_t xid_version_number[1]; OSMO_ASSERT(comp_fields); OSMO_ASSERT(dst); OSMO_ASSERT(dst_maxlen >= 2 + sizeof(xid_version_number)); - /* Bail if there is no input */ - if (llist_empty(comp_fields)) - return -EINVAL; + /* Prepend header with version number */ + if (version >= 0) { + xid_version_number[0] = (uint8_t) (version & 0xff); + dst = + tlv_put(dst, SNDCP_XID_VERSION_NUMBER, + sizeof(xid_version_number), xid_version_number); + byte_counter += (sizeof(xid_version_number) + 2); + } - /* Prepend header */ - dst = - tlv_put(dst, SNDCP_XID_VERSION_NUMBER, - sizeof(xid_version_number), xid_version_number); - byte_counter += (sizeof(xid_version_number) + 2); + /* Stop if there is no compression fields supplied */ + if (llist_empty(comp_fields)) + return byte_counter; /* Add data compression fields */ rc = gprs_sndcp_pack_fields(comp_fields, comp_bytes, @@ -1283,11 +1286,10 @@ static int decode_xid_block(struct llist_head *comp_fields, uint8_t tag, } /* Transform an SNDCP-XID message (src) into a list of SNDCP-XID fields */ -static int gprs_sndcp_decode_xid(struct llist_head *comp_fields, +static int gprs_sndcp_decode_xid(int *version, struct llist_head *comp_fields, const uint8_t *src, unsigned int src_len, - const struct - entity_algo_table - *lt, unsigned int lt_len) + const struct entity_algo_table *lt, + unsigned int lt_len) { int src_pos = 0; uint8_t tag; @@ -1297,6 +1299,10 @@ static int gprs_sndcp_decode_xid(struct llist_head *comp_fields, int rc; int tlv_count = 0; + /* Preset version value as invalid */ + if (version) + *version = -1; + /* Valid TLV-Tag and types */ static const struct tlv_definition sndcp_xid_def = { .def = { @@ -1327,6 +1333,10 @@ static int gprs_sndcp_decode_xid(struct llist_head *comp_fields, return -EINVAL; } + /* Decode sndcp xid version number */ + if (version && tag == SNDCP_XID_VERSION_NUMBER) + *version = val[0]; + /* Decode compression parameters */ if ((tag == SNDCP_XID_PROTOCOL_COMPRESSION) || (tag == SNDCP_XID_DATA_COMPRESSION)) { @@ -1548,7 +1558,8 @@ static int gprs_sndcp_complete_comp_fields(struct llist_head } /* Transform an SNDCP-XID message (src) into a list of SNDCP-XID fields */ -struct llist_head *gprs_sndcp_parse_xid(const void *ctx, +struct llist_head *gprs_sndcp_parse_xid(int *version, + const void *ctx, const uint8_t *src, unsigned int src_len, const struct llist_head @@ -1559,6 +1570,12 @@ struct llist_head *gprs_sndcp_parse_xid(const void *ctx, struct llist_head *comp_fields; struct entity_algo_table lt[MAX_ENTITIES * 2]; + /* In case of a zero length field, just exit */ + if (src_len == 0) + return NULL; + + /* We should go any further if we have a field length greater + * zero and a null pointer as buffer! */ OSMO_ASSERT(src); comp_fields = talloc_zero(ctx, struct llist_head); @@ -1575,8 +1592,8 @@ struct llist_head *gprs_sndcp_parse_xid(const void *ctx, } /* Parse SNDCP-CID XID-Field */ - rc = gprs_sndcp_decode_xid(comp_fields, src, src_len, lt, - lt_len); + rc = gprs_sndcp_decode_xid(version, comp_fields, src, src_len, + lt, lt_len); if (rc < 0) { talloc_free(comp_fields); return NULL; @@ -1591,7 +1608,8 @@ struct llist_head *gprs_sndcp_parse_xid(const void *ctx, } else { /* Parse SNDCP-CID XID-Field */ - rc = gprs_sndcp_decode_xid(comp_fields, src, src_len, NULL, 0); + rc = gprs_sndcp_decode_xid(version, comp_fields, src, src_len, + NULL, 0); if (rc < 0) { talloc_free(comp_fields); return NULL; -- cgit v1.2.3