Age | Commit message (Collapse) | Author | Files | Lines |
|
Change-Id: I6e9d95ef007e984d25403eb64f8aeab02e3fa8e7
|
|
Change-Id: I65a5313704f692c411714006def143cfdb7cc04a
|
|
Change-Id: Ia7f1615edf1cac630631c14ccaeebab758aa4f72
|
|
Change-Id: I4858d1cb0c4ffb0a152b1d975e30d50efa6296a7
|
|
Change-Id: Ib3315f61eb90704b1216094a80e9f379f4150d33
|
|
Remove the paragraph about writing to the Free Software Foundation's
mailing address. The FSF has changed addresses in the past, and may do
so again. In 2021 this is not useful, let's rather have a bit less
boilerplate at the start of source files.
Change-Id: I4a49dbeeec89b22624c968152118aecf8886dac6
|
|
If we decode Exist bit as "1" but we are at the end of the message, and
all the Next items we'd read are expected to be possibly NULL, then swap
the Exist bit in the decoded structure as "0" in order to tell the
decoder user that the related information structure is actually unset,
as if "0" was received.
Related: SYS#5552
Related: OS#4955
Related: OS#5020
Change-Id: I38602e4b680ed87297c7e440691a494c07cad446
|
|
All additional release fields are considered optional, and the
CSN_DESCR for Content_t already marks almost all as such, except
DownlinkDualCarrierCapability_r7.
It has been found that some MS transmits a MS RA Capability with a Length=61 bits
where the last bit in the buffer is setting the Exist bit for
DownlinkDualCarrierCapability_r7 as 1. Hence, the CSN1 decoder failed to
decode the whole message because it expected to keep reading there
despite there's no more bytes to read.
While this is could actually be considered an MS bug, let's relax our
expectancies and simply consider the case { 1 <end> } as it was { 0 },
and mark skip decoding DownlinkDualCarrierCapability_r7. That waht
wireshark (packet-gsm_a_gsm.c) or pycrate do for instance.
This patch itself doesn't fix the problem where actually the Exist bit
is stored as 1 in the output decoded structure, but simply allows keep
ongoing with decoding until the end. This issue will be fixed in a
follow-up patch.
Related: SYS#5552
Related: OS#4955
Related: OS#5020
Change-Id: I9a2541bd3544802a646890f32725201836abb0da
|
|
This RA Cap creaes a decoding error on our CSN1 decoder, but seems to be
handled properly by wireshark's own decoder as well as pycrate.
The ending bit of last byte in "MS RA capability 1" has a "1" which
according to spec should flag the existance of
DownlinkDualCarrierCapability_r7, but nothing else comes after it. This
matches the expectancies as per Length field of the first RA Cap.
Related: SYS#5552
Related: OS#4955
Related: OS#5020
Change-Id: I51235e8575f4b992b44078713ec67bbccfd13293
|
|
This one is larger than some of the other already available.
The decoder is wroking as expected here.
Change-Id: I5d986f68395326f894349446194090b1ddaecd69
|
|
RLCMACTest.cpp:31:30: error: conflicting declaration of ‘const log_info gprs_log_info’ with ‘C’ linkage
31 | extern const struct log_info gprs_log_info;
gprs_debug.h:54:30: note: previous declaration with ‘C++’ linkag
Change-Id: I5922950dd0057bf7eb8578e2144f127082323fc6
|
|
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 no real use of C++ in that file, and it causes problems when
using llist_head entry macros in future patches adding initial support
for multiple BTS in PCU object, so let's move it to plain C.
Change-Id: Ic771a89fd78b5e66151a5384f0ff6a8895589466
|
|
Change-Id: If84c4b3cb870068a85405116f1d505ffcff9c26e
|
|
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
|
|
Change-Id: I6382c81f7569b4c5a68521c04f6b03a34bfc39dd
|
|
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
|
|
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
|
|
The later is only accepted when compiling as c++.
Change-Id: Ib5004643d4eff3bc9d11b66ddaf262f1c0d2aef6
|
|
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
|
|
Change-Id: I6a6e79d7e12cd5e8e969bf0eaa30ddac6b0aa7d3
|
|
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
|
|
Change-Id: I6d1e93cb1a07a7bf05483dbc877105a86a17829b
|
|
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
|
|
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
|
|
Change-Id: Ieec8e6e0823c6f6985f9d343af6d503b8fe9e6ab
|
|
Change-Id: Idf0808461f7e7a1bce58d11a54238c215126451a
|
|
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
|
|
This is how this field is named in Wireshark.
Change-Id: I140443c48af8e4bb1b6279e6de986879b7d9c276
|
|
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
|
|
Change-Id: I48fd701566e1364ce7fccaa3e3a1a0296b932988
|
|
Change-Id: I33f86b79e72394bdb7d99762f8ec21d80e06dc30
|
|
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
|
|
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
|
|
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
|
|
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
|
|
Change-Id: Iabcb768bd714680aa768b35c786dea2015d1e451
|
|
(as they are part of the RlcMacUplink_t structure that is also used to call csnStreamDissector function).
Port from wireshark.git commit 9f8b638cfa8a660fb64c54dcadb83e6747db0a15.
Ported-by: Pau Espin Pedrol <pespin@sysmocom.de>
Change-Id: If46f8cc3f21f527f911dcac6ff1b78f182104a00
|
|
Currently code using that function in osmo-pcu is disabled, allegadly
because SGSN was sending incorrect values, but it looks more like a CSN1
issue.
Related: OS#1525, OS#3499
Change-Id: I92c86397f988afaa791871d823a45fa85054f3bb
|
|
Current stderr output is empty anyway, and not checking it allows
enavling different log levels to easily debug issues.
Change-Id: I5b12e919e08a6eeaad31a459e5a15fdee4d76a61
|
|
Old method takes lots of lines of codes and prints inn unconfortable way
because left-trailing zeros are dropped, making it difficult to split in
bytes.
Change-Id: I56c24f934824e4e52a91a7273aec384b2e15aa67
|
|
Change-Id: Ia0f8cc224a4c38e80699f834fd83d4c0d99322ea
|
|
Otherwise final output is undefined.
Change-Id: I9b501b8a99473b4d79279f8a3a9854e0b2eb3284
|
|
Change-Id: Ib8f0fcbd6bb68d77727c021f0d90d5248e895772
|