All vectors should be valid, since they were generated by an MS.
As can be seen, osmo-pcu fails to decode one of the vectors.
Change-Id: I37a2ddd394eeffa1cae0f3e419eeee0200a57fcf
OS#4955
Change-Id: Ib5677048f5668185ffe752f97c97d5612eee4d72
There's actually 3 errors:
* Its value should be updated, not the pointer itself
* Value should be increased, not decreased
* bitvec_read_field() API is already advancing it, no need to do it
Fixes: OS#4838
Change-Id: I009abc373794e148091e637ffee80c6461960945
According to 3GPP TS 44.004, section 7.4a, two alternative RACH block
formats are specified: 8 bit (1 octet) and 11 bit. This change adds
CSN.1 definitions for 11 bit EGPRS Packet Channel Request as per
3GPP TS 44.060, table 11.2.5a.2.
Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Related: OS#1548
I faced a problem while working on EGPRS Packet Channel Request
coding support: the unit test I wrote for it was passing when
compiled with AddressSanitizer, but failing when compiled
without it o_O. Somehow this was observed only with GCC 10.
Here is a part the standard output diff for that unit test:
*** testEGPRSPktChReq ***
decode_egprs_pkt_ch_req(0x2b5) returns 0
- ==> One Phase Access
+ ==> unknown 0xdd5f4e00
decode_egprs_pkt_ch_req(0x14a) returns 0
- ==> One Phase Access
+ ==> unknown 0xdd5f4e00
decode_egprs_pkt_ch_req(0x428) returns 0
- ==> Short Access
+ ==> unknown 0xdd5f4e01
At the same time, debug output of the CSN.1 decoder looked fine.
So WYSINWYG (What You See Is *NOT* What You Get)! As it turned
out, this was happening because I used an enumerated type to
represent the sub-type of EGPRS Packet Channel Request.
typedef struct
{
EGPRS_PacketChannelRequestType_t Type; // <-- enum
EGPRS_PacketChannelRequestContent_t Content;
} EGPRS_PacketChannelRequest_t;
The problem is that length of an enumerated field, more precisely
the amount of bytes it takes in the memory, is compiler/machine
dependent. While the CSN.1 decoder assumes that the field holding
sequential number of the chosen element is one octet long, so its
address is getting casted to (guint8 *) and the value is written
to the first MSB.
// csnStreamDecoder(), case CSN_CHOICE:
pui8 = pui8DATA(data, pDescr->offset);
*pui8 = i; // [ --> xx .. .. .. ]
Let's make sure that none of the existing RLC/MAC definitions is
using enumerated types, and add a warning comment to CSN_CHOICE.
Affected CSN.1 definitions (unit test output adjusted):
- Additional_access_technologies_struct_t,
- Channel_Request_Description_t.
Change-Id: I917a40647480c6f6f3b0e68674ce9894379a9e7f
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
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
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
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
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
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
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
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
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
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
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
Current stderr output is empty anyway, and not checking it allows
enavling different log levels to easily debug issues.
Change-Id: I5b12e919e08a6eeaad31a459e5a15fdee4d76a61
Vadim Yanitskiy
Pau Espin
Vadim Yanitskiy