According to the 3GPP TS 04.80, table 2.5 'Release complete', a
RELEASE_COMPLETE message may containg optional FACILITY element.
Meanwhile, the 0xff byte is used to indicate that there is no
decoded message (legacy field). Let's avoid overwriting of
a decoded message.
Change-Id: I0c85292222749a48ca0c4b2e93f4fa2d61468c18
The idea is to be able to add a gsm0808_cell_id to a gsm0808_cell_id_list2:
first convert it to a list, then re-use gsm0808_cell_id_list_add(). It will be
used by osmo-bsc to manage neighbor-BSS cell identifiers from VTY.
Change-Id: Ibf746ac60b1b1e920baf494b396658a5ceabd788
There are some symbols for use between control_cmd.c and control_if.c,
which are not supposed to be exported publicly. Let's make sure we
keep those symbols local.
Change-Id: Ia85f36a9c4b2ebf4003718e0a230959638370320
Add a new VTY command which shows all rate counters registered
with libosmocore.
Change-Id: Id60a5aa2d961ae99cddf1e776358a5517dbc573d
Depends: Idb3ec12494ff6a3a05efcc8818e78d1baa6546bd
Related: OS#3245
This new function can be used to print a rate counter group according
to a format string. The intention is to generalize and replace manual
printing of counters as implemented for the 'show statistics' VTY
command of osmo-bsc.
Related: OS#3245
Related: osmo-bsc commit 71d524c059c5a5c90e7cb77d8a2134c1c68b9cde (g#9217)
Change-Id: Idb3ec12494ff6a3a05efcc8818e78d1baa6546bd
For some strange reason, the osmo_mncc_name() inline function
was not in the mncc.h header, but in the mncc.c file. Let's fix that.
Change-Id: I2c3666510c981dffa4ba25bed517fd7ebd1250f5
Fixes following AddressSanitizer report during gea_test run with gcc
8.1.0:
==8899==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffc5f1719bb at pc 0x7fe574adc5fe bp 0x7ffc5f171460 sp 0x7ffc5f171450
WRITE of size 1 at 0x7ffc5f1719bb thread T0
#0 0x7fe574adc5fd in osmo_store64be_ext ../../include/osmocom/core/bit64gen.h:75
#1 0x7fe574adc649 in osmo_store64be ../../include/osmocom/core/bit64gen.h:104
#2 0x7fe574ade936 in _kasumi_kgcore libosmocore/src/gsm/kasumi.c:186
#3 0x7fe574ae2532 in gea4 libosmocore/src/gsm/gea.c:44
#4 0x7fe574ae266c in gea3 libosmocore/src/gsm/gea.c:60
#5 0x7fe574a9b616 in gprs_cipher_run libosmocore/src/gsm/gprs_cipher_core.c:95
#6 0x56422d3fb2ee in test_gea libosmocore/tests/gea/gea_test.c:29
#7 0x56422d3fb506 in main libosmocore/tests/gea/gea_test.c:49
#8 0x7fe5730f406a in __libc_start_main (/usr/lib/libc.so.6+0x2306a)
#9 0x56422d3fadf9 in _start (libosmocore/tests/gea/.libs/lt-gea_test+0x1df9)
The kasumi_test is updated to calculate the entire array of bits
according to expected result. Before this commit it worked by writing
the entire last 64bit block, and addressSanitizer cannot catch it
because the allocated buffer is 64bit aligned too.
Change-Id: I7b2a0224a3b5527d5a3ad7e17efc73081b63eac1
The FSM allows to set individual action callback functions for each
state but it does not allow to leave the action callback pointer
unpopulated. However, there are cornercases where having no callback
function is desirable.
- Check if action callback is popolated before executing it.
Change-Id: I36d221c973d3890721ef1d376fb9be82c4311378
* prefix all symbols/constants with osmo_
* use stdint.h types instead of kernel types
* use Doxygen API documentation
* use Osmocom CRC16-CCITT functions
* use Osmocom bit-reversal functions
* integrate with Automake
Change-Id: I109085ab3e412c20b19cd42fb7137aa0e4167542
I've been importing from 94d7dbf108813ea45a91e27e9a8bd231d5a23fa7
but the isdnhdlc code hasn't seen any changes since 2012 anyway.
Change-Id: I3c58f9cb6921c2fdd0f2fcb11f622a0be88c7c63
Linux offers file descriptor based periodic (interval) timers,
which can achieve a higher precision than our userspace based
timers and which can be slave'd to CLOCK_MONOTINIC or other clock
sources. Let's add some code for osmo_fd wrapped versions that
integrate well with our select() abstraction.
The code has been used in osmo-bts-trx since June 2017 (change-id
I51b19adde14ebb7ef3bb863d45e06243c323e22e), and I'm just renaming
and moving it to libosmocore here. After a merge, the osmo-bts
implementations can be removed in favor if this one.
Change-Id: Ibeffba7c997252c003723bcd5d14122c4ded2fe7
In Change-Id: I8c2c103cdc7f9a45d7b2080c572f559fc3db58e4 we introduced
a check to enforce contention resolution always being used in
MS-originated LAPDm establishment on the main DCCH / SAPI0. This is
only required after RACH request (IMM.ASS.) and not after a normal
assignment command which was sent already via a dedicated channel.
Hence, we cannot enforce a strict requirement for contention resolution
in those cases.
We *could* use the RSL Channel Activation type as a constraint on
whether or not to enforce contention-resoluiton-only LAPDm
establishment, but this is out of the scope of the LAPDm code but would
have to be done inside OsmoBTS.
Related: OS#3252
Change-Id: Id903492ee90809fe98defcf4abc0419b8150069f
The RSL_IE_MS_POWER / RSL_IE_TIMING_ADVANCE is how we communicate
the SACCH L1 header values on the MS side between LAPDm and L3 (which
is a non-standard use of RSL).
However, those IEs only maek sense on the SACCH, where we have B4 frame
format and where we actually have a L1 header containing related
information. Let's make sure to skip those IEs on regular RLL UNIT DATA
INDICATION happening on other channel types.
Change-Id: I6f13e02192531479287f71de674d17ca2ceabdc6
Closes: OS#3249
This is a purely cosmetic clean-up to use the msgb_tv_push() API
to pre-pend a Tag-Value IE to a msgb, rather than the existing
open-coding approach.
Change-Id: I19bbfa1e327a617685ed11d4182e533df33215cb
Add:
- gsm0808_current_channel_type_1()
- gsm0808_permitted_speech()
- gsm0808_chosen_channel()
- gsm0808_channel_type_name()
gsm0808_permitted_speech() is moved from osmo-bsc's bssap_speech_from_lchan();
gsm0808_chosen_channel() is moved from osmo-bsc's lchan_to_chosen_channel();
Rationale: will be re-used by inter-BSC handover, makes sense to keep with the
other gsm0808 utils.
Related: OS#2283 (inter-BSC handover, BSC side)
Change-Id: I8a3cc5d4548e9a78d945d54c69ccced251edcec9
* MO SAPI0 establishment *must always* have L3 payload for contention
resolution
* SAPI3 establishment *must never* use contention resolution
* MT establish must never use contention resolution
Change-Id: I8c2c103cdc7f9a45d7b2080c572f559fc3db58e4
Closes: OS#2370
It seems that during all those years it has never been noted that
the back-pointer from the lapdm_entity to the lapdm_channel was
never initialized. Let's fix that.
Change-Id: Iaca66cd6a2c9f315561e365b51163927868fc346
Sometimes the library probiding dlopen is not the same one providing
dlsym.
This is the case when compiling with AddressSanitizer enabled. In this
case, AC_SEARCH_LIBS([dlopen]...) reports no lib is required, but tests
using dlsym still require to link against -ldl.
Change-Id: Ic619b0885688066b60c97caf1e2c7e5402c1d9f7
Imagine following scenario:
1- client connects to CTRL iface, a new conn is created with POLL_READ
enabled.
2- A non-related event happens which triggers a TRAP to be sent. As a
result, the wqueue for the conn has now enabled POLL_WRITE, and message
will be sent next time we go through osmo_main_select().
3- At the same time, we receive the GET cmd from the CTRL client, which
means POLL_READ event will be also triggered next time we call
osmo_main_select().
4- osmo_main_select triggers osmo_wqueue_bfd_cb with both READ/WRITE
flags set.
5- The read_cb of wqueue is executed first. The handler closes the CTRL
conn for some reason, freeing the osmo_fd struct and returns.
6- osmo_qeueue_bfd_cb keeps using the already freed osmo_fd and calls
write_cb.
So in step 6 we get a heap-use-after-free catched by AddressSanitizer:
[0;m20180424135406115 [1;32mDLCTRL[0;m <0018> control_if.c:506 accept()ed new CTRL connection from (r=10.42.42.1:53910<->l=10.42.42.7:4249)
[0;m20180424135406116 [1;34mDLCTRL[0;m <0018> control_cmd.c:378 Command: GET bts.0.oml-connection-state
[0;m20180424135406117 [1;34mDLINP[0;m <0013> bts_ipaccess_nanobts.c:417 Identified BTS 1/0/0
[0;m[1;36m20180424135406118 [1;34mDNM[0;m[1;36m <0005> abis_nm.c:1628 Get Attr (bts=0)
[0;m[1;36m20180424135406118 [1;34mDNM[0;m[1;36m <0005> abis_nm.c:1628 Get Attr (bts=0)
[0;m20180424135406118 [1;34mDCTRL[0;m <000e> osmo_bsc_ctrl.c:158 BTS connection (re)established, sending TRAP.
[0;m20180424135406119 [1;32mDLCTRL[0;m <0018> control_if.c:173 close()d CTRL connection (r=10.42.42.1:53910<->l=10.42.42.7:4249)
[0;m=================================================================
==12301==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000003e04 at pc 0x7f23091c3a2f bp 0x7ffc0cb73ff0 sp 0x7ffc0cb73fe8
READ of size 4 at 0x611000003e04 thread T0
#0 0x7f23091c3a2e in osmo_wqueue_bfd_cb /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/libosmocore/src/write_queue.c:65
#1 0x7f23091ad5d8 in osmo_fd_disp_fds /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/libosmocore/src/select.c:216
#2 0x7f23091ad5d8 in osmo_select_main /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/libosmocore/src/select.c:256
#3 0x56538bdb7a26 in main /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/osmo-bsc/src/osmo-bsc/osmo_bsc_main.c:532
#4 0x7f23077532e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
#5 0x56538bdb8999 in _start (/home/jenkins/workspace/osmo-gsm-tester_run-prod/trial-896/inst/osmo-bsc/bin/osmo-bsc+0x259999)
Fixes: OS#3206
Change-Id: I84d10caaadcfa6bd46ba8756ca89aa0badcfd2e3
gnutls_global_init must be called at least once for
gnutls < 3.3.0. It doesn't hurt calling it twice, except
a reference counter is increased.
gnutls >= 3.3.0 will call it automatic.
Fixes: OS#2986
Change-Id: I241b6ae5aa8df13dd78f04658cf0953e9561c9e2
3GPP TS 48.058 has a very clear definition of which messages are
"transparent" and hence have the T-bit == 1. This is *not* just
all RLL messages, but basically only RLL_DATA.{ind,req} and
RLL_UNITDATA.{ind,req}. All other messages are non-transparent.
Change-Id: I9f83654af189d818563d799bf623325b7fee8e70
Closes: OS#3188
Catched by AddressSanitizer in osmo-bts-trx while running tests in
osmo-gsm-tester:
==31738==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 5744 byte(s) in 1 object(s) allocated from:
#0 0x7ff7ec789ed0 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1ed0)
#1 0x7ff7e952697c (/lib/x86_64-linux-gnu/libc.so.6+0x10297c)
#2 0x7ff7e95274df in getifaddrs (/lib/x86_64-linux-gnu/libc.so.6+0x1034df)
#3 0x7ff7eadcdc8f in osmo_sockaddr_is_local libosmocore/src/socket.c:537
Change-Id: I778d3c1f162abce0595e62670c29c5134bccd28d
Catched by address sanitizer in osmo-bts-trx during osmo-gsm-tester test
run.
==25503==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55b4e8468780 at pc 0x7fd824f543ba bp 0x7fffc21009f0 sp 0x7fffc21009e8
READ of size 16 at 0x55b4e8468780 thread T0
#0 0x7fd824f543b9 in osmo_get_macaddr libosmocore/src/macaddr.c:132
#1 0x55b4e842df33 in abis_open osmo-bts/src/common/abis.c:256
#2 0x55b4e84286c9 in bts_main osmo-bts/src/common/main.c:342
#3 0x7fd8235ab2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
#4 0x55b4e838e759 in _start (/home/jenkins/workspace/osmo-gsm-tester_run-prod/trial-807/inst/osmo-bts/bin/osmo-bts-trx+0xfc759)
Change-Id: I3727ef339279c8eeb85908735467bfd0e02ca259
Provide comprehensive API to obtain string representations of Cell Identifiers
and -Lists.
Change gsm0808_test.c to use the new functions (which simplifies the output a
bit), so that we don't duplicate printing code in gsm0808_test.c, and so that
the not-so-trivial printing code is also tested.
In gsm0808_test, also test gsm0808_cell_id_list_name_buf()'s return value and
truncation behavior.
The rationale for gsm0808_cell_id_list_name(), i.e. printing an entire list of
cell identifiers, is that even though the maximum is 127 elements, a list of
more than a few elements is hardly ever expected in practice (even more than
one element isn't actually expected: either "entire BSS" or a single LAC). It
is thus useful to log the entire list when it shows up in Paging and Handover.
Change-Id: I9b2106805422f96c5cc96ebb9178451355582df3
According to the GSM TS 04.07, section 11.2.3.1.1 "Protocol
discriminator", bits 1 to 4 of the first octet of a standard
L3 message contain the protocol discriminator IE.
Meanwhile, the GSM48_PDISC_USSD represents value 0x11, i.e.
0b10001, that requires 5 bits, and moreover it is not
documented anywhere. Let's drop it.
Change-Id: Ic4eb8a6db4ff1dfd535bd0c84e7acf1908422f64
don't blindly trust the tag-length value in an IPA CCM ID GET
message. This could result in a remotely-triggered integer underflow.
Change-Id: I4723361e1094b358310541a7dc4c5c921c778a15
Clarify semantics and micro-optimise for the case of single Cell Identifer IEs.
Test in gsm0808_test.c
So far we have gsm0808_enc_cell_id_list2(), but there also exist instances of
single Cell Identifiers (3GPP TS 48.008 3.2.2.17).
It is possible to decode the same using the cell identifier list API, but this
forces the caller to also keep a full struct gsm0808_cell_id_list2 with all its
127 entries around.
E.g. for handover, there are two Cell Identifiers (Serving and Target); I'd
need two full cell id lists for each, and these would be dynamically allocated
for each handover operation, whether it uses them or not.
Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: I9f9c528965775698ab62ac386af0516192c4b0cc
Allow passing multiple struct tlv_parsed in an array, to allow parsing as many
repeated IEs as are expected by the caller.
From tlv_parse(), call tlv_parse2() with dec_multiple = 1 to yield the previous
behavior. tlv_parse() remains valid API.
An example of multiple IEs is the BSSMAP Handover Request, containing Cell
Identifier (Serving) and Cell Identifier (Target), both defined by 3GPP TS
48.008 3.2.2.17 with identical IE tags; both are mandatory.
Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: Id04008eaf0a1cafdbdc11b7efc556e3035b1c84d
This will be used by the upcoming neighbor_ident API in osmo-bsc, where the vty
interface allows composing neihbor BSS cell identifier lists, and we want to
allow adding individual items from individual user commands.
It will also be useful to accumulate cell identifiers in case a subscriber sees
multiple alternative cells from a neighboring BSS, and we want to pass these on
to the MSC in a Handover Required.
Related: OS#2283 (inter-BSC HO, BSC side)
Change-Id: I5781f5fa5339c92ab2e2620489b002829d206925
This will be used by cell idenitifier list code, like upcoming neighbor_ident
VTY in osmo-bsc and regression tests.
Change-Id: Iebc5cdf61b697b1603900993fc265af3eca0cedf
There seems to be quite some confusion / overlap between enum
gsm48_reject_value, gsm48_gsm_cause and gsm48_gmm_cause. I tried to go with
gsm48_gsm_cause_names[], but e.g. GSM48_REJECT_CONGESTION is not represented.
Instead of attempting to mix/merge those enums, provide a separate value string
array for enum gsm48_reject_value.
This will be used by osmo-msc's libvlr (refactoring of FSM result handling),
I27bf8d68737ff1f8dc6d11fb1eac3d391aab0cb1.
Change-Id: I6661f139e68a498fb1bef10c266c2f064b72774a
In the osmo-msc, I would like to set the subscr conn FSM identifier by a string
format, to include the type of Complete Layer 3 that is taking place. I could
each time talloc a string and free it again. This API is more convenient.
From osmo_fsm_inst_update_id(), call osmo_fsm_inst_update_id_f() with "%s" (or
pass NULL).
Put the name updating into separate static update_name() function to clarify.
Adjust the error message for erratic ID: don't say "allocate", it might be from
an update. Adjust test expectation.
Change-Id: I76743a7642f2449fd33350691ac8ebbf4400371d
On erratic id in osmo_fsm_inst_update_id(), don't say "Attempting to allocate
FSM instance".
Escape the invalid id using osmo_quote_str().
Change-Id: I770fc460de21faa42b403f694e853e8da01c4bef
Since alloc relies on osmo_fsm_inst_update_id() to set the name, never skip
that.
In osmo_fsm_inst_alloc(), we allow passing a NULL id, and in
osmo_fsm_inst_update_id(), we set the name without id if id is NULL.
Change-Id: I6d6b09a811b82770818f19b189a57d9fc4a8133b
strcmp() *must not* be passed NULL pointers, or we hit:
../../../src/libosmocore/src/fsm.c:123:8: runtime error: null pointer passed as argument 2, which is declared to never be null
ASAN:DEADLYSIGNAL
(Or, alternatively, a segfault.)
If any of the search string or an FSM instance's name string should be NULL,
simply never match.
Technically, an FSM should never have a NULL name, but a current bug actually
allows this (pass NULL id to alloc), which will be addressed by an upcoming
patch. To test for it, we need to first make sure this here doesn't segfault.
Change-Id: I2e5f82c06d1a4727bd93e955366e3b62b2df1b32
Rationale: with osmo_escape_str(), you get the escaped contents of the string,
but not so graceful handling of NULL strings. The caller needs to quote it, and
for NULL strings not quote it.
osmo_quote_str() is like osmo_escape_str() but always quotes a non-NULL string,
and for a NULL string returns a literal NULL, i.e. it should (tm) give the
exact C representation of a string.
That's useful in testing, to show exactly what char* situation we have, without
jumping through hoops like
if (str)
printf("\"%s\"", osmo_escape_str(str, -1));
else
printf("NULL");
Copy the unit test for osmo_escape_str() and adjust. To indicate that the
double quotes are returned by osmo_quote_str(), use single quotes in the test
printf()s.
I considered allowing to pick the quoting characters by further arguments, but
that complicates things: we'd need to escape the quoting characters. Just
hardcode double quotes like C.
Change-Id: I6f1b3709b32c23fc52f70ad9ecc9439c62b02a12
fix for some spelling issues found by lintian
Signed-off-by: Thorsten Alteholz <osmocom@alteholz.de>
Change-Id: I69976ecae6939d9ff51bfe4ce7374890c6563b82
After investigating osmo-msc showing this log message and looking at the
code, it's a bit difficult to find out what's going on in the code:
socket.c:224 unable to bind socket: (null):0: Protocol not supported
The root cause was not yet found, but probably SCTP is not enabled in
the kernel of the host running it.
The cod eis most probably failing during socket() and not due to bind
error as the log says, so let's print an error if socket() fails.
Then, if setsockopt fails, we want to still keep trying in case an extra
addr was offered by addrinfo_helper. It is definetly wrong to continue
if setsockopt fails, because then we are skipping the bind(), which is a
fundamental part of what osmo_sock_init2 does.
Then, let's print the bind error when it really happens, and re-write
the extra log at the end if we reach the point at which no suitable addr
is found.
Change-Id: I1854422ad92dadf33ed4d849e15c0380c3bf1626
The CTRL interface has a ctrl_cmd_def_* API that allows deferring a CTRL
command reply until later. However, the command handling currently fails to
acknowledge this and deallocates the struct ctrl_cmd anyway.
Fix: in struct ctrl_cmd, add a defer pointer to be populated by
ctrl_cmd_def_make(). A cmd thus marked as deferred is not deallocated at the
end of command handling. This fix needs no change in calling code.
(Another idea was to return a different code than CTRL_CMD_HANDLED when the
command is to be deferred, but that would require adjusting each user of
ctrl_cmd_def_make(). The implicit marking is safer and easier.)
Show that handling deferred commands is fixed by adjusting the expectations of
ctrl_test.c's test_deferred_cmd() and removing the now obsolete exit_early
label.
One symptom of the breakage is that osmo-bts-sysmo crashes when asked to report
a trx's clock-info, which is aggravated by the fact that the sysmobts-mgr does
ask osmo-bts-sysmo for a clock-info.
The crash appears since Id583b413f8b8bd16e5cf92a8a9e8663903646381 -- it looked
like just fixing an obvious memory leak, which it did as shown by the unit
test, but deferred ctrl commands actually relied on that leak. Both fixed now.
Related: OS#3120
Change-Id: I24232be7dcf7be79f4def91ddc8b8f8005b56318
If either an INVOKE, either a RETURN_RESULT component has the
data with incorrect length (see Annex A, 3GPP TS 04.80), the
whole message is probably incorrect.
Let's drop such messages instead of silent truncation.
Change-Id: I2a169b0b84aa26ea2521edd55ff005c27ae6d808
As it was already documented before, the 'ss_request' struct has
a rudiment of deprecated 'ussd_request' struct - the 'ussd_text'
field. It represents the data either of an INVOKE component,
either of a RETURN_RESULT component, encoded as ASCII in case
if DCS is 0x0f (i.e. decoded by the code itself), otherwise
raw bytes 'as is'.
Previously, there was no possibility to distinguish between
ASCII and raw bytes with different DCS. Moreover, the payload
decoding is not desired in some cases.
Let's introduce the new fields, which will carry the raw
unmodified payload, its length and DCS (Data Coding Scheme).
Change-Id: Ia193d175021e145bb3b131290231f307dbefc64a
libosmocore has no value strings for BSSMAP cause codes yet.
- Add value strings for BSSMAP cause codes and a function
to retrieve them
Change-Id: I313dd8d7b06374e1e35ddc18b7a42562d9e25d45
Related: OS#1609
Deprecate osmo_init_logging() for the benefit of adding an explicit talloc
context argument to new function osmo_init_logging2(). Pass a ctx to
log_init() instead of hardcoded NULL.
Before now, *all* of our code uses a NULL ctx for logging, which amounts to
talloc "leaks" hit by address sanitizer builds on newer gcc (e.g. gcc 7.3.0 on
debian 9).
This commit helps fixing "leaks" detected in e.g. osmo-bsc unit tests by a
sanitize build with gcc (Debian 7.3.0-12) 7.3.0.
Change-Id: I216837780e9405fdaec8059c63d10699c695b360
Fix GCC version 7.3.0 (Debian 7.3.0-12) compiler warning:
../../../../src/libosmocore/src/vty/command.c: In function ‘write_config_file’:
../../../../src/libosmocore/src/vty/command.c:2741:2: error: null destination pointer [-Werror=format-overflow=]
sprintf(config_file_tmp, "%s.XXXXXX", config_file);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Check agains NULL after each _talloc_zero() in write_config_file().
While at it, add a comment explaining why we don't use talloc_asprintf() instead.
Change-Id: I7bdc52afe724c1d21f419fe49a6e2ebca9420969
Use non-deprecated API to decode encode in gsm0808_enc_cell_id_list2().
Adjust gsm0808_test.c to now expect the correct results instead of previous
failure.
Change-Id: I1ce78883995e0d484368046b69db5afb2b4adc97
The speech codec defaults are not correct. The defaults recommended
in 3GPP TS 28.062, Table 7.11.3.1.3-2 are limited by 3GPP TS 48.008,
Section 3.2.2.103. Some defaults are actually reserved for future
use. Also the endianess of the 16 bit values is reversed.
- correct values so that they match the specification
- transmit bytes in the correct endianess
Change-Id: I6c3a34d39a375d71c4128fd38f06629e8b98b100
If the name stays the same the log messages will still log with the old
id. Since we can now change the id we need to update the name as well.
NULL as id was allowed before so we should allow that as well.
Change-Id: I6b01eb10b8a05fee3e4a5cdefdcf3ce9f79545b4
This is a more modern way of printing the Abis OML Formatted Object
Header, without assuming that it would be used in a log statement
or prescribing the log level to be used.
Change-Id: I9b2c2afec28882b817d104d5b062651ade7aadd8
Since commit bf383a1d83 tlv_parse()
will return the first occurrence of a repeated IE. Add a test to
verify this behaviour. This test passes with the current code and
fails if bf383a1d83 is reverted.
While here, fix lies in documentation about the return value of tlv_parse()
and fix a typo in another comment.
Change-Id: I041f38548c5e4236920991d6c681c1c1e04de9ca
Related: OS#2904
The implementation was entirely broken, reading data from wrong offsets
and always writing to the first element of the decoded list.
Also, add a new test for this function which found the problems.
Change-Id: If0fafbc7171da2a3044bfa9a167208a1afa1c07b
Related: OS#2847
Depends: Ife4e485e2b86c6f3321c9700611700115ad247b2
Cell ID lists with CI were misparsed because parse_cell_id_ci_list()
failed to report the amount of consumed bytes to its caller.
Also add a regression test which uncovered the bug.
Change-Id: Ife4e485e2b86c6f3321c9700611700115ad247b2
Depends: If6b941720de33dca66b6b1aa2cb95a3275708b7f
Related: OS#2847
This makes gsm0808_dec_cell_id_list() properly decode 3-digit MNCs.
Add a test which encodes/decodes a LAI_AND_LAC list with 3-digit MNCs.
Change-Id: If6b941720de33dca66b6b1aa2cb95a3275708b7f
Related: OS#2847
The cell ID list decoder merged in 11a4d9dd91
has a bug which was introduced part-way through the review process in
gerrit at https://gerrit.osmocom.org/#/c/6509/
When Neels suggested "why not just {...}id_list[MAXLEN] once?" I changed
the cell identifier list from a union of arrays to an array of unions.
After this change, elements smaller than the largest type in the union
were not laid out consecutively in memory anymore. E.g. uint16_t lac
values now occur at offsets of sizeof(id_list[0]) instead of offsets
of sizeof(uint16_t).
The problem is that I forgot to adjust the decoder accordingly, so the
decoder writes to the wrong offsets and returns cell identifier lists
which appear to contain uninitialized values when read back by API
consumers.
I found this problem while adding new regression tests to libosmocore to
test encoding and decoding. This commit adds one such tests for LAC list
decoding, which failed due to the above bug. I plan to write more tests,
however because this first test already uncovered a severe issue I chose
to submit a fix now and work on additional tests in later commits.
Change-Id: Ie1a5a9d858226be578cf11a03cf996d509bd51fb
Related: OS#2847
Global and LAI+LAC cell IDs were being misparsed due to an off-by-one.
This code was incorrectly converted from osmo-bsc, where an additional
offset of one byte was needed to skip the cell identifier field.
In libosmocore, these parsing routines receive a buffer pointer which
is already positioned at the start of the cell identifier field.
Change-Id: I7f3e8ace26176e9cbfe2542961d2a95662aa4d97
Related: OS#2847
Introduce gsm0808_dec_cell_id_list2() with supports additional types of
cell identifier lists. The new parsing routines are based on similar
routines used by the paging code in osmo-bsc's osmo_bsc_bssap.c.
Likewise, introduce gsm0808_enc_cell_id_list2() with support for the
same additional types of cell identifier lists.
The old API using struct gsm0808_cell_id_list is deprecated.
The previous definition was insufficient because it assumed that all
decoded cell ID types could be represented with a single uint16_t.
It was declared in a GSM protocol header (gsm/protocol/gsm_08_08.h)
despite being a host-side representation of data in an IE.
The only user I am aware of is in osmo-msc, where this struct is used
for one local variable. osmo-msc releases >= 1.1.0 make use of this API.
While here, fix a small bug in a test:
test_gsm0808_enc_dec_cell_id_list_bss() set the cell ID type to 'LAC'
but obviously wants to use type 'BSS'.
Change-Id: Ib7e754f538df0c83298a3c958b4e15a32fcb8abb
Related: OS#2847
An internal symbol '_talloc_zero' of talloc library was used
during a msgb allocation. This is not actually good because:
- it may be removed or modified by talloc developers;
- the behaviour may be changed by talloc developers;
- it's marked as internal using 'underscore';
- there is public API to do the same.
So, let's use the public API.
Change-Id: I1080c9071e997944cc0f9fc3716129e9395437ad
Printing an error message when msgb allocation failed was initially
intended, but have been commented out for years. This would
facilitate the bug hunting process, especially on embedded
platforms with limited resources (e.g. amount of RAM).
The GLOBAL logging subsystem with FATAL level is used
for printing such messages.
Change-Id: I3e2d1beabd6936fc28a1ad664c083ff1698bb644
The MSGB API is not a part of OpenBSC anymore, so let's remove
dead includes, which were probably left here during the
migration process.
Change-Id: Ief562a6e5b220a84902f95862d67279f953ee726
In osmo_mnc_from_str() do not try to return some values even if the validation
fails; hence don't try to decode a NULL pointer. That whole idea was half-baked
and a can of worms to begin with.
Change-Id: Ibaaa128ac60b941a015a31134eb52aef56bc6e22
osmo-bsc and osmo-bts share enums and value strings to describe
feature data that is exchanged via OML (manufacturer id) on startup.
Also the functions to set and get the respecitive bits in the feature
bitvectors are in osmo-bsc and osmo-bts. This is a code duplication
and should be resolved.
- add enum osmo_bts_features (replaces enum gsm_bts_features)
- add osmo_bts_features_descs (replaces gsm_bts_features_descs)
- add osmo_bts_set_feature (replaces gsm_btsmodel_set_feature)
- add osmo_bts_has_feature (replaces gsm_btsmodel_has_feature)
Change-Id: Id0c35aef11aa49aa40abe7deef1f9dbd12210776
osmo_mnc_from_str() preserves leading zeros in the string and is useful for
VTY config parsing (osmo-bsc, osmo-msc, osmo-sgsn, osmo-pcu).
osmo_{plmn,mnc}_cmp() takes care of the slight intricacy of ignoring the 3-digit flag
if the MNC is anyway >99. Will be used by osmo-sgsn.git and osmo-bsc.git. (All
current users just care about identical MNC, but a proper cmp doesn't hurt.)
Change-Id: Ib7176b1d65a03b76f41f94bc9d3293a8a07d24c6
Enable representing three-digit MNC with leading zeros. The MNCs 23 and 023 are
actually different; so far we treated both as 23. Re-encode an incoming BCD or
string of 023 as it were, i.e. not dropping the leading zero as 23.
Break ABI compatibility by changing the size and ordering of structs
gprs_ra_id, osmo_plmn_id, osmo_cell_global_id, ... by adding an mnc_3_digits
flag.
Change ordering in gprs_ra_id because the canonical oder is {Mobile Country
Code, Mobile Network Code}, so have the mcc member first.
ABI compatibility cannot be maintained for struct gprs_ra_id, since it is a
direct member of structs bssgp_bvc_ctx and bssgp_paging_info, and even just
adding a flag to the end would cause ABI changes of those structs. Similarly,
osmo_plmn_id is a direct member of osmo_location_area_id, and so forth.
Add new API to set and read this additional flag to preserve leading zeros:
- osmo_plmn_to_bcd(), osmo_plmn_from_bcd() after
gsm48_mcc_mnc_to_bcd() and gsm48_mcc_mnc_from_bcd().
- gsm48_decode_lai2(), gsm48_generate_lai2() after
gsm48_decode_lai(), gsm48_generate_lai().
- gsm0808_create_layer3_2() after gsm0808_create_layer3() and gsm0808_create_layer3_aoip().
- various osmo_*_name() functions in gsm23003.h (osmo_rai_name() still in
gsm48.h close to struct gprs_ra_id definition). The amount and duplication of
these may seem a bit overboard, but IMO they do make sense in this way.
Though most code will soon see patches unifying the data structures used, in
some cases (vty, ctrl) they are required singled out. Without these
functions, the formatting ("%0*u", mnc_3_digits ? 3 : 2, mnc) would be
duplicated all over our diverse repositories.
In various log output, include the leading MNC zeros.
Mark one TODO in card_fs_sim.c, I am not sure how to communicate a leading zero
to/from a SIM card FS. The focus here is on the core network / BSS.
To indicate ABI incompatibility, bump libosmogsm and libosmogb LIBVERSIONs;
adjust debian files accordingly.
Implementation choices:
- The default behavior upon zero-initialization will be the mnc_3_digits flag
set to false, which yields exactly the previous behavior.
- I decided against packing the mnc with the mnc_3_digits field into a
sub-struct because it would immediately break all builds of dependent
projects: it would require immediate merging of numerous patches in other
repositories, and it would make compiling older code against a newer
libosmocore unneccessarily hard.
Change-Id: Id2240f7f518494c9df6c8bda52c0d5092f90f221
If no event names are defined for an FSM, show a placeholder
message which points out the problem instead of segfaulting.
Change-Id: I87457945a7b76aa052305c9c531722be1ea0c1d1
Related: OS#3007
Event names are displayed in VTY commands so all FSM should have them.
Print an error message if an FSM is registered without event names.
We could also return an error code, however at present no caller checks
the return value of osmo_fsm_register() so this would be pointless.
Add event names to the test FSM and update expected output accordingly.
Change-Id: I08b100d62b5c50bf025ef87d31ea39072539cf37
Related: OS#3008
For all other decode operations we report the BER, but not for the
RACH. This results in osmo-bts-trx not being able to report BER
to the higher layers, which is possible on other BTS backends.
Let's close this gap by introducing gsm0503_rach_ext_decode_ber()
and gsm0503_rach_decode_ber() with the usual n_errors / n_bits_total
arguments.
Change-Id: I2b1926a37bde860dcfeb0d613eb55a71271928c5
There is a desire to install osmo_fsm vty commands automatically in
a library context, rather than requiring every application which
directly or indirectly uses osmo_fsm to run osmo_fsm_vty_add_cmd().
However, the function install_element_ve() asserts that elements
about to be installed have not already been installed.
This means we cannot shift responsibility into a library context
without first making sure that osmo_fsm commands are only installed
once per combined application+library context, because applications
won't know which commands any of its libraries has already installed.
A simple solution is to use a global flag which is checked by
osmo_fsm_vty_add_cmd() before installing osmo_fsm commands, and
is set once the commands have been installed. This way, no harm
is done if osmo_fsm_vty_add_cmd() is called multiple times.
Change-Id: I10b0b1c1c1bf44c3b8eafc465c1ee06ea2590682
Related: OS#2967
This breaks all existing / older osmocom-bb builds, and hence
cannot be accpeted. See also https://gerrit.osmocom.org/#/c/6679
Related: OS#2985
This reverts commit 3c38e60cd5.
Change-Id: Icfc52ca4e5cbe3a444d98037d27fa101e3614e06
The function _osmo_fsm_inst_term() terminates all child FSMs befor
it calls fi->fsm_cleanup(). This prevents the cleanup callback to
perform last actions on the child FSMs (e.g.
osmo_fsm_inst_unlink_parent()).
- Since moving the cleanup callack to the beginning of the function
would alter the termination behavior and possibly cause malfunction
in already existing implementation that use OSMO fsm, a new
optional callback that is called immediately at the beginning of
the terminatopn process is added.
Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Closes: OS#2915
gsmtap_sendmsg() does not free the msgb if it returns a failure rc, so the
callers must check the rc and free the msg.
Change-Id: I7cf64ed9b14247298ed8b4ab8735627f8235a499
If less than the msgb size was written by write(), we want to return -EIO.
Hence do not return zero when write() wrote zero bytes, return -EIO in that
case as well.
Previously, if write() returned zero, gsmtap_sendmsg() would return zero
*without* freeing the msg, hence neither would the (ideal) caller. So this
fixes a corner-case memleak.
Change-Id: I099ae1c663c018da5db884f7e9d52c45af3ed817
Not freeing on error does enable callers to try to re-send as well, so it is a
kind of useful feature, even though I find it likely for callers to either
forget about freeing the msg on error or double-free by accident...
I considered changing gsmtap_sendmsg() to always free, but since it is public
API, I chose to keep and document its current behavior properly instead. We
don't know what callers may exist out there.
Change-Id: Id3266ce36442024f16eaf6afa3f516d201930c41
Sometimes we want to create an FSM instance before we know its name. In
that case we should be able to update the id later.
Change-Id: Ic216e5b11d4440f8e106a297714f4f06c1152945
Add generic function which allows caller to set Mobile Identity
explicitly. This allows to use IMEI or IMEISV for example. Make
gsm48_generate_mid_from_imsi() into wrapper around new function.
Change-Id: Id79be7abfff75ecd0d248bbeed93e605abeec9b3
This reverts commit 5ec91980ac.
More or less like I expected, it creates fall-out. osmo-msc master builds are failing, as are the open build service builds. The patch has therefor *not* been sufficiently tested.
Change-Id: I8d961d7bbd91b6a8d7691f24cb67720c3d001c7e
The function _osmo_fsm_inst_term() terminates all child FSMs befor
it calls fi->fsm_cleanup(). This prevnts the cleanup callback to
perform last actions on the child FSMs (e.g.
osmo_fsm_inst_unlink_parent()).
move the function call to _osmo_fsm_inst_term_children() below the
call to fi->fsm->cleanup().
Change-Id: Ie89d435417306c6bf897274eabc3ed0a46485c26
Most GSM related specifications require the receiver to use the
*first* occurrence of repeated IEs. The Osmocom TLV parser so
far did the opposite: It reported only the *last* occurrence in
case of repeated IEs. Let's change our implementation to be
more in-line with relevant specs, such as 3GPP TS 24.008 8.6.3.
Change-Id: Icde09e075f68c842a7a96cf7160c8e44b77cf82d
In If1bd79026a3c680ccf7587d545d12f7759a998fc, an erratic logging output crept
in for an earlier patch state and was merged by accident; fix 'logging print
file (0|1|basename)' output.
Add value string to map LOG_FILENAME_* enum to VTY args, use for both command
evaluation as well as printing the vty config.
The default is 'logging print file 1', hence we could omit an output when '1'
is chosen. But for clarity, always output the current setting.
Change-Id: I1c931bff1f1723aa82bead9dfe548e4cc5b685e0
* match return type of osmo_gsup_encode() with osmo_gsup_decode() to allow
propagating error to caller
* check return value of osmo_gsup_encode() in GSUP test
* return errors instead of braking app with aseert
Change-Id: Idaa1deecb6d9e15329bd51867b4f6a03357461f0
Related: OS#2864
Add wrapper for osmo_strlcpy() which uses sizeof() to automatically
determine buffer's size and use it for GSMTAP logging. This is pretty
common use case for osmo_strlcpy() so it's a good idea to save some
typing by using generic define.
Related: OS#2864
Change-Id: I03d0d3d32a8d572ad573d03c603e14cdc27a3f7b
Some times I *really* regret ever having merged OSMO_VALUE_STRING,
as it generates completely unusable and way too long strings :(
Change-Id: I8de7c01f9ea1d66c384e57449c4140186f5ce6c5
At the moment it is not possible to unlink a child from from
its parent, nor is it possible to assign a new parent to a
child FSM.
- osmo_fsm_inst_unlink_parent():
Make it possible to unlink childs from a parent.
- osmo_fsm_inst_change_parent():
Make it possible to change the parent of a child.
Change-Id: I6d18cbd4ada903cf3720b3ad2a89fc643085beef
Since commit e094157e12, TCH frame
length definitions were added to libosmocodec.
No need to define them again.
Change-Id: Id8c6132534e36ea1e368432bb259fd4f3a531f90
The function inet_ntoa() stores its result in a static buffer and
returns the pointer. When inet_ntoa() is called subsequently it
overwrite the content of its static buffer with the new result.
Since we osmo_sock_local_ip() is a library function we should use
the more safe variant inet_ntop() in order to prevent unintentionally
overwriting data that the caller might still need. Such an error
would be hard to find.
- Use the more safe inet_ntop() inestead of inet_ntoa()
Change-Id: I9852b57736432032542bd96b6fdd4a2f08fc1f64
The socket that is opend to probe the correct local ip-address is
not closed when the test is done.
- Close socket when it is not needed anymore
Change-Id: I7f3562a344b58f6298d2068314be1626a96e1b1d
As MNCC is rather hard to debug (wireshark cannot trace UNIX domain
sockets), let's add our own decoder that we can use from related
debug log statements in the respective programs.
Change-Id: I216aaf70868ba5f3860a60c4b2442957531a3011
Add a VTY command that allows configuring the output of source filename. So
far, this was not configurable by VTY at all.
Change-Id: If1bd79026a3c680ccf7587d545d12f7759a998fc
In the C API, add another enum log_file_type value, and when set print only the
basename of the source file path.
Rationale: especially when not building directly in the source dir, the paths
to the source files can become rather long. Usually, just the basename of the
file is sufficient to identify the source line.
Change-Id: If3e4d5fb2066f8bf86e59c82d1752b1a843cf58e
Add a separate flag and API to switch the category-in-hex output:
log_set_print_category_hex().
Add log_set_print_filename2() to modify only the print_filename flag. The old
log_set_print_filename() function still affects both flags. Explain the
rationale in the comment for log_set_print_filename().
There is no need to deprecate log_set_print_filename(); it might cause compiler
warnings and break strict builds unnecessarily.
Add VTY command 'logging print category-hex (0|1)'.
Since there is no VTY command to switch filename output, nothing needs to be
adjusted there (a command will be added in a subsequent patch).
Change-Id: Iba03a2b7915853c6dccaf6c393c31405320538b4
If color output is disabled, skip the empty snprintf() to (not) clear the ANSI
color.
Also, no need to use a format string of "%s", just pass the string constant
directly.
That is a micro optimisation as well as clarification of the code.
Change-Id: Ie7cb06de160830d2f8ee5718246c0fe311f68d49
llist_del(&fi->proc.child) is executed always, regardless whether
a parent is configured or not. This may lead into a double llist_del
when the child has been previously unlinked.
- check if fi->proc.parent is set, and only then execute
llist_del(&fi->proc.child);
Change-Id: I4b33d508c8a11b72fbf30125088a882894d9e6ac
A recent commit added an snprintf that passes a pointer to a literal directly
to snprintf. Since passing pointers to printf formats is a vulnerability in
case user supplied data may be passed in the format, modern compilers warn
against that, which breaks our -Werror builds. Even though this is just a
pointer to a literal, it needs to be an actual literal to make compilers happy.
Use printf("%s", c) instead of printf(c).
Note that our current build slave's gcc does not enforce that yet, while newer
compilers do.
logging.c:338:4: warning: format not a string literal and no format arguments [-Wformat-security]
ret = snprintf(buf + offset, rem, c_subsys);
Change-Id: Ifa4eb8a9fab66dcd987986065351b4a06421f1ec
When log_set_use_color() is enabled, color the log category string according to
the log level. The log line before and after the log category is printed in the
category's configured color.
ERROR and FATAL are red, NOTICE is yellow, INFO is green and DEBUG is blue.
The default behavior remains unchanged; If color is enabled, the category
string will now always be colored in the log level color, not the log category
color, and will stand out from the rest of the line.
Change-Id: I84f886ac880e9056a666bbb231ae06cbaaf65f44
When log_set_use_color() is enabled, color the log level string according to
the log level. The log line before and after the log level is printed in the
category's color.
ERROR and FATAL are red, NOTICE is yellow, INFO is green and DEBUG is blue.
The default behavior remains unchanged.
Change-Id: If2e52ae9ab83e538e04321c338e3fdffb2c7f9d3
Log the log level string after the category name, if enabled.
The default behavior remains unchanged.
Change-Id: Ie6be365cfa6aeabdf115bff19bac198440c9adf1
According to GSM 04.80 section 2.5 "Release complete", a message
of the mentioned type may contain optional IEs, such as Cause
and Facility. Let's parse them.
Change-Id: Ib8fc1f6bae472b0b264b6158f372b6cce255b222
Some SS messages (e.g. RELEASE COMPLETE) may contai multiple
IEs (Information Elements). Let's parse them all.
Change-Id: I20cc59c25fdbda176bcf76437174cda829518d60
According to GSM 04.08, 4.4.2 "ASN.1 data types":
the USSD-DataCodingScheme shall indicate use of
the default alphabet using the 0x0F value.
Previously, the UnstructuredSS Request messages with not
default alphabet were not being handled. Let's fix this.
Change-Id: I73d602f6f20b0afe7600d16bbd432069ae7be788
According to the GSM 04.80 (version 5.0.0) specification Annex A
"Expanded ASN.1 Module "SS-Protocol", the maximum size of a USSD
OCTET STRING is 160 bytes.
Thus according to ETSI TS 123 038 (version 10.0.0) specification
6.1.2.3 "USSD packing of 7 bit characters", in 160 octets, it's
possible to pack (160 * 8) / 7 = 182.8, that is 182 characters.
The remaining 6 bits are set to zero.
This change defines both mentioned values:
- GSM0480_USSD_OCTET_STRING_LEN 160
- GSM0480_USSD_7BIT_STRING_LEN 182
keeping the old MAX_LEN_USSD_STRING 'as is' due to compatibility
reasons. Now the new value is used for ss_request structure, while
old one is still used for deprecated ussd_request structure.
Change-Id: I6dead74f9ecea079752ff2400cdaf7c30187784e
According to GSM 04.80 Section 2.5 'Release complete' Table 2.5,
the 'RELEASE COMPLETE' message payload is optional, so let's drop
the length check in gsm0480_decode_ss_request() for this type.
Change-Id: I63b7f8ce403169a9dbdbdb031db16693de2196d6
It's not very useful to get just the raw pointer address in case of
lapd_datalink receive error. Log it's state in addition.
Change-Id: Ie8c5df262312f886f509113f2707e36811df3bd5
When a bad GSM voice frame is received, it's being replaced
by a silence frame. This may cause unpleasant audio effects.
This change implements a functionality to craft a replacement
frame from the last known good frame. Currently, only FR is
supported, support for other codecs may be added latter.
Change-Id: I06a21f60db01bfe1c2b838f93866fad1d53fdcd1
GSMTAP doesn't have a lot of space for the source file name. It is better to
send only the basename of the file, because only the first bit of a long path
may not convey the source file at all, needing guess work from the line number.
Before: "Source File Name: ../../../../src/libosmocore/src"
After: "Source File Name: telnet_interface.c"
Change-Id: Ie8fc9e782bcf8fa6e2e957d02e7d73c3a7c2bca8
Previously we've checked for existing log target with a given hostname
from vty code but it was ignored inside the check so only the very first
'log gsmtap' entry was enabled while the rest were silently ignored.
Change-Id: I8fd8bda9e07d403a54735da30addb742e56538a2
Print which function has triggered assert_loginfo(). It's handy in
debugging logging-related issues in libosmocore.
Change-Id: I8418d0c431106f50aa8779cd89396f02373304ad
If the length provided in the patcket exceeds the buffer length,
tlv_parse() returns -2 but leaves tlv.val and tlv.len initializd.
Many callers of tlv_parse() do not check its return value, but
rely on TLVP_PRESENT() to see if a particular TLV was parsed
successfully. By clearing tlv.val and tlv.len we make it less
likely that those callers will use an overlong TLV length value.
Change-Id: I4dda6938e1650b4bcaac45809a4763f86f5a9794
Add gsm48_encode_ra() which takes appropriate struct as [out] parameter
instead of generic buffer. Using uint8_t buffer instead of proper struct
type prooved to be error-prone - see Coverity CID57877, CID57876.
Old gsm48_construct_ra() is made into tiny wrapper around new
function. The test output is adjusted because of the change in function
return value which was constant and hence ignored anyway.
Related: OS#1640
Change-Id: I31f9605277f4945f207c2c44ff82e62399f8db74
As a leftover from code move from OsmoBTS we have eB adjustement outside
of eB check in gsm0503_tch_burst_map() which is rightfully noted by
Coverity. Let's fix this by moving the adjustement under the
corresponding if.
Change-Id: I385cd6ffea4d13ef911910fc87c92b73809888a2
Fixes: CID57691
Previously the were no default value provided in case of unspecified
hostname in "log gsmtap" vty config. This leads to confusing log
messages because NULL was used as a hostname:
Inconsistent indentation -- leading whitespace must match adjacent lines, and
indentation must reflect child node levels. A mix of tabs and spaces is
allowed, but their sequence must not change within a child block.
Fix this by using 127.0.0.1 as default log destination and logging
hostname in case of errors.
Related: OS#2608
Change-Id: I58b1d4ec522af18024be2e56c9103b3db7936813
Now that we use osmo_sock_get_name() to print connection information
at disconnect, let's use the same also at accept() time.
Furthermore, let's call it CTRL connection everywhere for consistency.
Change-Id: I33ee7d0ed853c5b2a4ae4e8ef945f8f27753cdea
When read() or write() system calls return '0' on a stream socket,
it means that the connection has been closed ("EOF"). We must
accordingly close this socket and remove all related state.
Before this patch, every new CTRL connection would introduce a leak
of both some memory/state, as well as a file descriptor :(
Change-Id: I4fb70e5f123b37dece29f156c5f430c875e7cbaf
So far, error reporting just says "Trap/Reply", more accurately report 'GET
REPLY', 'SET REPLY' and 'TRAP' as appropriate.
Change-Id: Ic25a251502499aeda4e2952ec4190a1fa0bebb01
Validate that incoming CTRL commands...
- have decimal IDs,
- return error on trailing characters,
- have invalid characters in variable identifiers,
- send detailed error messages as reply to the requestor.
Adjust ctrl_test.{c,ok}, which best show the change in behavior.
Message handling causes log messages on stderr; previously, stderr was empty.
Add '[ignore]' in testsuite.at so that the nonempty stderr doesn't cause test
failures.
Change-Id: I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1
Recent patch I563764af1d28043e909234ebb048239125ce6ecd introduced returning
NULL from rate_ctr_group_alloc() when the index passed already exists.
Instead of returning NULL, find an unused group index and use that, adjust the
error message.
In stats_test.c, adjust, and also assert allocated counter group indexes
everywhere.
Rationale:
The original patch causes osmo-sgsn to crash as soon as the second subscriber
attempts to establish an MM context. Of course osmo-sgsn is wrong to a) fail to
check a NULL return value and crash and b) to fail to allocate an MM context
just because the rate counter group could not be allocated (it still rejects
the MM context completely if rate_ctr_group_alloc() fails).
Nevertheless, the price we pay for rate counter correctness is, at least in
this instance, way too high: osmo-sgsn becomes completely unusable for more
than one subscriber.
Numerous other places exist where rate_ctr_group_alloc() is called with a
constant index number; from a quick grep magic I found these possible breaking
points:
osmo-sgsn/src/gprs/gb_proxy.c:1431: cfg->ctrg = rate_ctr_group_alloc(tall_bsc_ctx, &global_ctrg_desc, 0);
osmo-sgsn/src/gprs/gprs_sgsn.c:139: sgsn->rate_ctrs = rate_ctr_group_alloc(tall_bsc_ctx, &sgsn_ctrg_desc, 0);
osmo-sgsn/src/gprs/gprs_sgsn.c:270: ctx->ctrg = rate_ctr_group_alloc(ctx, &mmctx_ctrg_desc, 0);
osmo-sgsn/src/gprs/gtphub.c:888: b->counters_io = rate_ctr_group_alloc(osmo_gtphub_ctx,
>phub_ctrg_io_desc, 0);
osmo-bsc/src/libfilter/bsc_msg_acc.c:87: lst->stats = rate_ctr_group_alloc(lst, &bsc_cfg_acc_list_desc, 0);
osmo-pcu/src/bts.cpp:228: m_ratectrs = rate_ctr_group_alloc(tall_pcu_ctx, &bts_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:793: tbf->m_ctrs = rate_ctr_group_alloc(tbf, &tbf_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:879: tbf->m_ul_egprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_ul_egprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:880: tbf->m_ul_gprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_ul_gprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:970: tbf->m_dl_egprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_dl_egprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:977: tbf->m_dl_gprs_ctrs = rate_ctr_group_alloc(tbf, &tbf_dl_gprs_ctrg_desc, 0);
osmo-pcu/src/tbf.cpp:1475: ul_tbf->m_ctrs = rate_ctr_group_alloc(ul_tbf, &tbf_ctrg_desc, 0);
osmo-pcu/src/bts.cpp:226: m_ratectrs = rate_ctr_group_alloc(tall_pcu_ctx, &bts_ctrg_desc, 1);
We can fix all of these callers and then reconsider returning NULL, but IMO
even into the future, rate counter group indexes are not something worth
failing to provide service for. For future bugs we should keep the automatic
index picking in case of index collisions. We will get an error message barfed
and can fix the issue in our own time, while the application remains completely
usable, and even the rate counters can still be queried (at wrong indexes, but
life is tough).
Related: I49aa95b610f2faec52dede2e4816da47ca1dfb14 (osmo-sgsn's segfault)
Change-Id: Iba6e41b8eeaea5ff6ed862bab3f34a62ab976914
The recently added ctrl_cmd_parse2() returns non-NULL cmd with error messages
upon parsing errors. In handle_control_read(), use ctrl_cmd_parse2() and send
those back to the CTRL command sender as reply.
Retain the previous "Command parser error" reply only in case ctrl_cmd_parse2()
should return NULL, which shouldn't actually happen at all.
Change-Id: Ie35a02555b76913bb12734a76fc40fde7ffb244d
If a control command fails to parse, we so far discard specific error messages
and instead send just "Command parser error".
In ctrl_cmd_parse() we actually compose detailed error replies, but in the end
simply talloc_free() them and return NULL.
A first step to report these errors to the ctrl command issuer is to not return
NULL and instead return the cmd with type = CTRL_TYPE_ERROR. Add
ctrl_cmd_parse2() to return such instead of NULL.
To stay API compatible, provide ctrl_cmd_parse2() to return a cmd on errors.
ctrl_cmd_parse() retains identical behavior but becomes just a simple wrapper
around ctrl_cmd_parse2() which discards the cmd on error.
No need really to deprecate ctrl_cmd_parse() yet; especially as long as
compiler warnings might break jenkins builds.
Change-Id: I5047c9f977d70b03eea77cbcfd2b96d43ea46880
In ctrl_handle_msg() (code recently propagated from handle_control_read()),
talloc_free() the parsed ctrl_cmd in all code paths. In particular, a free was
missing in case ctrl_cmd_handle() returns CTRL_CMD_HANDLED.
CTRL_CMD_HANDLED is triggered by GET_REPLY / SET_REPLY parsing, as show by
ctrl_test.c. With the memleak fixed, adjust expected test output and make a
detected mem leak abort the test immediately.
Change-Id: Id583b413f8b8bd16e5cf92a8a9e8663903646381
In order to allow unit testing the ctrl iface msgb handling, have a separate
msgb entry point function from the actual fd read function.
An upcoming patch will prove a memory leak in CTRL msgb handling by a unit test
that needs this separation.
Change-Id: Ie09e39db668b866eeb80399b82e7b04b8f5ad7c3
To report invalid characters in identifiers, it is desirable to escape any
weird characters. Otherwise we might print stray newlines or control characters
in the log output.
ctrl_test.c already uses a print_escaped() function, which will be replaced by
osmo_escape_str() in a subsequent patch.
control_cmd.c will use osmo_escape_str() to log invalid identifiers.
Change-Id: Ic685eb63dead3967d01aaa4f1e9899e5461ca49a
To send a Ciphering Mode Command, we may need to derive a Kc from UMTS AKA
tokens. gsm_milenage() derives Kc from 3G tokens, but also derives an SRES.
For SRES, it requires an OPC, which may need to be derived from OP first. All
we need is a Kc, so we could feed a zero OPC ... but to simplify the function
call for cases where just a Kc is required, separate the c3 function out from
gsm_milenage(), as osmo_auth_c3(). Obviously call osmo_auth_c3() from
gsm_milenage() (meaning that osmo-hlr's 55.205 derived auc tests still cover
exactly that implementation).
Prepares: If04e405426c55a81341747a9b450a69188525d5c (osmo-msc)
Related: OS#2745
Change-Id: I85a1d6ae95ad9e5ce9524ef7fc06414848afc2aa
For validating CTRL input, we want to verify that an input variable is a series
of valid osmo_identifier_valid() separated by dots. Allow validating any
additional chars with identifiers, for CTRL vars will be just ".".
Change-Id: I13dfd02c8c870620f937d789873ad84c6b1c45de
Check that no group with the given name and index already exist before
allocating it. Add corresponding test case.
Change-Id: I563764af1d28043e909234ebb048239125ce6ecd
Related: OS#2757
The Cause IE in the 08.08 CIPHER MODE REJECT is a normal TLV IE,
and not just a value. Let's make sure we encode the cause value
properly.
Change-Id: I4f5b231edf6dcb0a9c2bbafb2a59f301f3b2402b
Closes: OS#2766
Some Abis/RSL messages such as "Release Indication" contained 3 extra
bytes from an L3 Information header which should not be there according
to specs in GSM 08.58 (section 8.3 "Radio link layer management
messages"). Other RSL messages were affected by the same issue, except
for "Establish Indication", which had already a workaround in
send_rslms_dlsap.
This commit fixes the issue in a generic way, removes the "Establish
Indication" and fixes the test accounting for the bug, as it otherwise
fails after applying the changes.
Fixes: OS#1635, OS#2336
Change-Id: Ibb116214e8b1798d65a8b0917150496a3c14f344
In 'show online-help' output, add the node names (currently all derived from
the prompt) as <node><name> entry, so that in the osmo-gsm-manuals, each
section of node commands gets a title. So far, each section of commands has no
name at all, and it is entirely up for guessing which part of the VTY the
commands are about.
Node section names, e.g. for OsmoHLR, will be like
1 VTY reference
1.4 config
1.5 config-log
1.6 config-line
1.7 config-ctrl
1.8 config-hlr
1.9 config-hlr-gsup
Before this patch, all but '1 VTY reference' were plain empty.
A better solution would be to list the actual command name that enters the
node, and to nest the commands identically to VTY node nesting, but since this
information is currently hidden in node command implementations, it is
impossible to derive it. So we should actually make the VTY reflect the node
nesting structure in its data model, which would resolve both the accurate node
name problem as well as produce well-structured output to generate the VTY
references from. This patch is a workaround for lack of a more profound fix of
the VTY data model. At least it makes the VTY references' sections even
remotely useful.
Change-Id: Iaf745b2ab3d9b02fc47025a0eba3beb711068bfe
We use 'show online-help' to generate VTY reference manuals. It is not helpful
to include the common node commands on each and every node level, it clutters
the actual useful help.
Have a separate first section called 'Common Commands', but omit them
elsewhere.
Change-Id: Ie802eccad80887968b10269ff9c0e9797268e0d4
When creating asciidocs for osmo_counter an empty is not useful.
If there aren't any counter, output a hidden comment
Change-Id: Ie2768100e69dcd7d8d77533688585dd9b43c4a5e
Previously ctrl request for all counters in
group (e. g. 'rate_ctr.abs.msc.0') will result in human-readable
description which is not regular enough and is hard to both parse and
generate. The ctrl interface is intended for m2m, not for human
interaction. Let's simplify things by making response similar to counter
group request ('rate_ctr.*').
Reply now looks as follows:
GET_REPLY 9084354783926137287 rate_ctr.abs.msc.0 loc_update_type:attach 0;loc_update_type:normal 0;
Previously it was:
GET_REPLY 9084354783926137287 rate_ctr.abs.msc.0 All counters in msc.0
loc_update_type:attach 0
loc_update_type:normal 0
Change-Id: I7a24cc307450efdcd28168fffe477320c59fcd36
Related: OS#2550
When calling the timer_cb, that may have effected an fi termination and
deallocation, e.g. from dispatching events and/or complex choices made.
Current timer_cb implementations expect T to reflect the fired timer number, so
we can't actually set T=0 before calling the timer_cb.
Instead, never reset T to zero, let it always reflect the timer that last
fired. When a new timer starts, T will be set to its new value.
Adding a T arg to the timer_cb() would have been the cleanest solution, so that
fi->T can be set to zero before dispatching the timer_cb. But since we've
already rolled out this FSM API, we should stay backwards compatible.
In the case where the timer returned 1 to request termination, we can assume
that the fi still exists, but to be consistent, don't set T = 0 in that code
path either.
Change-Id: I18626b55a1491098b3ed602df1b331f08d25625a
This should never happen with the current code, but if it ever does, we
should log the error instead of silently returning 0.
Change-Id: I544001d3072e5f12a96a67e4178f9b945c5f6b6c
Related: OS#2550
Before user have to know group name and index in advance to request rate
counter value. Introduce introspection function which allows user to
obtain all the groups and their indexes by requesting 'rate_ctr.*'
variable.
This simplifies KPI dumping over ctrl interface.
Change-Id: Ifad8b4f0360c8bcd123a838676516476e84c246a
Related: OS#2550
Some callers pass NULL and len == 0. The semantics are that we then
nul-terminate an emtpy string. Avoid a sanitizer warning by not calling
memcpy() for the NULL case.
Change-Id: I883048cf2807e606c6481634dbd569fc12aed889
All successful and all error code paths of bssgp_fc_in() free the msgb, except
the code path calling fc_enqueue() when the msg is dropped (due to queue being
full, or failure to allocate).
Callers could theoretically catch the -ENOSPC return value and discard the
msgb. However, in other code paths, a callback's return value is returned,
which is expected to free the msgb, so such callback would have to never return
-ENOSPC when it freed the msgb. Much simpler semantics would be to free the
msgb in every code path, no matter which kind of error occurred.
Who is currently calling bssgp_fc_in and how do they handle the return value?
- bssgp_fc_test.c ignores the return value (and hits a mem leak aka sanitizer
build failure if the queue is full).
- fc_timer_cb() ignores the return value.
- bssgp_tx_dl_ud() returns the bssgp_fc_in() rc.
- which is returned by a cascade of functions leading up to being returned,
for example, by gprs_llgmm_reset(), which is usually called with ignored
return code.
At this point it is already fairly clear that bssgp_fc_in() should always free
the msgb, since the callers don't seem to distinguish even between error or
success, let alone between -ENOSPC or other errors.
bssgp_fc_test: assert that no msgbs remain unfreed after the tests.
Adjust expected results.
Helps fix sanitizer build on debian 9.
Change-Id: I00c62a104baeaad6a85883c380259c469aebf0df
Using the NULL context creates mem leaks that bother sanitizer builds.
Allocate as talloc "child" of the rate_ctr_group, so that the mangled desc (if
any) gets freed when the rate_ctr group is freed.
Remove the comment concerning osmo-msc: the way to fix the unexpected talloc
state in osmo-msc tests is to have no invalid rate counter names in osmo-msc.
See Ib1db8e3dc6c833174f1b0b1ca051b0861f477408 (osmo-msc).
Change-Id: Ief9abfeb78b7706200bcc6aaa5dcb04fbeaa9b5b
The accelerated convolutional decoder uses SSSE3 instructions such
as PSIGNW (via _mm_sign_epi16) which go beyond what SSE3 offers. So
let's make sure we use the right compiler flag (-mssse3) and also the
right runtime check.
Without this patch, we would use illegal instructions e.g. on Opteron
Gen3 such as Opteron 2427, which are also used as build.opensuse.org
build hosts (build31 through build36) where we wouldn't pass "make
check" as a result.
Change-Id: I2754164384109f2821fd98ffb48f625893f2923d
Fixes: OS#2386
"man getrandom" states sys/random.h is required.
Fixes warning below:
warning: implicit declaration of function ‘getrandom’; did you mean ‘srandom’? [-Wimplicit-function-declaration]
rc = getrandom(out, len, GRND_NONBLOCK);
^~~~~~~~~
Change-Id: I2e73fd018e887893dc5527d6d73644d627eb963a
Let's fix some erroneous/accidential references to wrong license,
update copyright information where applicable and introduce a
SPDX-License-Identifier to all files.
Change-Id: I39af26c6aaaf5c926966391f6565fc5936be21af
Commit in e9e9e427b7 attempted to fix a
compilation warning but introduced a regression documented in OS#2613.
The commit was reverted in 4aa0258269296f078e685e21fb08b115567e814.
After closer lookup and testing, it seems vector_slot(vline, index) is
expected to be NULL in this case as set by vty_complete_command:
/* In case of 'help \t'. */
if (isspace((int)vty->buf[vty->length - 1]))
vector_set(vline, NULL);
As a result, the correct fix for the compilation warning is to test
against NULL instead of testing for empty string.
Change-Id: Id9e02bbf89e0a94e1766b1efd236538712415c8a
The EFR coding contains some repeated bits. In case there are
transmission errors, some bits may of course get corrupted. It looks
like there's an improvement can be made by taking a majority vote on
those "repetition bits", i.e. if 2 out of 3 bits are the same, then use
that instead of expecting to match all 3 bits.
See 3GPP TS 45.003 Section 3.1.1.3 for reference.
Change-Id: I2a28a4d7fb82aed4d39fe8efeea702effdba3858
There's an error in tch_efr_unreorder() function in gsm0503_coding.c
that results in increased RBER. One of the indices used by repetition
bit recombining in this function doesn't match 3GPP TS 45.003 section
3.1.1.3, specifically "w(k) = s(223) for k = 231 and 232".
This bug resulted in RBER even under ideal conditions, with no
fading or AWGN present.
Change-Id: I153da7bbc1bb3e01ed31eb5a7417e90841cfcde3
On systems with GNU/Linux kernel older than 3.17 (Debian 8 "jessie" for
example) the osmo_get_rand_id() would always return failure due to
missing getrandom() syscall.
To support such systems, let's add fallback code which uses GnuTLS
library. It can be disabled explicitly via '--disable-gnutls' option at
compile-time, otherwise ./configure will fail if both getrandom() and
GnuTLS are not available. When building with '--enable-embedded' the
fallback is disabled automatically.
Related: OS#1694
Change-Id: Ic77866ce65acf524b768882c751a4f9c0635740b
The patch seemed sensible, but introduces a segfault when hitting tab
on the interactive VTY. Reproduction example:
osmo-msc
telnet 127.0.0.1 4254
OsmoMSC> enable <TAB>
So we need to understand what that line of code actually intends to do.
Until then, revert this to avoid the segfault.
The segfault happens at:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7bc0894 in cmd_complete_command_real (vline=0x5555558d59e0, vty=0x5555558d57b0, status=0x7fffffffe024) at ../../../../src/libosmocore/src/vty/command.c:1953
1953 if (*(char *)vector_slot(vline, index) == '\0')
This reverts commit e9e9e427b7.
Change-Id: I3fe213bdfb96de9469aae64e67000dafee59302e
According to
https://www.gnu.org/software/automake/manual/automake.html#Libtool-Flags
the libraries supposed to be added to *_LDADD or *_LIBADD
while *_LDFLAGS should contain additional libtool linking
flags. Previously we used both. Let's unify this and move all the
libraries into proper automake variable. While at it - also add
libosmocore.la for tests to LDADD since all the tests link against it
anyway.
Change-Id: Ia657a66db75df831421af5df1175a992da5ba80f
If an application calls osmo_init_logging() multiple times, let's
bail out in a safe way without corrupting the state + returning an
error.
Change-Id: Icf337a430fb367bbca48a1b02822a2cb3b644e5f
osmo_bcd2char() has always supported both decimal and hex.
However, osmo_char2bcd() use to only implement decimal digits.
With this patch, it also suppots conversion of hex characters from ASCII
to BCD.
This would be relevant in cases where somebdoy would want to use 'code
11', 'code 12' or 'ST' signals in any addresses (SCCP GT e.g.)
Change-Id: I7bbcc6de08024567ab64765c12d7de71df787a7a
In Change-Id Ifc6ac824f5dae9a848bb4a5d067c64a69eb40b56 we introduced
a variable de-reference before we check if it's NULL.
Let's reorder the statements to avoid this.
Fixes: Coverity CID#178219
Change-Id: I99265a7ee76f85c479543c19ce8c05ce5d43ae69
The regular 'sh ns' lists all available NS. Sometimes it's handy to know
which of those are persistent.
* add "show ns persistent" command
* adjust parameters of dump-ns*() functions to use bool where
appropriate
Change-Id: Ib812864bae3ea414cc107a7b4f49bea4e6161795
The rate_ctr.c code would do this mangling automatically, but let's
avoid using this from new versions of our code for
simplicity/explicitness.
Change-Id: I24a556f447cfac25efb6e83cac2d0c2972d98fe3
Harald Welte
Vadim Yanitskiy
Pau Espin
Philipp Maier
Daniel Willmann
Neels Hofmeyr