For example encode_auth_info() from gsup.c calls
msgb_tlv_put(msg, iei, 0, NULL)
to put a tag and len with content data following later.
However, this would cause a memcpy() from a NULL pointer, in tlv_put(). Allow
passing NULL and len = 0 for cases like the above:
If val is NULL, use memset(0) instead of memcpy().
If len is zero, do not copy nor memset anything.
Hence make tlv_put() behave in a well-defined and valid way for any and all
input args; no negative fallout is possible from this patch.
Add proper API doc comment.
Fixes a sanitizer build failure in gsup_test:
../../../../src/libosmocore/include/osmocom/gsm/tlv.h:99:2: runtime error: null pointer passed as argument 2, which is declared to never be null
Helps fix sanitizer build on debian 9.
Change-Id: I13dce9cd1228817890d3e81edeeb660c893c1d64
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
Print remaining msgbs when done, then free the entire tall_msgb_context. To be
able to do that, call msgb_talloc_ctx_init() and use its return value.
A subsequent patch will fix a known mem leak and add assertions for 0b in 1
blocks remaining in the tall_msgb_context.
Helps fix sanitizer build on debian 9.
Change-Id: I67d347ab2642b0bfc27b21b44231a7f3146ff641
The test fills up the queue / sends too large PDUs on purpose. Make that
obvious by outputting returned errors in the expected output.
Cosmetic:
- fc_in()'s return value is ignored, hence don't return anything.
- add comment.
Change-Id: I57d6fce2515a65f6dd037e75af5397079215cb46
Ever since this test was changed to use osmo_gettimeofday_override, the times
it sees are exact every time and don't need rounding to pass the expected
output.
Change-Id: I4a9a5d31fc02eb55caf7ba9c141426d8115bb740
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
It was decided that osmo-mgw as direct successor of osmo-bsc_mgcp
will use the same VTY port number (similar to osmo-nitb, osmo-bsc
and osmo-bsc-sccplite all using the same VTY port number)
Change-Id: Iec1da9f3b4d170416279f05876d9e1ae2970c577
Remove initial msgb talloc context creation: if we create a root ctx for msgb
that all msgb are allocated in, we would in a final cleanup discard all msgbs,
i.e. we would not verify that all msgb are cleaned up properly.
If we create the msgb context and *don't* clean it up in the end, the sanitizer
build fails because the context root is not cleaned up.
Easiest is to actually allocate all msgb at NULL ctx, because then any msgb
that aren't cleaned up properly would still linger, while we don't leave a root
ctx that we need to clean up either.
Helps fix sanitizer build on debian 9.
Change-Id: I1f2d1d05c75bbf4d92787f9735083f18cdc90f6f
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
The testsuite fails on some specific build machines in the OBS
build cluster. Let's try to figure out which CPU flags they have
to narrow down the cause of this.
Change-Id: Ib23e5bfb3c894206fad62d6cc6151583b1bb75a6
Fixes the warning below:
warning: ‘sqn’ may be used uninitialized in this function [-Wmaybe-uninitialized]
fprintf(stderr, "Requesting --sqn %"PRIu64" implies IND=%u,"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
" so no further --ind argument is allowed.\n",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sqn, test_aud.u.umts.ind);
~~~~~~~~~~~~~~~~~~~~~~~~~
The warning is a false positive as sqn is only used in case
sqn_is_set!=0, and in that code path, sqn is set.
Change-Id: Ib5903db01ea6765bd6bb688e63f70925c5012f98
"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
Following I5021c64a787b63314e0f2f1cba0b8fc7bff4f09b a deprecation of
vty_install_default() and install_default() commands is indicated.
However, compiler warnings may clutter build output or even fail strict builds,
hence I am submitting the deprecation in a separate patch.
Depends: I5021c64a787b63314e0f2f1cba0b8fc7bff4f09b
Change-Id: Icf5d83f641e838cebcccc635a043e94ba352abff
In Change-Id Ic77866ce65acf524b768882c751a4f9c0635740b we introduced a
gnutls fall-back for random number generation, and made this a default
unless explicitly disabled at compile time. This means the debian
package needs related build dependency.
Change-Id: I918e4b7bf1cb621679dce6339b3c4b69d653e2a6
Related: OS#1694
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
This is for Change-Id Icf337a430fb367bbca48a1b02822a2cb3b644e5f
to catch repeated calls to osmo_init_logging() whcih is what osmo-pcu
will perform after some upcoming fixes there.
Change-Id: Ie14a7eadf8ba01e8d31a83e330491036358ac650
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
See osmo-ci change I2409b2928b4d7ebbd6c005097d4ad7337307dd93 for rationale.
Depends: I2409b2928b4d7ebbd6c005097d4ad7337307dd93
Change-Id: I99fdd20e7b18c2f4a59a9db35e5efccca6546b77
Sounds stupid, but we actually didn't support hex nibbles in one of
the two directions of the conversion, so let's make sure we test for
this.
Change-Id: I8445da54cc4f9b1cd64f286c2b238f4f7c87accb
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
In Change-Id Ifc6ac824f5dae9a848bb4a5d067c64a69eb40b56 we introduce
name mangling to replace any '.' in counter (group) names to be
converted to ':'. Let's test for this functionality explicitly as part
of the stats_test.
Change-Id: Ie35682aa79526e2ffeab6995cd640b7847d855bf
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
As rate counters are automatically exposed on the CTRL interface,
we need to make sure they don't contain special characters such as '.'
which are not permitted/supported by CTRL.
In order to be able to run old versions of osmocom programs with
libosmocore versions after this commit, we introduce some special
name mangling: Any '.' in the names are replaced with ':' during
counter group registration, if valid identifiers can be obtained
this way.
Change-Id: Ifc6ac824f5dae9a848bb4a5d067c64a69eb40b56
Let's enforce that the names of FSMs and their instances are valid
osmocom identifiers. This is important as the FSMs are automatically
exported via those names on the CTRL inteface, and we have to make sure
CTRL syntax actually permits them.
Change-Id: I9ef59432f43a3cdb94e4cbb0c44ac3f9b2aac0f2
We define the notion of an 'osmocom identifier' which is basically a
7-bit US-ASCII without any special characters beyond "-_:@". We
introduce a function to verify if an identifier consists only of the
permitted characters.
Change-Id: I96a8d345c5a69238a12d040f39b70c485a5c421c
Change I24e500e132f5c8e8133d35548cb7b4e4552331d0 was merged, but IMHO needs
improvement. Fix these:
- clean the git source tree before each build step, in common prep_build().
- fix indenting inside the build() macros.
- change build() arg to be build_dir, to absorb cleanup steps into build().
- in jenkins.sh, use $ENABLE_SANITIZE as global env, not passed as arg.
- in jenkins.sh, don't do 'make distcheck' twice. It is not necessary to do it
from source tree as well as separately from source tree, since distcheck
already moves to a different build dir.
Change-Id: I09d306350602f21943d5bd45f7388c83ede9b524
Combine the C source file name and the string name into the fsm's internal name
token, and use it in most places instead of the plain struct name: osmo-bsc's
new FSMs have identical struct names in each static c context.
Output in a file name that includes all of these more detailed name tokens.
Also parse '(1 << EVENT)' as event names.
Note that besides this patch, there are also some tweaks to the osmo-bsc patch
that improve the fsm-to-dot experience...
- call fsm-to-dot for single files to avoid name conflicts, or rename each
struct as a unique name.
- Add comments for the event name a callback is intended for, so that not all
transitions are interpreted as TEARDOWN (because it is invoked in common
error handling, which causes the script to interpret it as the causing
event). (or change the event-checking if into a switch that names the valid
event and has a default case for all others.)
Change-Id: Ib60df7fd19efc99ba9fe797f14c0e3239c4bea20
When dumping NSE via vty:
* check which local address would be used to communicate with a given
NSE and print it
* print link layer type last to make output more consistent
Change-Id: I6932a29c7899d36bcc275f05dda9670b0e69bef0
Related: SYS#3610
* add comment about underlying assumption that structs in ip/frgre union
members in gprs_nsvc struct have the same memory layout
* remove such assumption from gprs_ns_ll_str()
* use gprs_ns_ll_str() for NSE dump
Change-Id: Idcb912b7b3f7460fd2b058e16650c0bde8f757ee
Neels Hofmeyr
Harald Welte
Pau Espin
Philipp Maier
Neels Hofmeyr