Commit Graph

560 Commits

Author SHA1 Message Date
Neels Hofmeyr f2644aee55 logging.h: define ansi color constants
It's hard to figure out what color logging categories have with those ANSI
color code strings. Instead, define these OSMO_LOGCOLOR_* constants.

Naming: commonly, the logging.h header has the "LOG" prefix in the name, but it
seems saner to include the OSMO_ prefix: it seems too likely that some
libosmocore user somewhere already has defined "LOGCOLOR_RED" somewhere.

Change-Id: I03b6b1f73ae7ee61d37ff921e071a3d0881d3e9a
2019-11-23 07:58:47 +00:00
Neels Hofmeyr 002a51d731 add osmo_sockaddr_str_cmp()
Currently planned user: for Distributed GSM in osmo-hlr: setting per-MSC
service addresses in VTY: replace/remove existing entries.

osmo_sockaddr_str_cmp() is useful to catch identical resulting IP addresses,
regardless of differing strings (e.g. '0::' and '::' are equal but differ in
strings).

Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501
2019-11-21 21:17:12 +01:00
Vadim Yanitskiy b639b4d4f7 logging/vty: fix vty_read_file(): do not write warnings to stdin
Setting vty->fd to 0 is a bad idea, which may cause the process
to write() warnings to its own _stdin_ (yes, it's possible).
For example, when a configuration file contains deprecated
logging commands. Let's use stderr by default.

Change-Id: Icdeaea67a06da3a2f07b252e455629559ecc1829
2019-11-21 10:48:30 +07:00
Vadim Yanitskiy 4abda9ea26 logging/vty: fix: actually ignore deprecated logging commands
We shall not prevent programs from starting if their configuration
files contain deprecated 'logging level ...' commands. Just print
a warning and return CMD_SUCCESS instead of CMD_WARNING.

While writing a unit test, another funny bug has been uncovered.
Parsing of a deprecated command indeed triggers a deprecation
warning, originated from libosmovty's log_deprecated_func().
This function simply calls vty_out(), but...

Since the invocation of the vty_out() happens _before_ the VTY
is initialized, the process is actually writing that warning
to its own stdin! Most likely, because we use talloc_zero()
to allocate a new instance of struct 'vty'.

As a side effect, the evil warning magically appears in the output
of 'make check', breaking the test statistics. Let's work around
this bug for now by redirecting stdin to /dev/null.

Change-Id: Ia934581410cd41594791d4e14ee74c16abe1009a
Fixes: Ic9c1b566ec4a459f03e6319cf369691903cf9d00
2019-11-21 10:48:02 +07:00
Neels Hofmeyr d0e8d6b9b5 fix OSMO_SOCKADDR_STR_FMT for IPv6
The format prints IP:port separated by a colon, which of course is confusing
when the IPv6 address itself contains mostly colons. The new format adds square
braces.

  cafe:face::1:42 -> [cafe:face::1]:42

The IPv4 format remains unchanged:
  1.2.3.4:42

Change-Id: I161f8427729ae31be0eac719b7a4a9290715e37f
2019-11-11 20:14:01 +01:00
Neels Hofmeyr 83d691048b test: add OSMO_SOCKADDR_STR_FMT to sockaddr_str_test.c
This shows the weird format choice for showing IPv6 addresses' port, fixed in
subsequent patch.

Change-Id: I8e5ebfbbc3a2b88aed820e8f845d9f6ededb29de
2019-11-11 20:14:01 +01:00
Pau Espin e40b9637ea gsm: gsm_utils: Fix return type of API ms_class_gmsk_dbm() and add unit tests
Only known user of API is in osmocom-bb and it compiles fine after the
change.

Related: OS#4244
Change-Id: Ia10345008b3aca50b30482ef3b852b03eca71995
2019-11-04 12:41:22 +01:00
Pau Espin b99f4ca2d8 gsm_04_08.h: Introduce API osmo_gsm48_rfpowercap2powerclass()
Related: OS#4244
Change-Id: I32e9cc1c2397b44f0d48db2acdf782a821365b63
2019-11-03 23:21:06 +00:00
Neels Hofmeyr 2ceb758ba4 add osmo_sockaddr_str_is_nonzero()
Often, an IP address of 0.0.0.0 is considered an unset value (for clients
requiring a server address; not for listening on "any").

osmo_sockaddr_str_is_set() does return false when the port is 0, but there is
no simple way to tell whether the IP address is actually set to a server
address.

Add osmo_sockaddr_str_is_nonzero() to return false if:
- the port is zero, or
- the IP address is zero (0.0.0.0 or ::0), or
- the IP address cannot be parsed.

A practical use example: osmo-msc so far accepts an RTP IP address of 0.0.0.0
as valid. I noticed when trying to trigger error handling from a ttcn3 test.
osmo-msc can use this function to reject invalid addresses from MGCP messages.

Related: I53ddb19a70fda3deb906464e1b89c12d9b4c7cbd (osmo-msc)
Change-Id: I73cbcab90cffcdc9a5f8d5281c57c1f87b2c3550
2019-11-01 14:33:58 +01:00
Neels Hofmeyr b1bbe24c09 fsm: refuse state chg and events after term
Refuse state changes and event dispatch for FSM instances that are already
terminating.

It is assumed that refusing state changes and events after FSM termination is
seen as the sane expected behavior, hence this change in behavior is merged
without being configurable.

There is no fallout in current Osmocom code trees. fsm_dealloc_test needs a
changed expected output, since it is explicitly creating complex FSM structures
that terminate. Currently no other C test in Osmocom code needs adjusting.

Rationale:

Where multiple FSM instances are collaborating (like in osmo-bsc or osmo-msc),
a terminating FSM instance often causes events to be dispatched back to itself,
or causes state changes in FSM instances that are already terminating. That is
hard to avoid, since each FSM instance could be a cause of failure, and wants
to notify all the others of that, which in turn often choose to terminate.

Another use case: any function that dispatches events or state changes to more
than one FSM instance must be sure that after the first event dispatch, the
second FSM instance is in fact still allocated. Furthermore, if the second FSM
instance *has* terminated from the first dispatch, this often means that no
more actions should be taken. That could be done by an explicit check for
fsm->proc.terminating, but a more general solution is to do this check
internally in fsm.c.

In practice, I need this to avoid a crash in libosmo-mgcp-client, when an
on_success() event dispatch causes the MGCP endpoint FSM to deallocate. The
earlier dealloc-in-main-loop patch fixed part of it, but not all.

Change-Id: Ia81a0892f710db86bd977462730b69f0dcc78f8c
2019-10-29 17:28:30 +01:00
Neels Hofmeyr 988f6d72c5 add osmo_fsm_set_dealloc_ctx(), to help with use-after-free
This is a simpler and more general solution to the problem so far solved by
osmo_fsm_term_safely(true). This extends use-after-free fixes to arbitrary
functions, not only FSM instances during termination.

The aim is to defer talloc_free() until back in the main loop.

Rationale: I discovered an osmo-msc use-after-free crash from an invalid
message, caused by this pattern:

void event_action()
{
       osmo_fsm_inst_dispatch(foo, FOO_EVENT, NULL);
       osmo_fsm_inst_dispatch(bar, BAR_EVENT, NULL);
}

Usually, FOO_EVENT takes successful action, and afterwards we also notify bar.
However, in this particular case, FOO_EVENT caused failure, and the immediate
error handling directly terminated and deallocated bar. In such a case,
dispatching BAR_EVENT causes a use-after-free; this constituted a DoS vector
just from sending messages that cause *any* failure during the first event
dispatch.

Instead, when this is enabled, we do not deallocate 'foo' until event_action()
has returned back to the main loop.

Test: duplicate fsm_dealloc_test.c using this, and print the number of items
deallocated in each test loop, to ensure the feature works. We also verify that
the deallocation safety works simply by fsm_dealloc_test.c not crashing.

We should probably follow up by refusing event dispatch and state transitions
for FSM instances that are terminating or already terminated:
see I0adc13a1a998e953b6c850efa2761350dd07e03a.

Change-Id: Ief4dba9ea587c9b4aea69993e965fbb20fb80e78
2019-10-29 16:46:04 +01:00
Philipp Maier 69e00ccd6f gsm0508: add functions to calculate beginning of a block
The calculation of the beginning of a block for TCH/F, TCH/H and FACCH
can be challenging since those channels are affected by the diagonal
interleaving of the TCH channels. However, GSM 05.02 Section 7 Table 1
of 5 specifies how the blocks are distributed over the TDMA frame
interval. Lets add a mapping function that is based on that table

Related: OS#3803
Change-Id: I3d71c66f8c401f5afbad9b1c86c24580dab9e0ce
2019-10-28 19:43:14 +00:00
Pau Espin a0c8195ad3 vty: Return error if cmd returns CMD_WARNING while reading cfg file
Otherwise bad configurations can easily sneak in and produce unexpected
behavior.

Change-Id: Ic9c1b566ec4a459f03e6319cf369691903cf9d00
2019-10-28 19:15:29 +00:00
Pau Espin d12f698dbb logging: Introduce mutex API to manage log_target in multi-thread envs
log_enable_multithread() enables use of locks inside the
implementation. Lock use is disabled by default, this way only
multi-thread processes need to enable it and suffer related
complexity/performance penalties.

Locks are required around osmo_log_target_list and items inside it,
since targets can be used, modified and deleted by different threads
concurrently (for instance, user writing "logging disable" in VTY while
another thread is willing to write into that target).

Multithread apps and libraries aiming at being used in multithread apps
should update their code to use the locks introduced here when
containing code iterating over osmo_log_target_list explictly or
implicitly by obtaining a log_target (eg. osmo_log_vty2tgt()).

Related: OS#4088
Change-Id: Id7711893b34263baacac6caf4d489467053131bb
2019-10-09 14:19:52 +02:00
Pau Espin 9168cc9830 tdef_test: verify case where osmo_tdef_set returns -EEXIST
Change-Id: I436daa804aac11622fde24afe9ea35193d9e9beb
2019-10-07 13:14:14 +00:00
Pau Espin 0cbe8f0100 tdef: Introduce min_val and max_val fields
This is useful for timers expected to have a range of valid or expected
values.

Validation is done at runtime when timer values are set by the app or by
the user through the VTY.

Related: OS#4190
Change-Id: I4661ac41c29a009a1d5fc57d87aaee6041c7d1b2
2019-10-07 13:14:14 +00:00
Neels Hofmeyr 1b729ce106 gsup: add OSMO_GSUP_SUPPORTED_RAT_TYPES_IE and OSMO_GSUP_CURRENT_RAT_TYPE_IE
OSMO_GSUP_SUPPORTED_RAT_TYPES_IE corresponds to the Supported RAT Types
Indicator from 3GPP TS 29.002. See 8.1.2 MAP_UPDATE_LOCATION service,
which indicates the capabilities of the MSC/VLR to the HLR.

So far, have room for eight RAT types in the gsup_msg. That is an arbitrary
random choice without any rationale.

OSMO_GSUP_CURRENT_RAT_TYPE_IE is useful to communicate the currently
used RAN / RAT type of the current subscriber during Location Updating Request.

Change-Id: I93850710ab55a605bf61b95063a69682a2899bb1
2019-09-28 14:34:07 +02:00
Vadim Yanitskiy 14b87b2bbf gsm29205_test: fix error: missing braces around initializer
Since structure 'osmo_gcr_parsed' does contain arrays, GCC is not
happy about the way we initialize it. Let's do it explicitly.

Change-Id: Ia814b4a4ed5bec84ff1f69232f7f7d5ca0d19794
2019-09-27 03:16:17 +00:00
Ruben Undheim dd61cd2fff No fail if no /proc/cpuinfo
Change-Id: I4b9e12e34f69d98fa87179c7ee390e31001ec943
2019-09-26 19:33:13 +00:00
Philipp Maier ae140bc84a ecu_fr: increase test coverage for FR ECU implementation
The ECU implementation for FR is currently tested by calling the related
functions directly and by using the generic ECU abstraction layer. However,
the test "test_fr_concealment" only tests directly. Lets add a version
that uses the generic ECU abstraction layer as well.

The generic ECU abstraction layer obsolets the public API functions
osmo_ecu_fr_reset() and osmo_ecu_fr_conceal(), lets tag those functions
as dprecated.

Change-Id: Ib0c8a9b164f14ea4fa00688f760a76cdb4890af4
2019-09-20 11:21:46 +02:00
Harald Welte 750d8311f5 codec/ecu: Introduce new generic Error Concealment Unit abstraction
We don't want to expose the details of a given ECU implementation to
the user (e.g. osmo-bts), but have a generic abstraction layer where
an ECU implementation can simply register a few call-back functions
with the generic core.

As the developer and copyright holder of the related code, I hereby
state that any ECU implementation using 'struct osmo_ecu_ops' and
registering with the 'osmo_ecu_register()' function shall not be
considered as a derivative work under any applicable copyright law;
the copyleft terms of GPLv2 shall hence not apply to any such ECU
implementation.

The intent of the above exception is to allow anyone to combine
third party Error Concealment Unit implementations with libosmocore,
including but not limited to such published by ETSI.

Change-Id: I4d33c9c7c2d4c7462ff38a49c178b65accae1915
2019-09-02 09:13:50 +00:00
Harald Welte 2d90611cb0 context: Add support for [per-thread] global talloc contexts
Rather than having applications maintain their own talloc cotexts,
let's offer some root talloc contexts in libosmocore.  Let's also
make them per thread right from the beginning.  This will help
some multi-threaded applications to use talloc in a thread-safe
way.

Change-Id: Iae39cd57274bf6753ecaf186f229e582b42662e3
2019-08-27 13:43:31 +02:00
Neels Hofmeyr d0b3b9edac add vty logp command to echo on all log targets
When reading SUT logs resulting from TTCN3 runs, it can be hard to figure out
which log section corresponds to which test code. Add a 'logp' command on VIEW
and ENABLE nodes that simply echos an arbitrary message on log output, useful
to set markers / explanations from the TTCN3 code, which then appear in all log
outputs and can make it trivial to figure out which log section is interesting.

	logging_vty_test# logp lglobal notice This is the log message
	DLGLOBAL NOTICE This is the log message

From TTCN3, could be used like this, e.g. in BSC_Tests.ttcn:

	private function f_logp(charstring log_msg) runs on MSC_ConnHdlr
	{
		// log on TTCN3 log output
	        log(log_msg);
		// log in stderr log
	        f_vty_transceive(BSCVTY, "logp lglobal notice " & log_msg);
	}

	...

	f_logp("f_probe_for_handover(" & log_label & "): Ending the test: Handover Failure stops the procedure.");

Change-Id: Ife5dc8999174c74e0d133729284fe526d6eaf8d9
2019-08-13 15:35:02 +02:00
Pau Espin f65278f807 tests: logging: Remove undefined param passed to logging_vty_add_cmds
Since March 15th 2017, libosmocore API logging_vty_add_cmds() had its
parameter removed (c65c5b4ea0). However,
definition in C file doesn't contain "(void)", which means number of
parameters is undefined and thus compiler doesn't complain. Let's remove
parameters from all callers before enforcing "(void)" on it.

Related: OS#4138
Change-Id: Iaea795521361a8e5b3b45eaeb35e6eda69163af3
2019-08-05 14:10:45 +02:00
Pau Espin 2605ffc3d7 gsm0808_test: Fix wrong use of memcp
After recent system upgrade, gcc 9.1.0, I started getting gsm0808_test
failing locally:
Assert failed memcmp(&enc_ct, &dec_ct, sizeof(enc_ct)) == 0 libosmocore/tests/gsm0808/gsm0808_test.c:992

During investigation with gdb, fields of both structures seem to contain
same values. However, closer lookup gives some hints on why it fails:
(gdb) print memcmp(&enc_ct, &dec_ct, sizeof(enc_ct))
$1 = 85
(gdb) print memcmp(&enc_ct, &dec_ct, 12)
$14 = 85
(gdb) print ((uint8_t*)&enc_ct)[11]
$15 = 85 'U'
(gdb) print ((uint8_t*)&dec_ct)[11]
$16 = 0 '\000'

So the 12th byte in struct gsm0808_channel_type is basically an
alignment padding byte added by the compiler (to align perm_spch_len to
4-byte alignment). Since both compared structs are initialized without
memset(0) but using compiler's designated initializers, it seems the compiler
decided it's no longer needed to zero the padding byte, making memcp fail in
this case.

In order to avoid the failure, let's properly check every field instead
of using memcp here.

Change-Id: I17fe7a0a5dc650f050bba1f47d071be749550729
2019-07-30 12:51:29 +00:00
Alexander Couzens 4e284b6379 utils.h: require a semi colon after OSMO_ASSERT
When using `OSMO_ASSERT(exp);` clang will warn about
an empty expression because the semi colon was superflous.
Use do {} while (0) to enfore the need of a semi colon.
This might break other test.

Change-Id: I2272d29a81496164bebd1696a694383a28a86434
2019-07-08 13:23:45 +00:00
Pau Espin 32e6710487 vty: command.c: Fix: single-choice optional args are no longer passed incomplete to vty func
For instance, take command "single0 [one]":
If user executes "single0 on", VTY func will receive argv[0]="one"
instead of argv[0]="on".

Related: OS#4045
Change-Id: I5f4e2d16c62a2d22717989c6acc77450957168cb
2019-06-14 17:44:21 +02:00
Pau Espin 7e1b03f763 vty: command.c: Fix: multi-choice args are no longer passed incomplete to vty func
For instance, take command "multi0 (one|two|three)":
If user executes "multi0 tw", VTY func will receive argv[0]="two"
instead of argv[0]="tw".

Fixes: OS#4045
Change-Id: I91b6621ac3d87fda5412a9b415e7bfb4736c8a9a
2019-06-14 17:44:16 +02:00
Pau Espin 14aadd5b6f vty: tests: Verify incomplete optional parameters are passed to vty funcs
The test shows that in the case were "single0 on" is executed, VTY
function should return complete "single0 one" but it doesn't.

Related: OS#4045
Change-Id: Ib5b9dc07e2b280dc95011b3926afb1d490cadd81
2019-06-14 12:43:19 +02:00
Pau Espin 6df2e44404 vty: command.c: Fix is_cmd_ambiguous() returning always 0
inner block defined variable "enum match_type ret" was being masking
outter block variable "int ret = 0". The ret variable was being given
non zero values only inside the inner block, so that change was done on
the inner variable and not the outer one, which is returned.

Fixes: 5314c513f2
Change-Id: Iec87d7db49a096d07e38ff8a060b923a52bfd6ba
2019-06-11 21:50:17 +02:00
Oliver Smith 186f878266 gsm48_decode_bcd_number2: fix ENOSPC edge case
Return ENOSPC if the decoding buffer is one byte too small, instead of
returning 0 and silently truncating the string. Add a new "truncated"
variable to detect if the loop breaks in the final iteration.

The string is not truncated if there is exactly one 0xf ('\0') higher
nibble remaining. This is covered by the existing test case "long
15-digit (maximum) MSISDN, limited buffer".

Related: OS#4049
Change-Id: Ie05900aca50cc7fe8a45d17844dbfcd905fd82fe
2019-06-07 11:01:51 +02:00
Vadim Yanitskiy 8c9befeaee vty_transcript_test.vty: add choice auto-complete tests
This patch is a result of discussion we had in [1]. The key idea
is that libosmovty should properly auto-complete the commands
containing choice, such as the following one:

  multi0 (one|two|three)

[1] If9b0c0d031477ca87786aab5c269d00748e896c8

Right now, sending the following command:

  (osmo-foo-bar)# multi0 th

would basically match the following vector:

  multi0 three

however the resulting argv would be:

  ["multi0", "th"]

Moreover, sending the following command:

  (osmo-foo-bar)# multi0 t

would basically match the following vectors:

  multi0 two
  multi0 three

because both start from 't', so the resulting argv would be:

  ["multi0", "t"]

which is ambiguous!

The expected output is:

  (osmo-foo-bar)# multi0 th
  ok argc=1 three

  (osmo-foo-bar)# multi0 t
  % Ambiguous command.

This is going to be fixed in the follow up patches.

Change-Id: I83c3aef813173952641035862c534ef16384780e
2019-06-06 06:26:30 +00:00
Vadim Yanitskiy 1dc82643c5 gsm48_encode_bcd_number(): clarify optional LHV header initialization
Change-Id: Iafd911dd55691b3715391e3899cd6971245c8d7f
2019-05-29 08:48:46 +00:00
Vadim Yanitskiy e4799f5603 gsm48_decode_bcd_number2(): return -EINVAL if LV has too big length
Change-Id: Ie07b2e8bc2f9628904e88448b4ee63b359655123
2019-05-28 06:50:41 +07:00
Vadim Yanitskiy 7194087457 gsm48_decode_bcd_number2(): fix: return -ENOSPC on truncation
The documentation of gsm48_decode_bcd_number2() clearly states that
the output truncation is a erroneous case, so it should actually
return negative in such cases. Let's return -ENOSPC.

Change-Id: I75680f232001ba419a587fed4c24f32c70c3ad2b
2019-05-28 06:50:41 +07:00
Vadim Yanitskiy 2cd1dda631 gsm48_decode_bcd_number2(): fix output truncation
Thanks to the new unit test for BCD number encoding / decoding, it was
discovered that gsm48_decode_bcd_number2() does not properly handle
encoded LV if the output buffer size is equal to the original MSISDN
length + 1 (\0-terminator): one digit is lost.

For example, decoding of 15-digit long MSISDN to a buffer of size
16 (15 digits + 1 for \0) would give us only 14 digits.

The problem was that 'output_len' was being decremented before
checking the remaining buffer length and writing a digit to it.
As a result, the maximum length was always one byte shorter.

Change-Id: I61d49387fedbf7b238e21540a5eff22f6861e27a
Fixes: OS#4025
2019-05-28 06:50:41 +07:00
Vadim Yanitskiy aa0683d9f8 gsm0408/gsm0408_test.c: introduce BCD number encoding / decoding test
So far, both gsm48_encode_bcd_number() and gsm48_decode_bcd_number2()
did not have any unit test coverage. Let's fill this gap by testing
the following scenarios:

  - encoding / decoding of a regular 9-digit MSISDN;
  - encoding / decoding of a MSISDN with optional LHV;
  - encoding / decoding of a long 15-digit MSISDN;
  - encoding / decoding of a MSISDN to a buffer:
    - with exactly matching size,
    - with lower size (truncation);
  - decoding LV buffer with incorrect length,
  - encoding / decoding an empty input buffer.

As it turns out, gsm48_decode_bcd_number2() does not properly
handle encoded LV if the output buffer size is equal to the
original MSISDN length + 1 (\0-terminator): one digit is lost.

For example, decoding of 15-digit long MSISDN to a buffer of size
16 (15 digits + 1 for \0) would give us only 14 digits. This is
reflected in the unit test output:

  Decoding HEX (buffer limit=16) '0821436587092143f5'...
    Expected: (rc=0) '123456789012345'
      Actual: (rc=0) '12345678901234'

Moreover, if the output buffer is shorter than decoded number,
gsm48_decode_bcd_number2() silently truncates it and returns 0,
while its description states, that the rc should reflect this.

To be fixed in the follow-up patches.

Change-Id: I4b2c330cf8ffe4427c0bee7d5f3b74be56ecd85d
Related: OS#4025
2019-05-28 06:50:41 +07:00
Harald Welte c53afd95c5 oap_client: Fix license: GPLv2+ instead of AGPLv3+
libosmo{core,gsm,vty} code is GPLv2+.  The OAP code originated in
osmo-msc.git and was moved here without changing the license. That
was a mistake, it always was meant to be under GPLv2-or-later after
moving to libosmocore.git.

Copyright is with sysmocom, so I as the managing director can
approve the license change.

Change-Id: I08311fa8214c15f8df8945b9894226608cf96f15
2019-05-27 23:26:45 +02:00
Harald Welte 8006f5393e TLV: Add one-shot TLV encoder
So far, the TLV code contained two types of functions
* tlp_parse() to parse all TLVs according to definition into tlvp_parsed
* various helper functions to encode individual TLVs during message
  generation

This patch implements the inverse of tlv_parse(): tlv_encode(), which
takes a full 'struct tlv_pared' and encodes all IEs found in it.  The
order of IEs is in numerically ascending order of the tag.

As many protocols have different IE/TLV ordering requirements, let's add
a tlv_encode_ordered() function where the caller can specify the TLV
ordering during the one-shot encode.

Change-Id: I761a30bf20355a9f80a4a8e0c60b0b0f78515efe
2019-05-19 07:33:32 +00:00
Alexander Couzens cc72cc45a4 add osmo_stat_item_inc/osmo_stat_item_dec to set it relative
Change-Id: Id2462c4866bd22bc2338c9c8f69b775f88ae7511
2019-05-07 13:20:57 +00:00
Oliver Smith affbb20646 GSUP: add inter-MSC handover related msgs and IEs
Based on a draft created by Neels, which is the result of reading a MAP
trace of two MSCs negotiating inter-MSC handovers, and of reading the
TS 29.002, TS 29.010 and related specs:
https://lists.osmocom.org/pipermail/openbsc/2019-January/012653.html

I figured out that the "Handover Number" mentioned in the specifications
is the same as the MSISDN IE that we already have, so we can use that
instead of creating a new IE (example usage in tests/gsup/gsup_test.c).

Create a new OSMO_GSUP_MSGT_E_ROUTING_ERROR message type, which the GSUP
server uses to tell a client that its message could not be forwarded to
the destination (see [1]). MAP has no related message.

[1]: Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 (osmo-hlr.git)

Related: OS#3774
Change-Id: Ic00b0601eacff6d72927cea51767801142ee75db
2019-04-26 20:36:39 +02:00
Neels Hofmeyr 9838c9070f GSUP: add Message Class IE
osmo-msc and osmo-hlr have distinct subsystems handling incoming GSUP messages.
So far we decide entirely by message type which code path should handle a GSUP
message. Thus no GSUP message type may be re-used across subsystems.

If we add a GSUP message to indicate a routing error, it would have to be a
distinct message type for subscriber management, another one for SMS, another
one for USSD...

To allow introducing common message types, introduce a GSUP Message Class IE.

In the presence of this IE, GSUP handlers can trivially direct a received
message to the right code path. If it is missing, handlers can fall back to the
previous switch(message_type) method.

Change-Id: Ic397a9f2c4a7224e47cab944c72e75ca5592efef
2019-04-13 21:38:58 +00:00
Neels Hofmeyr ecef7ec3c3 add osmo_{escape,quote}_str_buf2() for standard args ordering
To be able to append an escaped or quoted string using
OSMO_STRBUF_APPEND_NOLEN(), the function signature must have the buf and len as
first args, like most other *_buf() functions.

Add osmo_escape_str_buf2() and osmo_quote_str_buf2() to match this signature.

A recent patch [1] has changed the return value of osmo_escape_str_buf() to
char*, removing the const. However, the functions may return const strings,
hence re-add the const. The new signatures always return the non-const buffer.

To avoid code duplication, implement osmo_quote_str_buf() and
osmo_escape_str_buf() by calling the new functions.

I decided to allow slight changes to the behavior for current osmo_escape_str()
and osmo_escape_str_buf(), because impact on callers is minimal:

(1) The new implementation uses OSMO_STRBUF_*, and in consequence
osmo_quote_str() no longer prints an ending double quote after truncated
strings; Before, a truncated output was, sic:
  "this string is trunca"
and now this becomes, sic:
  "this string is truncat
I decided to not keep the old behavior because it is questionable to begin
with. It looks like the string actually ended at the truncation boundary
instead of the reason being not enough space in the output buffer.

(2) The new osmo_escape_str_buf2() function obviously cannot pass-thru an
unchanged char* if no escaping was needed. Sacrifice this tiny optimization
feature to avoid code duplication:
- it is an unnoticeable optimization,
- the caller anyway always passes a string buffer,
- the feature caused handling strings and buffers differently depending on
  their content (i.e. code that usually writes out strings in full length
  "suddenly" truncates because a non-printable character is contained, etc.)
I considered adding a skip_if_unescaped flag to the osmo_quote_str_buf2()
function signature, but in the end decided that the API clutter is not worth
having for all the above reasons.

Adjust tests to accomodate above changes.

[1] 4a62eda225
    Ibf85f79e93244f53b2684ff6f1095c5b41203e05

Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
2019-04-12 01:00:16 +02:00
Neels Hofmeyr 8531d6695f tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()
In OSMO_STRBUF_APPEND, use local variable names that are less likely to shadow
other local variables: prefix with _sb_.

In OSMO_STRBUF_APPEND, add a check to add to .pos only if it is not NULL.

Add OSMO_STRBUF_APPEND_NOLEN(), which works for function signatures that don't
return a length. This is useful for any osmo_*_buf() string writing functions,
so that these write directly to the strbuf.

Change-Id: I108cadf72deb3a3bcab9a07e50572d9da1ab0359
2019-04-11 07:40:25 +02:00
Neels Hofmeyr d79ccc65f7 add osmo_str_startswith()
Move from a static implementation in tdef_vty.c to utils.c, I also want to use
this in osmo-msc.

The point is that the telnet VTY allows unambiguous partly matches of keyword
args. For example, if I have a command definition of:

    compare (apples|oranges)

then it is perfectly legal as for the vty parser to write only

    compare app

One could expect the VTY to then pass the unambiguous match of "apples" to the
parsing function, but that is not the case.

Hence a VTY function implementation is faced with parsing a keyword of "app"
instead of the expected "apples".

This is actually a very widespread bug in our VTY implementations, which assume
that exactly one full keyword will always be found. I am now writing new
commands in a way that are able to manage only the starts of keywords.

Arguably, strstr(a, b) == a does the same thing, but it searches the entire
string unnecessarily.

Change-Id: Ib2ffb0e9a870dd52e081c7e66d8818057d159513
2019-04-11 05:36:36 +00:00
Neels Hofmeyr d28aa0c2f1 fsm_dealloc_test: no need for ST_DESTROYING
A separate ST_DESTROYING state originally helped with certain deallocation
scenarios. But now that fsm.c avoids re-entering osmo_fsm_inst_term() twice and
gracefully handles FSM instance deallocations for termination cascades, it is
actually just as safe without a separate ST_DESTROYING state. ST_DESTROYING was
used to flag deallocation and prevent entering osmo_fsm_inst_term() twice,
which works only in a very limited range of scenarios.

Remove ST_DESTROYING from fsm_dealloc_test.c to show that all tested scenarios
still clean up gracefully.

Change-Id: I05354e6cad9b82ba474fa50ffd41d481b3c697b4
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 1f9cc01861 fsm: support graceful osmo_fsm_inst_term() cascades
Add global flag osmo_fsm_term_safely() -- if set to true, enable the following
behavior:

Detect osmo_fsm_inst_term() occuring within osmo_fsm_inst_term():
- collect deallocations until the outermost osmo_fsm_inst_term() is done.
- call osmo_fsm_inst_free() *after* dispatching the parent event.

If a struct osmo_fsm_inst enters osmo_fsm_inst_term() while another is already
within osmo_fsm_inst_term(), do not directly deallocate it, but talloc-reparent
it to a separate talloc context, to be deallocated with the outermost FSM inst.

The effect is that all osmo_fsm_inst freed within an osmo_fsm_inst_term()
cascade will stay allocated until all osmo_fsm_inst_term() are complete and all
of them will be deallocated at the same time.

Mark the deferred deallocation state as __thread in an attempt to make cascaded
deallocation handling threadsafe.  Keep the enable/disable flag separate, so
that it is global and not per-thread.

The feature is showcased by fsm_dealloc_test.c: with this feature, all of those
wild deallocation scenarios succeed.

Make fsm_dealloc_test a normal regression test in testsuite.at.

Rationale:

It is difficult to gracefully handle deallocations of groups of FSM instances
that reference each other. As soon as one child dispatching a cleanup event
causes its parent to deallocate before fsm.c was ready for it, deallocation
will hit a use-after-free. Before this patch, by using parent_term events and
distinct "terminating" FSM states, parent/child FSMs can be taught to wait for
all children to deallocate before deallocating the parent. But as soon as a
non-child / non-parent FSM instance is involved, or actually any other
cleanup() action that triggers parent FSMs or parent talloc contexts to become
unused, it is near impossible to think of all possible deallocation events
ricocheting, and to avoid running into freeing FSM instances that were still in
the middle of osmo_fsm_inst_term(), or FSM instances to enter
osmo_fsm_inst_term() more than once. This patch makes deallocation of "all
possible" setups of complex cross referencing FSM instances easy to handle
correctly, without running into use-after-free or double free situations, and,
notably, without changing calling code.

Change-Id: I8eda67540a1cd444491beb7856b9fcd0a3143b18
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 3b414a4adc fsm: add flag to ensure osmo_fsm_inst_term() happens only once
To prevent re-entering osmo_fsm_inst_term() twice for the same osmo_fsm_inst,
add flag osmo_fsm_inst.proc.terminating. osmo_fsm_inst_term() sets this to
true, or exits if it already is true.

Update fsm_dealloc_test.err for illustration. It is not relevant for unit
testing yet, just showing the difference.

Change-Id: I0c02d76a86f90c49e0eae2f85db64704c96a7674
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 223d66a414 add fsm_dealloc_test.c
Despite efforts to properly handle "GONE" events and entering a ST_DESTROYING
only once, so far this test runs straight into a heap use-after-free. With
current fsm.c, it is hard to resolve the situation with the objects named
"other" also causing deallocations besides the FSM instance parent/child
relations.

For illustration, add an "expected" test output file fsm_dealloc_test.err,
making this pass will follow in a subsequent patch.

Change-Id: If801907c541bca9f524c9e5fd22ac280ca16979a
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 0e8df1c7e4 add osmo_use_count API
Provide a common implementation of use counting that supports naming each user
as well as counting more than just one use per user, depending on the rules the
caller implies.

In osmo-msc, we were originally using a simple int counter to see whether a
connection is still in use or should be discarded. For clarity, we later added
names to each user in the form of a bitmask of flags, to figure out exactly
which users are still active: for logging and to debug double get / double put
bugs. This however is still not adequate, since there may be more than one CM
Service Request pending. Also, it is a specialized implementation that is not
re-usable.

With this generalized implementation, we can:

- fix the problem of inadequate counting of multiple concurrent CM Service
  Requests (more than one use count per user category),
- directly use arbitrary names for uses like __func__ or "foo" (no need to
  define enums and value_string[]s),
- re-use the same code for e.g. vlr_subscr and get fairly detailed VLR
  susbscriber usage logging for free.

Change-Id: Ife31e6798b4e728a23913179e346552a7dd338c0
2019-04-08 13:47:17 +00:00