aboutsummaryrefslogtreecommitdiffstats
AgeCommit message (Collapse)AuthorFilesLines
2020-03-26gsm_rlcmac: Use 'struct bitvec' instead of 'bitvec'Pau Espin Pedrol3-20/+20
The later is only accepted when compiling as c++. Change-Id: Ib5004643d4eff3bc9d11b66ddaf262f1c0d2aef6
2020-03-26pcu_l1_if.cpp: Add missing header ctype.hPau Espin Pedrol1-0/+1
The file uses the isdigit() function. Change-Id: Id06ea25969ad9b964b3207479604132d25160f24
2020-03-25rlcmac: Rename field to MS RA Cap2 in Additional_MS_Rad_Access_Cap_tPau Espin Pedrol2-2/+2
This fix was spotted by wireshark.git developers while reviewing port of osmo-pcu commit e50ce6e45c4509805807d599cadf1a1b23d37f63. Related: https://code.wireshark.org/review/c/36574/ Fixes: e50ce6e45c4509805807d599cadf1a1b23d37f63 Change-Id: Ic5fc3252f61b6a042d9c3def7a6a32b311dd9d9e
2020-03-25cosmetic: rlcmac: Fix comment typo and whitespace introduced recentlyPau Espin Pedrol1-1/+2
Change-Id: Ie56898e11e2c9426393af0f6558d340c454fe6c4
2020-03-23rlcmac: Introduce MS Radio Access Capabilities 2 to fix related spare bitsPau Espin Pedrol6-58/+57
There's two variants for the Ms Radio Access Capabilities. * The usual encoding with spare bits (usually to fill up to octet boundary) as defined in TS 24.008 Table 10.5.146 And there's too: * MS Radio Access Capabilities 2 IE from TS44.060 section 12.30, which is the same but removing all spare bits, and which is used in messages like Packet Resource Request and Additional MS RAC messages. The later is used basically for messages having extra IEs after the MS Radio Access capabilities IE, since they are encoded immediatelly afterwards. So this patch does: * Adds the expected spare bits (M_PADDING) to MS_Radio_Access_capability_t * Creates a new MS_Radio_Access_capability2_t without padding * Updates code to use the new "2" version where needed. Note RLCMACTest long de/encoding line logs change only because the name of the struct changes (the "2" is added). Change-Id: Ibd756f80a03452a651e2771dbc628d701e55ac4b
2020-03-23rlcmac: Log names of de/encoded rlcmac packet typesPau Espin Pedrol2-157/+120
Change-Id: I6a6e79d7e12cd5e8e969bf0eaa30ddac6b0aa7d3
2020-03-23rlcmac: Fix bug receiving RA capPau Espin Pedrol4-20/+16
It seems the assumptions regarding maximum number of RA capabilitites in one message were wrong. Doing some rough calculations, each RA capabilitiy value (without extensions) can take around 20ish bits, which means for a message containing up to 52 bytes that quite a lot of different values could be theoretically fed in. Let's be safe and increase the array size to be able to handle all different access technologies listed in See TS 24.008 table 10.5.146 following restrictions: * "The MS Radio Access capability is a type 4 information element, with a maximum length of 52 octets." * "Among the three Access Type Technologies GSM 900-P, GSM 900-E and GSM 900-R only one shall be present." * "the mobile station should provide the relevant radio access capability for either GSM 1800 band OR GSM 1900 band, not both". Wireshark requires similar fix (it's not important though because it currently uses another ad-hoc decoder for RAcap). Related: OS#4463 Change-Id: I5334eaacfbc238fae8bea50c9e9667c2117f81ff
2020-03-23csn1: Validate recursive array max size during decodingPau Espin Pedrol5-5/+163
This way if CSN1 encoded bitstream contains more elements than what the defintion expects it will fail instead of overflowing the decoded buffer. RA cap struct placed in unit test is taken from a real android phone sending the value when attaching to the network. Then SGSN sends it back and osmo-pcu would crash similar to unit test: *** stack smashing detected ***: terminated Process terminating with default action of signal 6 (SIGABRT): dumping core at 0x4C62CE5: raise (in /usr/lib/libc-2.31.so) by 0x4C4C856: abort (in /usr/lib/libc-2.31.so) by 0x4CA62AF: __libc_message (in /usr/lib/libc-2.31.so) by 0x4D36069: __fortify_fail (in /usr/lib/libc-2.31.so) by 0x4D36033: __stack_chk_fail (in /usr/lib/libc-2.31.so) by 0x124706: testRAcap2(void*) (RLCMACTest.cpp:468) Related: OS#4463 Change-Id: I9fe0e55e0a6a41ae2cc885fba490c1d4a186231e
2020-03-23rlcmac: Don't pass array element to CSN1 descriptorsPau Espin Pedrol1-12/+11
This way the macros can be used to access the arrays themselves and calculate its static size to enable validation later. In the case of Packet_Access_Reject_t, modify the description to use a M_REC_TARRAY_1 object to get rid of access to 2nd element. The new description is the correct one, since the first element is mandatory according to TS 44.060 Table 11.2.1. Change-Id: I6f10350d4734360c7a15a702c72b59efd84987ee
2020-03-21tests/RLCMACTest: Several fixes and improvements to RAcap testsPau Espin Pedrol3-40/+91
It was recently discovered that the Racap value used for testRAcap was actually malformed (it was taken from a TTCN3 test). It had the presence bit for "EGPRS multislot class" set but no struct was put after it. So let's move that malformed value to a new test named testMalformedRAcap(). Since it doesn't make sense trying to re-encode or do similar things with an initially malformed value, let's drop all those parts in this new test. For the old testRAcap() test, use a new bitstream this time with the "EGPRS multoslot class" struct set inside (class 3). Change-Id: I1e7f8d8866695732ee24a79d8b54d660fd4f22d5
2020-03-20tests/RLCMACTest: free allocated bitvectorsPau Espin Pedrol1-0/+3
Change-Id: I6d1e93cb1a07a7bf05483dbc877105a86a17829b
2020-03-19csn1.c: Almost all of the logging is DEBUG, not NOTICEHarald Welte2-107/+107
low-level text decodes of CSN.1 messages certainly are not NOTICEable events, but rather something used for debugging. Right now we get various text CSN.1 log output of osmo-pcu in it's default configuration. Despite all log levels being relatively high (NOTICE), we still see those messages as they simply are logged at the wrong level. Related: OS#2577 Change-Id: I7b42c9e21ad8d8a5b54e7a3b68490934ce3d3198
2020-03-18Use downlink BSSGP RA Cap IEPau Espin Pedrol1-6/+1
This commit is basically a revert of f4bb42459ca4f3e18f9ee3d3a27389b85c7692e8, which disabled the code. That commit claimed the SGSN may have providen inacurate or wrong data at the time, but then it should be fixed in the SGSN. Related: OS#1525, OS#3499 Change-Id: Ie36ae23203110018d4b5ae47591e0a64989e23a0
2020-03-16Use clock_gettime(CLOCK_MONOTONIC) and timespec everywherePau Espin Pedrol12-122/+130
We should really be using monotonic clock in all places that gettimeofday is used right now. Since clock_gettime() uses timespec, let's move all code to use timespecs instead to avoid having to convert in several places between timespec and timeval. Actually use osmo_clock_gettime() shim everywhere to be able to control the time everywhere from unit tests. Change-Id: Ie265d70f8ffa7dbf7efbef6030505d9fcb5dc338
2020-03-11csn1: fix: do not return 0 if no bits left in the bufferVadim Yanitskiy3-6/+7
Both csnStreamDecoder() and csnStreamEncoder() shall not return 0 prematurely if no more bits left in the input / output bit-vector. Returning CSN_ERROR_NEED_MORE_BITS_TO_UNPACK might make more sense, however we don't know in advance (i.e. without entering the loop) whether it's an error or not. Some CSN.1 definitions have names like 'M_*_OR_NULL', what basically means that they're optional and can be ignored or omitted. Most of the case statements do check whether the number of remaining bits is enough to unpack / pack a value, so let's leave it up to the current CSN_* handler (pointed by pDescr) if no bits left. Return CSN_ERROR_NEED_MORE_BITS_TO_UNPACK only if the number of remaining bits is negative as this is an error in any case. Change-Id: Ie3a15e210624599e39b1e70c8d34efc10c552f6c
2020-03-11rlcmac: fix encode_gsm_*(): do not suppress encoding errorsVadim Yanitskiy3-11/+14
Change-Id: Ieec8e6e0823c6f6985f9d343af6d503b8fe9e6ab
2020-03-11tests/llc: Change unrealistic time jump to avoid runtime error under ARMPau Espin Pedrol1-2/+2
When running the test under a RaspberryPI4: +../../../src/llc.cpp:216:29: runtime error: signed integer overflow: 864197544 * 1000 cannot be represented in type 'long int' 864197544 comes from comparing initial insertion time and dequeue time (test does a big jump in time): 987654321 - 123456777 let's use more realistic time changes, since the current one account for about 37 years. Change-Id: I28abc9192e0e7c590bc1c3c88950627cf669ffaf
2020-03-07tests/rlcmac: also enable logging for DRLCMACDATA categoryVadim Yanitskiy2-1/+3
Change-Id: Idf0808461f7e7a1bce58d11a54238c215126451a
2020-03-07gsm_rlcmac: improve dissection of MS RA Capability IEVincent Helfre5-12/+80
Port from wireshark.git de028e81c53f9c45ccc5adb3bffd2f16ae2017bf This commit breaks transcoding of the test vectors containing the MS RA Capability IE due to the reasons explained in [1]. The more fields we add, the longer gets the output of the CSN.1 encoder. This is not critical, since we never need to encode messages containing the MS RA Capability IE on practice. [1] Ibb4cbd3f5865415fd547e95fc24ff31df1aed4c0 Ported-by: Pau Espin Pedrol <pespin@sysmocom.de> Change-Id: Ibb4cbd3f5865415fd547e95fc24ff31df1aed4c0
2020-03-06csn1: fix csnStreamDecoder(): skip bits unhandled by serialize()Vadim Yanitskiy3-4/+11
This change fixes a bug that was reported by Keith Whyte and confirmed in [1]. The problem is that a user-defined handler in case of CSN_SERIALIZE may parse only a part of the given bit-stream, leaving some bits unhandled. This is expected because the sender (i.e. the MS) may use more recent RLC/MAC message definitions containing new fields at the end. Those bits that were left unhandled by serialize() shall not be interpreted as continuation of the message, they shall be skipped. Note that the encoded vector in the RLCMAC unit test still does not match the original one. That's a known bug explained in [2]. [1] If5873355d52d7ddb06c2716154a88d34100f6ab5 [2] Ic46d6e56768f516203d27d8e7a5adb77afdf32b7 Change-Id: Id4cc042fed68fc54aca0355dcb986cab3f6b49ea Related: OS#4338
2020-03-06tests/rlcmac: add a new test vector for Packet Resource RequestVadim Yanitskiy3-0/+11
This test vector (from HTC Desire 628) demonstrates a bug in the CSN.1 decoder. For some reason, OsmoPCU fails to decode it: DCSN1 ERROR csnStreamDecoder: error NEED_MORE BITS TO UNPACK (-5) at EGPRS_TimeslotLinkQualityMeasurements (idx 164) while Wireshark dissects it without any errors. Change-Id: If5873355d52d7ddb06c2716154a88d34100f6ab5 Reported: https://osmocom.org/issues/4338#note-15 Related: OS#4338
2020-03-05Send UL-CTRL Packet to GSMTAP even if we fail to decode.Keith1-5/+6
Move the call to send_gsmtap() before the call to decode_gsm_rlcmac_uplink() as if the latter returns error we return and never get to see the packet on the GSMTAP. Change-Id: Ia6af9f40590f28fcae3fef50d9c601d8435412cd
2020-03-02gsm_rlcmac: fix Packet_Resource_Request_t: s/Slot/I_LEVEL_TN/Pau Espin Pedrol4-8/+8
This is how this field is named in Wireshark. Change-Id: I140443c48af8e4bb1b6279e6de986879b7d9c276
2020-03-02tests/rlcmac: also verify encoding of MS RA CapabilityVadim Yanitskiy4-7/+58
The main idea of this change is to demonstrate a weakness of the CSN.1 codec that most likely causes a unit test breakage in [1]. The problem seems to be that the transitional structures, where the CSN.1 decoder stores the results, do not contain any details about presence of the optional fields (such as M_UINT_OR_NULL). In other words, it's impossible to know whether some optional field is omitted in the encoded message (NULL), or is it just set to 0. This means that the encoder will always include all optional fields, even if they're not present in the original message. [1] Ibb4cbd3f5865415fd547e95fc24ff31df1aed4c0 Change-Id: Ic46d6e56768f516203d27d8e7a5adb77afdf32b7
2020-03-02llc_queue::{dequeue,enqueue}() refactorPau Espin Pedrol4-47/+57
As seen in OS#4420, setting the MetaInfo.recv_time outside of llc_queue before calling llc_queue::enqueue() and later on using that value in llc_queue itself at dequeue time is not a good idea, since it can provoke errors if the recv_time was not set correctly. For instance, LlcTest was not setting the value for recv_time on some test, which ended up with a huge millisec value when substracting now() from it: """ llc.cpp:215:29: runtime error: signed integer overflow: 1582738663 * 1000 cannot be represented in type 'long int' """ This issue only appeared when started building on a raspberrypi4. Let's better set/store the MetaInfo.recv_time internally during llc_queue::enqueue(). Then, enqueue() only needs the MetaInfo.expire_time, so let's change its arg list to only receive that to avoid confusions. Take the chance to move the llc_queue APIs to use osmo_gettimeofday, since we need to fake the time now that the API itself sets that time. Also take the chance during this refactor to disallow passing null pointer by default since no user needs that. Finally, update the LlcTest accordingly with all API/behavior changes. Related: OS#4420 Change-Id: Ief6b1464dc779ff22adc2b02da7a006cd772ebce
2020-02-19tests/rlcmac: fix malformed MS RA capability in testRAcap()Vadim Yanitskiy3-13/+22
Long story short: as it turns out the test vector '12a5146200'O has been generated by TITAN, and it's malformed. The length indicator it contains must be at least 29 bits, not 21. This field is calculated by TITAN automatically, so I guess there is a bug somewhere in its RAW encoder implementation. It's funny that Wireshark decodes the old malformed vector without any problems if it's encapsulated into the BSSGP DL-UNITDATA. The reason for that is because BSSGP dissector does not actually use the CSN.1 codec and relies on its own hand-written parser [1], which does not respect the length constraints. Furthermore, table 10.5.146/3GPP TS 24.008, describing the format of MS Radio Access Capability IE, has the following comment: < Multislot capability struct > ::= { 0 | 1 < HSCSD multislot class : bit (5) > } ... -- error: struct too short, assume features do not exist so ideally our CSN.1 decoder should be more tolerant to the old malformed vector, but unfortunately error handling is not implemented. [1] See de_gmm_ms_radio_acc_cap() in epan/dissectors/packet-gsm_a_gm.c. Change-Id: I5f810397b8d09c18e069168023429f6a4d899c86
2020-02-18gsm_rlcmac: fix misleading LOGP statement in decode_gsm_ra_cap()Vadim Yanitskiy2-2/+2
Change-Id: I48fd701566e1364ce7fccaa3e3a1a0296b932988
2020-02-17csn1: use proper format specifier for unsigned integersVadim Yanitskiy2-23/+23
Change-Id: I33f86b79e72394bdb7d99762f8ec21d80e06dc30
2020-02-17csn1: bitvec_get_uint() may return a negative, use %dVadim Yanitskiy1-1/+1
Change-Id: I3cfd66643ec140150a4089b0e1c493d911d3d7d4
2020-02-17csn1: fix csnStreamDecoder(): update bit_offset in CSN_EXIST{_LH}Vadim Yanitskiy1-0/+1
Found while doing differential analysis (comparison against the original implementation from Wireshark). Change-Id: Ibd0b7400d78f7873c2a8d45267332f511b5c6fbb
2020-02-17csn1: fix csnStreamDecoder(): always keep remaining_bits_len updatedVadim Yanitskiy1-2/+4
Found while doing differential analysis (comparison against the original implementation from Wireshark). Change-Id: I9f7fa9c3f2f4ff5213dded930cee7ec509b9d799
2020-02-17csn1: fix csnStreamDecoder(): do not subtract no_of_bits twiceVadim Yanitskiy1-1/+0
Found while doing differential analysis (comparison against the original implementation from Wireshark). Change-Id: Id2a4f03035cd8354d3fba0ad37571453d3986d21
2020-02-17csn1: get rid of C++ specific code, compile with GCCVadim Yanitskiy6-213/+209
The implementation of CSN.1 codec was taken from Wireshark, where it's implemented in pure C. For some reason it was mixed with C++ specific features, mostly using references in parameter declaration. Not sure what are the benefits. Change-Id: I56d8b7fbd2f9f4e0bdd6b09d0366fe7eb7aa327a
2020-02-17tests/rlcmac: additionally match debug output of the CSN.1 codecVadim Yanitskiy5-4/+43
This would allow us to catch more bugs. Note that I had to remove printing of pointer address to make the output deterministic. Change-Id: I1a77441eb957353c919bc73f8e3a2e38f4a383a9
2020-02-16csn1: fix existNextElement(): use bitvec_get_bit_pos()Vadim Yanitskiy1-1/+1
As was discovered recently (see OS#4388), bitvec_read_field() would never return a negative value because its return type is unsigned (uint64_t). We don't really need to get more than one bit, so let's just use the bitvec_get_bit_pos() instead. Change-Id: I763a295cd955cd33f542292c85d97ff82f6b49bc Related: OS#4388
2020-02-15gsm_rlcmac.cpp: fix global-buffer-overflow error reported by ASANPascal Quantin1-1/+4
Port from wireshark.git f751918476bdde65f2289b86245a3c30dace6730. Ported-by: Pau Espin Pedrol <pespin@sysmocom.de> Change-Id: I70d4ff3e137b5fd13d367bd4ea6ab501e81e7a87
2020-02-14gsm_rlcmac.cpp: fix another global-buffer-overflow error reported by ASANPascal Quantin1-1/+4
Port from wireshark.git aa3bbe5aebdc180172e7956719b26199e4784fcc. Ported-by: Pau Espin Pedrol <pespin@sysmocom.de> Change-Id: I808ec66011cdfe8e1193298f7fb7e92d25b45be4
2020-02-14gsm_rlcmac: Update : PACKET RESOURCE REQUEST to Release 14.0.0AndersBroman2-0/+295
Port from wireshark.git 07fc801684ebff7aff02505cdb2c120caea846e0. Ported-by: Pau Espin Pedrol <pespin@sysmocom.de> Change-Id: Iceb59c58406180bc57fe6eb27127b4d11a0a3df7
2020-02-11tests/rlcmac: mark Packet Polling Request as malformedVadim Yanitskiy1-1/+1
It contails no valid identity, and thus violates the specs. Let's keep it 'as-is' to check that decoder actually fails. Change-Id: I663edfdaac0c065e08ab7b6dc50d2f18e433433c Related: OS#4392
2020-02-11csn1: fix csnStreamDecoder(): catch unknown CSN_CHOICE valuesVadim Yanitskiy2-2/+10
After the recent changes [1], it was noticed that one of the unit tests fails. In particular, a decode-encode cycle of Packet Polling Request produces a different vector: vector1 = 49 13 e0 08 50 88 40 13 a8 04 8b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b vector2 = 49 13 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b vector1 == vector2 : FALSE As it turns out, the original (input) vector itself is malformed because it contails no valid identity, and thus violates the specs. The CSN.1 decoder from Pycrate [2] throws an exception while trying to decode it. I believe we should do the same. Let's stop decoding the bit stream and return an error in case if neither of a given list of the choice items matched. [1] Ia0f8cc224a4c38e80699f834fd83d4c0d99322ea [2] https://github.com/P1sec/pycrate Change-Id: I420144773ed5e80372534e0f18db5e74cdb2999d Fixes: OS#4392
2020-02-11csn1: fix some mistaken CSN.1 error namesVadim Yanitskiy1-2/+2
Change-Id: I3bd9954ee36212c94f0c4d91581da300c56fce60
2020-02-10encoding: assert return value of bitvec_set_u64()Vadim Yanitskiy1-1/+2
Change-Id: Ic0de3ae34f06e41aacacb917f5a0214623259bdc Fixes: OS#182120
2020-02-10tbf: fix NULL pointer dereference in create_[ul|dl]_ass()Vadim Yanitskiy1-2/+4
The problem is that bitvec_free() is not NULL-safe. Ideally we need to fix it in libosmocore [1], but let's also fix it here, so OsmoPCU can be safely used with older libosmocore versions. [1] https://gerrit.osmocom.org/c/libosmocore/+/17114 Change-Id: I7647d17b3d03f8e193ef6e793a2d3c1967744eef Fixes: CID#208181, CID#208179
2020-02-09tbf: cosmetic: fix spacing in gprs_rlcmac_tbf::create_ul_ass()Vadim Yanitskiy1-1/+1
Change-Id: Ice4c4db20551753fa4219e7a216309229f7a2ab5
2020-02-08Fix trailing newline mess with LOGP(C) in rlcmac/csn1Pau Espin Pedrol2-1/+38
Output was incorrect before this patch. LOPC was being called without having any initial LOGP, and trailing newline was usually missing at the end. Since csnDecoder/encoder functions are recursive, it's difficult to handle logging state in a coherent way inside them. Let's better simply control start/end of logging related topics in the callers of those functions, and simply use LOGPC everywhere in csn1.cpp. Change-Id: I50da7560939fac360b7545e2a6bfaf45ed0c4832
2020-02-08pcu_sock: cosmetic: fix typo in a comment messageVadim Yanitskiy1-1/+1
Change-Id: Ia7e91d803152ac3f2af88f4552fced27f59d8270
2020-02-08pcu_sock: fix memleak, allocate pcu_sock_state on stackVadim Yanitskiy1-52/+26
It was noticed that OsmoPCU leaks memory when trying to reconnect to the BTS. It could be easily fixed, but we don't really need to allocate the PCU socket state on heap as we never have more than one connection. Change-Id: Iea8930f443caa16f522f7c5375e0004e4e2315cb
2020-02-08VTY: install talloc context introspection commandsVadim Yanitskiy2-0/+2
Change-Id: I2e1e26be1162b0fb695969e5ac265704adaf9d5c
2020-02-08VTY: get rid of pcu_vty_go_parent() / pcu_vty_is_config_node()Vadim Yanitskiy1-31/+0
Since I2b32b4fe20732728db6e9cdac7e484d96ab86dc5, go_parent_cb() is completely optional. It no longer has the task to determine the correct parent node. The is_config_node() callback is no longer needed too. Get rid of them. Since Ic5e69a396df659933fd4d50298b9925e837a6861 we depend on 1.3.0. Change-Id: Id7ce8c4e1ac43747ad40a06d01433c366da07b42
2020-02-06csn1: fix csnStreamDecoder(): avoid conditional calls to bitvec_read_field()Vadim Yanitskiy1-4/+4
As was discovered by pespin, changing logging level of DCSN1 makes the CSN.1 decoder behave differently (see OS#4375). In particular, this makes RLCMACTest (encode / decode test) fail. I did a quick investigation and noticed that some of the logging statements call bitvec_read_field(). By definition this function moves the internal pointer (current bit position) of a given vector and increments readIndex by a given amount of bits. The problem is that LOGPC would not evaluate its format string if the logging message is not going to be printed, e.g. if a given logging level is lower than the current one, or in case if logging is not enabled at all. The first two conditional calls to bitvec_read_field() are related to CSN_PADDING_BITS, so that's not critical because padding is always in the end of messages. The later two are related to CSN_RECURSIVE_ARRAY and CSN_RECURSIVE_TARRAY respectively. Let's use bitvec_get_uint() instead to keep readIndex unchanged. Change-Id: Ia331048db9f790ca407fd341ced01df12d10a233 Fixes: OS#4375