gcc-9.2.1 issues the following errror when compiling with -Werror:
In function ‘tl16v_put’,
inlined from ‘tvlv_put’ at ../../include/osmocom/gsm/tlv.h:156:9,
inlined from ‘tvlv_put’ at ../../include/osmocom/gsm/tlv.h:147:24,
inlined from ‘msgb_tvlv_put’ at ../../include/osmocom/gsm/tlv.h:223:9,
inlined from ‘bssgp_tx_paging’ at gprs_bssgp.c:1250:2:
../../include/osmocom/gsm/tlv.h:131:2: error: ‘memcpy’ reading between 128 and 65535 bytes from a region of size 9 [-Werror=stringop-overflow=]
131 | memcpy(buf, val, len);
| ^~~~~~~~~~~~~~~~~~~~~
Unfortunately I've not been able to work around it with some nice GCC
#pragma GCC diagnostic ignored "-Wstringop-overflow"
Change-Id: I22a0c399c6c00eaf87277002096a82844c9e198e
So far we had a function to allocate a new bssgp_bvc_ctx, but not
the opposite one. Let's finally introduce it, so it will be used
at least in OsmoPCU.
Please note that the new symbol has 'bssgp_' prefix, not 'btsctx_'.
Change-Id: Ia78979379dbdccd6e4628c16f00d0c06d9212172
Do not return a warning and therefore fail parsing the config when the
"encapsulation framerelay-gre local-ip" command is used and FR/GRE is
disabled. Having this in the config does no harm and allows keeping the
same config if it is enabled later.
This fixes the currently failing vty tests for osmo-sgsn.
Fixes: a0c8195ad3 ("vty: Return error if cmd returns CMD_WARNING while reading cfg file")
Change-Id: Ic225232fbfca49ba868427eaf898e1f6e34e1ca8
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
We have a number of library-internal static global buffers which are
mainly used for various stringification functions. This worked as
all of the related Osmocom programs were strictly single-threaded.
Let's make those buffers at least thread-local. This way every thread
gets their own set of buffers, and it's safe for multiple threads to
execute the same functions once. They're of course still not
re-entrant. If you need re-entrancy, you will need to use the _c()
or _buf() suffix version of those functions and work with your own
(stack or heap) buffers.
Change-Id: I50eb2436a7c1261d79a9d2955584dce92780ca07
Calling sizeof() on a pointer would result in getting size of the
pointer (usually 4 or 8 bytes) itself, but not the size of the
memory it points to.
Change-Id: I83f55a9638b75d9097d37992f7c84707791f10f6
Fixes: CID#194266
We have a habit of returning static buffers from some functions,
particularly when generating some kind of string values. This is
convenient in terms of memory management, but it comes at the expense
of not being thread-safe, and not allowing for two calls of the
related function within one printf() statement.
Let's introduce _c suffix versions of those functions where the
caller passes in a talloc context from which the output buffer shall
be allocated.
Change-Id: I8481c19b68ff67cfa22abb93c405ebcfcb0ab19b
We have a number of static buffers in use in libosmo*. This means
the related functions are not usable in a thread-safe way. While
we so far don't have many multi-threaded programs in the osmocom
universe, the static buffers also prevent us from calling the same
e.g. string-ify function twice within a single printf() call.
Let's make sure there's an alternative function in all those cases,
where the user can pass in a caller-allocated buffer + size, and make
the 'classic' function with the static buffer a wrapper around that
_buf() variant.
Change-Id: Ibf85f79e93244f53b2684ff6f1095c5b41203e05
Handle NSEI the same way as BVCI is handled: assign it to variable
instead of repetitive calls to msgb_nsei() - this simplifies log update
in follow-up patches and makes code slightly easier to read.
Change-Id: I919a717ca22646849d6ec7f62c677c536db0ed31
The naming of these constants dates back to when the code was private
within OpenBSC. Everything else was renamed (bsc_fd -> osmo_fd) at
the time, but somehow the BSC_FD_* defines have been missed at the
time.
Keep compatibility #defines around, but allow us to migrate the
applications to a less confusing naming meanwhile.
Change-Id: Ifae33ed61a7cf0ae54ad487399e7dd2489986436
When putting together a sockaddr_in, we must not only set the IP
address and port, but also set the address family to AF_INET. And
while at it, let's zero-initialize the entire 'struct sockdadr_in'.
Change-Id: I1c8d8fe7f79a2ec737baa7800247269c3271983e
we cannot use "nsi->nsip.remote_ip", as this address is not set
when SNS is in use. We can only have a valid nsi->nsip.remote_ip
if there's only a single NS-VC inside the NS Instance, as this would
connect() the UDP socket to the remote IP/port, breaking any possibility
to have multiple NS-VCs to different SGNS-side IP addresses.
Closes: OS#3845
Change-Id: Ic094621eb01d7458063f531289d5eeadf52bf330
Section 6.2.1 of 3GPP TS 48.016 states:
> A pre-configured endpoint shall not be used for NSE data or signalling
> traffic (with the exception of Size and Configuration procedures) unless
> it is configured by the SGSN using the auto-configuration procedures.
However, in the current SNS implementation, the initial IP/Port over
which we perform the SNS-SIZE + SNS-CONFIG are treated as one of the
normal NS-VCs. Specifically, we also perform the NS-ALIVE procedure on
it, which is clearly wrong.
Let's explicitly create the "initial" NS-VC with data and signalling
weight of 0, and ensure we never start the alive timer or send any
non-SNS PDUs on this connection as long as SNS was not used to change
either of the two weights to non-zero.
While at it, also safeguard against processing any incoming non-SNS
messages on such a all-zero-weight connection.
Change-Id: I16a91a07e5914d123b2ea2f8413b94e7cd518628
Closes: OS#3844
The NS implementation part of the Gb implementation libosmogb
so far implemented a rather classic dialect of Gb, with lots of
heritage to FR (Frame Relay) transports. At least since Release 6
of the NS specification, there's an IP Sub-Network Service (SNS),
which
* permits for dynamic configuration of IP endpoints and their NS-VCs
* abandons the concept of a NSVCI on IP transport
* forbids the use of RESET/BLOCK/UNBLOCK procedures on IP transport
This commit introduces BSS-side IP-SNS support to libosmogb in a
minimally invasive way. It adds a corresponding SNS FSM to each NS
instance, and implements the new SIZE/CONFIG/ADD/DELETE/CHANGE_WEIGHT
procedures very closely aligned with the spec.
In order to use the SNS flavor (rather than the classic one),
a BSS implementation should use gprs_ns_nsip_connect_sns() instead
of the existing gprs_ns_nsip_connect().
This implementation comes with a set of TTCN-3 tests in
PCU_Tests_RAW_SNS.ttcn, see Change-ID
I0fe3d4579960bab0494c294ec7ab8032feed4fb2 of osmo-ttcn3-hacks.git
Closes: OS#3372
Closes: OS#3617
Change-Id: I84786c3b43a8ae34ef3b3ba84b33c90042d234ea
This function performs sending a NS-ALIVE PDU and starting Tns-Test,
let's use it in all places where we used to do that.
As part of this, also fix a bug where the sendto() return value (number
of bytes sent) would actually propagate up all the way to
gprs_ns_rx_reset() return value, which in turn affects the test results
on stdout.
Change-Id: I4d303117f77fabb74bbb91887b9914a81c2a084a
Modern NS specifications contain a SNS (Sub Network Service) for
negotiating IP/port/weight parameters of NS-over-IP links dynamically.
This patch adds message encoding routines for SNS-CONFIG, SNS-SIZE
and their respective acknowledgements.
Related: OS#3372
Change-Id: I5c47e1c3c10deb89a7470ee2c03adfc174accc93
When I added the definitions for the IP-SNS in commit
f030b210e8 back in 2010, I forgot to update
the string definitions in ns_cause_str[]. Let's fix that
Change-Id: I419ccc482d99b01263a60aede83dacd2d9de56ab
According to Section 9.2.4 of 3GPP TS 48.016, the NS-BLOCK-ACK PDU has a
mandatory NSVCI IE which we so far were missing.
Change-Id: Ie7205e99d57f1e42d941f1be2460d8c9f46aadfe
Closes: OS#3808
gcc-8.2 is printing the following warning, which is an error
when used -Werror like our --enable-werror:
In file included from gprs_bssgp.c:34:
In function ‘tl16v_put’,
inlined from ‘tvlv_put.part.3’ at ../../include/osmocom/gsm/tlv.h:156:9,
inlined from ‘tvlv_put’ at ../../include/osmocom/gsm/tlv.h:147:24,
inlined from ‘msgb_tvlv_push’ at ../../include/osmocom/gsm/tlv.h:386:2,
inlined from ‘bssgp_tx_dl_ud’ at gprs_bssgp.c:1162:4:
../../include/osmocom/gsm/tlv.h:131:2: error: ‘memcpy’ forming offset [12, 130] is out of the bounds [0, 11] of object ‘mi’ with type ‘uint8_t[11]’ {aka ‘unsigned char[11]’} [-Werror=array-bounds]
memcpy(buf, val, len);
Where "130" seems to be the maximum value of uint8_t, shifted right one +
2. But even as we use strnlen() with "16" as maximum upper bound, gcc
still believes there's a way that the return value of gsm48_generate_mid_from_imsi()
could be 130. In fact, even the newly-added OSMO_ASSERT() inside
gsm48_generate_mid() doesn't help and gcc still insists there is a problem :(
Change-Id: I0a06daa19b7b5b5badbb8b3d81a54c45b88a60ec
The longest BCd-digit type identity is the IMEISV with 16, so there's
no point in trying to parse up to 255 decimal digits, which will do
nothing but to overflow the caller-provided output buffer.
Let's also clearly define the required minimum size of the output
buffer and add a reltead #define for it.
Change-Id: Ic8488bc7f77dc9182e372741b88f0f06100dddc9
Since commit 797558ea17 we send the
NS_UNBLOCK_ACK message before dispatching the NS_UNBLOCK signal,
instead of afterwards.
Add comments which explain the intended order of events.
Suggested-by: Pau
Related: OS#2388
Change-Id: I4b93853c952a97302f8afc14f462f22c3e487564
In gprs_ns_process_msg(), we were dispatching the S_NS_UNBLOCK
signal before sending out the NS_POUT_UNBLOCK_ACK message.
Signal handlers might send messages to the other side, assuming
that NS is now unblocked. However, since such messages will arrive
before the UNBLOCK_ACK message the receiver might discard them.
This problem has been observed with our TTCN3 BSSGP_Emulation
as a peer to osmo-pcu.
This patch makes TTCN3 PCU TC_paging() test pass regardless of
whether the test or osmo-pcu is started first. Before this patch,
this test would only pass if the test was started before osmo-pcu.
A remaining problem is that the test does not yet keep passing
reliably unless osmo-pcu is restarted between test runs.
Change-Id: I3af54a14bb6bcfa167c9a9d9f67835e7f5b9f1bb
Related: OS#2890
Related: OS#2388
If we cannot bind the listening socket, reset related fields in
the osmo fd structure to NULL again. Otherwise our caller might
eventually try to use an uninitialized osmo fd.
Change-Id: Ia953b2eff54cac0bd980944291f75db14df09a34
Related: OS#3643
The intention was to use the file's basename, but __BASE_FILE__ means "the root
file that is being parsed and contains #include statements".
If we had a function using __BASE_FILE__ and that was defined in an #included
file, __BASE_FILE__ would indicate the first file where the #include is, and
not the file where the function is defined. __BASE_FILE__ works for us because
we don't ever include function definitions that log something, so __BASE_FILE__
always coincides with __FILE__ for our logging; but still __BASE_FILE__ is
semantically the wrong constant.
Related: OS#2740
Change-Id: Ibc1d3746f1876ac42d6b1faf0e5f83bd2283cdcc
To reset the state of BSSGP allow to flush the BSSGP queues.
When testing (with TTCN3) the test object should be resetted between
each test.
Introduce the functions:
bssgp_fc_flush_queue() - flushs a single flow control object
bssgp_flush_all_queues() - flushs queues of all BSSGP connections
Change-Id: I29b6ad6742ddf9b0b58b4af37d9a1cf18e019325
Return "invalid mandatory information" error status to
the sender in case bssgp_tlv_parse() failed.
To avoid loops, do not respond with an error status to
STATUS PDUs which failed parsing.
Change-Id: If73719b75a94d6742bdefc9b6572525cb00a96ee
Related: OS#3178
The return code from bssgp_tlv_parse() was not checked for a parsing
error. In case of a parsing error the stored return code could have
been overwritten later in this function.
Explicitly check for a parsing error and log corresponding packets.
Change-Id: Id3d7c52ec3df2bcf4efcee0e0b14fe22ef96964e
Related: OS#3178
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
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
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
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
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
Alexander Chemeris
Pau Espin
Harald Welte
Oliver Smith
Daniel Willmann