Attention is currently required from: neels.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-iuh/+/37414?usp=email )
Change subject: ranap_cn_rx_co(): do not ranap_cn_rx_co_free() on error
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/37414?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: If7545f91c69f06bc32c55cab7dcfcad8350b0473
Gerrit-Change-Number: 37414
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 06 Jul 2024 10:56:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-iuh/+/37393?usp=email
to look at the new patch set (#2).
Change subject: support RAB Assignment in cn_ranap_rx_*_msg_co()
......................................................................
support RAB Assignment in cn_ranap_rx_*_msg_co()
Allow decoding all Assignment Request/Response messages.
A RAB Assignment request is
- connection oriented
- 'initiatingMessage'
- procedureCode == RAB_Assignment
A RAB Assignment response is
- connection oriented
- 'successfulOutcome' or 'unsuccessfulOutcome' or 'outcome'
(3GPP TS 25.413 specifies RAB Assignment as a "Class 3 Elementary
Procedure", but doesn't seem to specify which of these three 'outcome'
types to use for Class 3. In the field we've seen 'successfulOutcome'
and 'outcome'. Either one is fine.)
- procedureCode == RAB_Assignment
Add decoding the request to cn_ranap_rx_initiating_msg_co().
Add decoding the unsuccessful response to
cn_ranap_rx_unsuccessful_msg_co();
Decoding the successful response is already present in
cn_ranap_rx_successful_msg_co().
osmo-gsm-shark uses this to connect a voice call's signalling with RTP.
Change-Id: Ifec566a98cb6141d27b9e5e33d5a78f8b1530658
---
M src/ranap_common_cn.c
1 file changed, 46 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/93/37393/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/37393?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: Ifec566a98cb6141d27b9e5e33d5a78f8b1530658
Gerrit-Change-Number: 37393
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-iuh/+/37413?usp=email )
Change subject: fix memleak: free after aper_decode() also on error
......................................................................
fix memleak: free after aper_decode() also on error
It turns out that aper_decode() wants the caller to ASN_STRUCT_FREE()
always, also even when it returned != RC_OK.
When during a test I was feeding random data (a BSSMAP message) to
ranap_cn_rx_co_decode2(), I ended up with ASAN indicating a memory leak:
```
pkt DEBUG packet.1 RANAP 01 00 03 05 18 01 (decode_iu.c:658)
tag ERROR Error in RANAP ASN.1 decode (ranap_common_cn.c:401)
tag ERROR Not calling cn_ranap_handle_co() due to rc=-1 (ranap_common_cn.c:428)
pkt ERROR packet.1 RANAP failed to decode RANAP data (decode_iu.c:668)
=================================================================
==1920572==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 208 byte(s) in 1 object(s) allocated from:
#0 0x7f34520f3bc7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x7f34526ccddd (/lib/x86_64-linux-gnu/libtalloc.so.2+0x5ddd) (BuildId: 75c550e5dc091c77e1159c52b284f34d0c4d92cd)
Indirect leak of 102 byte(s) in 1 object(s) allocated from:
#0 0x7f34520f3bc7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x7f34526ccddd (/lib/x86_64-linux-gnu/libtalloc.so.2+0x5ddd) (BuildId: 75c550e5dc091c77e1159c52b284f34d0c4d92cd)
SUMMARY: AddressSanitizer: 310 byte(s) leaked in 2 allocation(s).
```
With this patch, the leak is gone.
Change-Id: I03ed2376e520ec6dbcc2bae22f9291e211c7cca9
---
M src/ranap_common_cn.c
1 file changed, 43 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/13/37413/1
diff --git a/src/ranap_common_cn.c b/src/ranap_common_cn.c
index 1a01bb0..26f9e69 100644
--- a/src/ranap_common_cn.c
+++ b/src/ranap_common_cn.c
@@ -399,13 +399,15 @@
dec_ret = aper_decode(NULL, &asn_DEF_RANAP_RANAP_PDU, (void **)&pdu, data, len, 0, 0);
if (dec_ret.code != RC_OK) {
LOGP(DRANAP, LOGL_ERROR, "Error in RANAP ASN.1 decode\n");
- return -1;
+ rc = -1;
+ goto error_free;
}
message->direction = pdu->present;
rc = _cn_ranap_rx_co(pdu, message);
+error_free:
ASN_STRUCT_FREE(asn_DEF_RANAP_RANAP_PDU, pdu);
return rc;
@@ -645,13 +647,15 @@
dec_ret = aper_decode(NULL, &asn_DEF_RANAP_RANAP_PDU, (void **)&pdu, data, len, 0, 0);
if (dec_ret.code != RC_OK) {
LOGP(DRANAP, LOGL_ERROR, "Error in RANAP ASN.1 decode\n");
- return -1;
+ rc = -1;
+ goto error_free;
}
message->direction = pdu->present;
rc = _cn_ranap_rx_cl(pdu, message);
+error_free:
ASN_STRUCT_FREE(asn_DEF_RANAP_RANAP_PDU, pdu);
return rc;
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/37413?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I03ed2376e520ec6dbcc2bae22f9291e211c7cca9
Gerrit-Change-Number: 37413
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-iuh/+/37414?usp=email )
Change subject: ranap_cn_rx_co(): do not ranap_cn_rx_co_free() on error
......................................................................
ranap_cn_rx_co(): do not ranap_cn_rx_co_free() on error
When ranap_cn_rx_co_decode2() returns an error, we should not call
ranap_cn_rx_co_free(). It results in logging
DRANAP INFO Freeing RANAP Procedure unknown 0x0 (CO) from RNC not implemented
Change the function flow to early-exit pattern to skip
ranap_cn_rx_co_free() on error.
Change-Id: If7545f91c69f06bc32c55cab7dcfcad8350b0473
---
M src/ranap_common_cn.c
1 file changed, 23 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/14/37414/1
diff --git a/src/ranap_common_cn.c b/src/ranap_common_cn.c
index 26f9e69..de481d0 100644
--- a/src/ranap_common_cn.c
+++ b/src/ranap_common_cn.c
@@ -426,16 +426,16 @@
int rc;
rc = ranap_cn_rx_co_decode2(&message, data, len);
-
- if (rc == 0)
- (*cb)(priv, &message);
- else
+ if (rc) {
LOGP(DRANAP, LOGL_ERROR, "Not calling cn_ranap_handle_co() due to rc=%d\n", rc);
+ return rc;
+ }
+
+ (*cb)(priv, &message);
/* Free the asn1 structs in message */
ranap_cn_rx_co_free(&message);
-
- return rc;
+ return 0;
}
static int cn_ranap_rx_initiating_msg_cl(RANAP_InitiatingMessage_t *imsg, ranap_message *message)
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/37414?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: If7545f91c69f06bc32c55cab7dcfcad8350b0473
Gerrit-Change-Number: 37414
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange