Commit Graph

1262 Commits

Author SHA1 Message Date
Pau Espin 20331ae5f6 pdch: Avoid sending GSMTAP_CHANNEL_UNKOWN for rejected UL EGPRS data block
Even if we don't accept it, let's submit GSMTAP with correct channel.
We don't return error like in code below, because otherwise the generic
UNKNOWN gsmtap message will be sent.

Change-Id: I853679ce8907d46fcb84ae4127335c10623f09c9
2020-04-29 21:02:12 +02:00
Pau Espin 3301cc900e pcu_l1_if: Don't use GSMTAP_CHANNEL_PACCH when sending unknown gsmtap blocks
It's actually counter-productive when analyzing wireshark traces, since
one may not spot a decoding issue and assume PACCH is sent on the wrong
TS.

Change-Id: I7a96148f1ca1ebfa88a3ff714ea3bb8798866046
2020-04-29 20:56:31 +02:00
Vadim Yanitskiy 3f27fb56e4 TBF/UL: fix rcv_data_block_acknowledged(): print the actual TLLI
Change-Id: I71b5c656d4b318d11bd5fe2b5d163c3a06e09a6a
2020-04-20 13:18:49 +07:00
Vadim Yanitskiy 72ec18ce5a sba: fix possible memleak in SBAController::alloc()
Change-Id: I417eda155cd5b1e46dd0b05db3f507abd79121d1
2020-04-20 11:25:05 +07:00
Vadim Yanitskiy b30f28f38d l1if: fix pcu_rx_rach_ind(): use proper format string specifiers
Change-Id: If95362ef4cc203a60856d6b47d95d441813a5c19
2020-04-02 05:42:57 +07:00
Pau Espin c374ab00ac csn1: Remove code block from CSN_NEXT_EXIST_LH
It was removed in wireshark.git e8407dd6c1378427daee77e8de540d0b5f7a0b73
and it's not there anymore in current master.

Change-Id: I73f4eeca3fd4f00a5bc4f06ef7a9bb9b8a70e37b
2020-03-30 10:08:02 +00:00
Pau Espin c8280a538a csn1: Properly verify CSN_BITMAP length
Change-Id: I9f7672b534f9345caff99095504749eebad25adb
2020-03-30 10:08:02 +00:00
Pau Espin f5e275aec0 csn1: verify enough bits present to decode whole CSN_UINT_ARRAY
Change-Id: I4a762a8fec4153b53e10df1ec8ba3708c1f47649
2020-03-30 10:08:02 +00:00
Pau Espin 70a211747b csn1: Fix Several typos & whitespace
Change-Id: Ibe31d52d4a5a4015196d73681082f68b99a80c77
2020-03-30 10:08:02 +00:00
Pau Espin 99eb353337 rlcmac: add dissection of 2G->3G/4G PS handover
Port from wireshark.git 428ee66ae1c524b49f9043729b1f1e9b4f52f409, from
Pascal Quantin.

The original commit is also changing the RRC_Container field to
M_CALLBACK, but we leave them as M_VAR_ARRAY since the callback is
basically used to add more dissection information in wireshark.

Change-Id: I0f374e78300efddff00c4df26a401adcdee18a12
2020-03-30 10:08:02 +00:00
Pau Espin f3ac06bbaf rlcmac: support decode FDD_CELL_INFORMATION of "UTRAN FDD Description
Port of patch (+ later fixes squashed) of wireshark.git commit
dea5452b95dfaf18e38670a8e2b3b38f9175fdfd, from Lei Chen:
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6856

Squashed wireshark.git fix commits:
774be29de0b4d93d01aecb1518c41d7d551071a9
51c31cd7bd3d8fc196a9f90a8af466ad84e9e6a8
6aca10831f86c562970b13efa811f46e25ee3091
c1ceac58cdb77051e9bd14c1f6f7669cf5779a86

Change-Id: I08523bc1bbdffde479ef974b4c7b56cfa5639591
2020-03-30 10:08:02 +00:00
Pau Espin b2653fe619 Move gsm_rlcmac.cpp -> .c
Original file from wireshark.git (packet-gsm_csn1.c) is being built and
maintained as a C file. There's no real need for us to maintain it as a
C++, and doing so will make both files derive over time (as already
happened). Let's keep it as a C compiler (which btw seems to be more
strict) to make it easier to port patches back and forth wireshark.git.

Take the chance to move some declarations we added to csn1.h to be able
to build it out of wireshark. Let's keep those in a separate header file
to ease looking for differences.

Change-Id: I818a8ae947f002d35142f9f5473454cfd80e1830
2020-03-30 10:08:02 +00:00
Pau Espin 799cf8221a gsm_rlcmac: Disable unused CSN1 descriptors
When switching to C compiler, it will warn/error. Use #if 0 as in the
original wireshark.git epan/dissectors/packet-gsm_rlcmac.c code.

Change-Id: If1be50947c02208f15892d99edeb394fb4f52b75
2020-03-30 10:08:02 +00:00
Vadim Yanitskiy d2e50e7f21 fix: properly include pure C headers from C++ code
Header files included from libosmocore may potentially contain
some language constructions allowed in C but not in C++, such
as type casting. Let's add 'extern "C" { ... }' and be safe.

Change-Id: I7197f7b34f30b49d5397506ce9d67cbf0e2cc196
2020-03-29 01:46:36 +07:00
Pau Espin a0cbde700a tbf.cpp: Include c++ <new> header required for new operator's replacement type
Including the <new> header is required as explained by the c++ specs [1]

osmo-pcu/src/tbf.cpp: In function ‘gprs_rlcmac_ul_tbf* tbf_alloc_ul_tbf(gprs_rlcmac_bts*, GprsMs*, int8_t, uint8_t, uint8_t, bool)’:
osmo-pcu/src/tbf.cpp:1002:39: error: no matching function for call to ‘operator new(sizetype, gprs_rlcmac_ul_tbf*&)’
 1002 |  new (tbf) gprs_rlcmac_ul_tbf(bts->bts);
      |                                       ^

Most of the times this issue is not detected because other STL headers
are already including <new>.

[1] http://www.cplusplus.com/reference/new/operator%20new/

Change-Id: Ie5fb536ae29dcf40e2a0dbe67432bebd61b8c7aa
2020-03-27 15:06:41 +00:00
Vadim Yanitskiy ce160147f4 pdch: cosmetic: use GSM_MI_TYPE_* constants from libosmocore
Change-Id: I1d85c8eded68bc8aa8e90c33b36d335fa775ded2
2020-03-27 14:55:22 +00:00
Vadim Yanitskiy d71c566ee6 pdch: fix packet_paging_request(): properly print paging MI
This problem problem was discovered by the Undefined Behavior Sanitizer:

  pdch.cpp:210:4: runtime error: load of misaligned address
                  0x60c00002abf2 for type 'uint32_t',
                  which requires 4 byte alignment

Do not convert TMSI to number, use osmo_mi_name() from libosmocore.
Also use this function to print other MI types (IMSI or IMEISV).

Change-Id: Icf8836f216793e342b239c8e6645aac1e82bf324
2020-03-27 14:55:22 +00:00
Vadim Yanitskiy 0b25f693b3 BSSGP: fix: properly encode P-TMSI in RR Paging Request
The TMSI/P-TMSI IE in BSSGP PAGING-PS/CS comes without the MI type
header, that must be present in RR Paging Request. Prepend it.

TTCN-3 test case: I7fbec5b2c5c3943a7413417b623f55c135c152d7

Change-Id: I97fd5ffc15a4a58112d7c37c69b7ac42b0741a0e
2020-03-27 14:55:22 +00:00
Vadim Yanitskiy f497412f20 BSSGP: cosmetic use OSMO_IMSI_BUF_SIZE from libosmocore
Change-Id: I2079118100a3422e9c2ee63a5ac74e3ee81080b3
2020-03-27 14:55:22 +00:00
Keith Whyte 2b6acd8873 Don't check ul_control_block before decoding into it.
This patch corrects an error introduced in
6fd8ffb6fe
That commit allowed us to send the data over GSMTAP even
if the Uplink Control Block had invalid content,
that is to say, if decode_gsm_rlcmac_uplink() returned error.

However the check for ul_control_block->u.MESSAGE_TYPE
was place before decode_gsm_rlcmac_uplink()

Change-Id: Ic47602e5c6a13571b92c0a939fc3514110b82444
2020-03-26 23:32:49 +00:00
Pau Espin e5e2f747c3 csn1.h: Fix trailing whitespace
Change-Id: If17d36378fabeb7d22a513b93b0ecfde899df520
2020-03-26 16:03:56 +01:00
Pau Espin 24fa27453f gsm_rlcmac.cpp: Avoid declaring variable in for loop
That's only allowed under C++.

Change-Id: I0da8849a0fb7f9a7ee5e726edea87e91dc411ea8
2020-03-26 16:03:56 +01:00
Pau Espin 980ef1e38b cosmetic: Do not indent header includes inside extern C block
Change-Id: I3091d917a13e45b3aef8e52a53dcafa308581652
2020-03-26 16:03:56 +01:00
Pau Espin 007056e4ad gsm_rlcmac: Use 'struct bitvec' instead of 'bitvec'
The later is only accepted when compiling as c++.

Change-Id: Ib5004643d4eff3bc9d11b66ddaf262f1c0d2aef6
2020-03-26 16:03:27 +01:00
Pau Espin 754b093d16 pcu_l1_if.cpp: Add missing header ctype.h
The file uses the isdigit() function.

Change-Id: Id06ea25969ad9b964b3207479604132d25160f24
2020-03-26 14:49:04 +01:00
Pau Espin de63548f04 rlcmac: Rename field to MS RA Cap2 in Additional_MS_Rad_Access_Cap_t
This fix was spotted by wireshark.git developers while reviewing port of
osmo-pcu commit e50ce6e45c.

Related: https://code.wireshark.org/review/c/36574/
Fixes: e50ce6e45c

Change-Id: Ic5fc3252f61b6a042d9c3def7a6a32b311dd9d9e
2020-03-25 20:26:07 +01:00
Pau Espin 10ec506cc9 cosmetic: rlcmac: Fix comment typo and whitespace introduced recently
Change-Id: Ie56898e11e2c9426393af0f6558d340c454fe6c4
2020-03-25 14:32:30 +01:00
Pau Espin e50ce6e45c rlcmac: Introduce MS Radio Access Capabilities 2 to fix related spare bits
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-23 19:02:01 +01:00
Pau Espin 20848c3ae5 rlcmac: Log names of de/encoded rlcmac packet types
Change-Id: I6a6e79d7e12cd5e8e969bf0eaa30ddac6b0aa7d3
2020-03-23 19:02:01 +01:00
Pau Espin 7faa5da209 rlcmac: Fix bug receiving RA cap
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-23 19:01:58 +01:00
Pau Espin efad80bfbf csn1: Validate recursive array max size during decoding
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-23 15:34:11 +01:00
Pau Espin 81b40cbaf3 rlcmac: Don't pass array element to CSN1 descriptors
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-23 15:29:48 +01:00
Harald Welte 570f9135cd csn1.c: Almost all of the logging is DEBUG, not NOTICE
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-19 15:09:45 +01:00
Pau Espin de7bb75ccf Use downlink BSSGP RA Cap IE
This commit is basically a revert of
f4bb42459c, 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-18 14:01:24 +01:00
Pau Espin 1de6873810 Use clock_gettime(CLOCK_MONOTONIC) and timespec everywhere
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-16 10:31:56 +00:00
Vadim Yanitskiy 29aeb901e4 csn1: fix: do not return 0 if no bits left in the buffer
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-11 19:55:55 +00:00
Vadim Yanitskiy 773cb74ec8 rlcmac: fix encode_gsm_*(): do not suppress encoding errors
Change-Id: Ieec8e6e0823c6f6985f9d343af6d503b8fe9e6ab
2020-03-11 19:55:55 +00:00
Vincent Helfre 1145fd263c gsm_rlcmac: improve dissection of MS RA Capability IE
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-07 05:04:40 +07:00
Vadim Yanitskiy 2679ec0a9f csn1: fix csnStreamDecoder(): skip bits unhandled by serialize()
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-06 21:49:04 +00:00
Keith Whyte 6fd8ffb6fe Send UL-CTRL Packet to GSMTAP even if we fail to decode.
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-05 19:51:06 -06:00
Pau Espin 0daf913e0c gsm_rlcmac: fix Packet_Resource_Request_t: s/Slot/I_LEVEL_TN/
This is how this field is named in Wireshark.

Change-Id: I140443c48af8e4bb1b6279e6de986879b7d9c276
2020-03-02 13:33:23 +00:00
Vadim Yanitskiy b47e53b5fa tests/rlcmac: also verify encoding of MS RA Capability
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-02 13:33:23 +00:00
Pau Espin 5fc6e010a5 llc_queue::{dequeue,enqueue}() refactor
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-03-02 12:05:06 +01:00
Vadim Yanitskiy e60d9c7e9d gsm_rlcmac: fix misleading LOGP statement in decode_gsm_ra_cap()
Change-Id: I48fd701566e1364ce7fccaa3e3a1a0296b932988
2020-02-18 05:30:57 +07:00
Vadim Yanitskiy 1553049226 csn1: use proper format specifier for unsigned integers
Change-Id: I33f86b79e72394bdb7d99762f8ec21d80e06dc30
2020-02-17 19:40:15 +07:00
Vadim Yanitskiy 4b57b6da54 csn1: bitvec_get_uint() may return a negative, use %d
Change-Id: I3cfd66643ec140150a4089b0e1c493d911d3d7d4
2020-02-17 19:40:15 +07:00
Vadim Yanitskiy d8e5e8bb3b csn1: fix csnStreamDecoder(): update bit_offset in CSN_EXIST{_LH}
Found while doing differential analysis (comparison against the
original implementation from Wireshark).

Change-Id: Ibd0b7400d78f7873c2a8d45267332f511b5c6fbb
2020-02-17 18:35:37 +07:00
Vadim Yanitskiy e87066d01e csn1: fix csnStreamDecoder(): always keep remaining_bits_len updated
Found while doing differential analysis (comparison against the
original implementation from Wireshark).

Change-Id: I9f7fa9c3f2f4ff5213dded930cee7ec509b9d799
2020-02-17 18:35:37 +07:00
Vadim Yanitskiy 584daba8e9 csn1: fix csnStreamDecoder(): do not subtract no_of_bits twice
Found while doing differential analysis (comparison against the
original implementation from Wireshark).

Change-Id: Id2a4f03035cd8354d3fba0ad37571453d3986d21
2020-02-17 18:35:37 +07:00
Vadim Yanitskiy 39a65056da csn1: get rid of C++ specific code, compile with GCC
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-17 02:31:15 +07:00