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/.
Harald Welte gerrit-no-reply at lists.osmocom.orgHarald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/13778 ) Change subject: osmo-bsc_nat: Parse MGCP Connection ID as hex ...................................................................... osmo-bsc_nat: Parse MGCP Connection ID as hex Our ttcn3-bscnat-tests would randomly fail. After the CRCX ACK returns from the MSC the bsc-nat reports it could not find a CI it it and deletes the connection on the BSC-side. This happens because the field is parsed as a decimal value instead of hexadecimal. So a value of 00FED122 is parsed as '0' which is a reserved value in our program. This fix parses the field as hexadecimal value and also logs an error if the value happens to be 0. make check will now test if a hexadecimal CI is parsed correctly. Fixes: OS#3951 Change-Id: I49b8b61644bf706162102dce268cae2265536fc5 --- M openbsc/src/libmgcp/mgcp_protocol.c M openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c M openbsc/tests/bsc-nat/bsc_data.c M openbsc/tests/bsc-nat/bsc_nat_test.c 4 files changed, 11 insertions(+), 8 deletions(-) Approvals: Jenkins Builder: Verified dexter: Looks good to me, approved Harald Welte: Looks good to me, approved diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c index 84dbc1f..689c91f 100644 --- a/openbsc/src/libmgcp/mgcp_protocol.c +++ b/openbsc/src/libmgcp/mgcp_protocol.c @@ -208,7 +208,7 @@ len = snprintf(sdp_record, size, "v=0\r\n" - "o=- %u 23 IN IP4 %s\r\n" + "o=- %x 23 IN IP4 %s\r\n" "s=-\r\n" "c=IN IP4 %s\r\n" "t=0 0\r\n", @@ -285,7 +285,7 @@ } len = snprintf(sdp_record, sizeof(sdp_record), - "I: %u%s\n\n", endp->ci, osmux_extension); + "I: %x%s\n\n", endp->ci, osmux_extension); if (len < 0) return NULL; @@ -512,7 +512,7 @@ uint32_t ci = strtoul(_ci, NULL, 10); if (ci != endp->ci) { - LOGP(DMGCP, LOGL_ERROR, "ConnectionIdentifiers do not match on 0x%x. %u != %s\n", + LOGP(DMGCP, LOGL_ERROR, "ConnectionIdentifiers do not match on 0x%x. %x != %x\n", ENDPOINT_NUMBER(endp), endp->ci, _ci); return -1; } @@ -891,7 +891,7 @@ osmo_jibuf_set_dequeue_cb(endp->bts_jb, mgcp_dejitter_udp_send, &endp->net_end); } - LOGP(DMGCP, LOGL_DEBUG, "Creating endpoint on: 0x%x CI: %u port: %u/%u\n", + LOGP(DMGCP, LOGL_DEBUG, "Creating endpoint on: 0x%x CI: %x port: %u/%u\n", ENDPOINT_NUMBER(endp), endp->ci, endp->net_end.local_port, endp->bts_end.local_port); if (p->cfg->change_cb) diff --git a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c index 311ab94..17dc659 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c +++ b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c @@ -812,11 +812,14 @@ return CI_UNUSED; } - if (sscanf(res, "I: %u", &ci) != 1) { + if (sscanf(res, "I: %x", &ci) != 1) { LOGP(DMGCP, LOGL_ERROR, "Failed to parse CI in msg '%s'\n", str); return CI_UNUSED; } + if (ci == CI_UNUSED) + LOGP(DMGCP, LOGL_ERROR, "CI field '%s' parsed as reserved value CI_UNUSED\n", str); + return ci; } diff --git a/openbsc/tests/bsc-nat/bsc_data.c b/openbsc/tests/bsc-nat/bsc_data.c index 71d5391..d29caef 100644 --- a/openbsc/tests/bsc-nat/bsc_data.c +++ b/openbsc/tests/bsc-nat/bsc_data.c @@ -157,8 +157,8 @@ /* patch the ip and port */ -static const char crcx_resp[] = "200 23265295\r\nI: 1\r\n\r\nv=0\r\nc=IN IP4 172.16.18.2\r\nm=audio 4002 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\n"; -static const char crcx_resp_patched[] = "200 23265295\r\nI: 1\r\n\r\nv=0\r\nc=IN IP4 10.0.0.1\r\nm=audio 999 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\na=fmtp:98 mode-set=2 octet-align=1\r\n"; +static const char crcx_resp[] = "200 23265295\r\nI: 0F\r\n\r\nv=0\r\nc=IN IP4 172.16.18.2\r\nm=audio 4002 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\n"; +static const char crcx_resp_patched[] = "200 23265295\r\nI: 0F\r\n\r\nv=0\r\nc=IN IP4 10.0.0.1\r\nm=audio 999 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\na=fmtp:98 mode-set=2 octet-align=1\r\n"; /* patch the ip and port */ static const char mdcx[] = "MDCX 23330829 8 at mgw MGCP 1.0\r\nC: 394b0439fb\r\nI: 1\r\nL: p:20, a:AMR, nt:IN\r\nM: recvonly\r\n\r\nv=0\r\no=- 1049380491 0 IN IP4 172.16.18.2\r\ns=-\r\nc=IN IP4 172.16.18.2\r\nt=0 0\r\nm=audio 4410 RTP/AVP 126\r\na=rtpmap:126 AMR/8000/1\r\na=fmtp:126 mode-set=2 octet-align=1;start-mode=0\r\na=ptime:20\r\na=recvonly\r\nm=image 4412 udptl t38\r\na=T38FaxVersion:0\r\na=T38MaxBitRate:14400\r\n"; diff --git a/openbsc/tests/bsc-nat/bsc_nat_test.c b/openbsc/tests/bsc-nat/bsc_nat_test.c index 2914a01..e0d0051 100644 --- a/openbsc/tests/bsc-nat/bsc_nat_test.c +++ b/openbsc/tests/bsc-nat/bsc_nat_test.c @@ -683,7 +683,7 @@ } ci = bsc_mgcp_extract_ci(crcx_resp); - if (ci != 1) { + if (ci != 0x0F) { printf("Failed to parse the CI. Got: %d\n", ci); abort(); } -- To view, visit https://gerrit.osmocom.org/13778 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I49b8b61644bf706162102dce268cae2265536fc5 Gerrit-Change-Number: 13778 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Willmann <dwillmann at sysmocom.de> Gerrit-Reviewer: Daniel Willmann <dwillmann at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: dexter <pmaier at sysmocom.de> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190425/a31734fb/attachment.htm>