In general, it's safe not to use talloc API here because those are
internal allocations, and there are no 'return' statements between
calloc() and free(). However, we don't really need to initialize
the heap memory with 0, so let's use the 'normal' malloc().
Change-Id: I6956cbd83b2999dbcf8e2d210134b0a166c33efb
When the logging framework is not initialized we get an error:
"ERROR: osmo_log_info == NULL! You must call log_init() before
using logging in ..."
There are sometimes situations where some code tries to log before
logging was initialied. This is a problem because the actual log
line with the debug info we need is covered up by the error message
from the logging framework.
Lets introduce a fallback logging function that is called when the
the logging framework is not available. This function can just use
fprintf to output to stderr.
Change-Id: I9b1b0988e02322e3e44fd4ceea3e1bc2d4df3c45
The event names contain '.', and there are spaces ' ' in the state
names. This is a problem since states and events can also be monitored
via the CTRL interface. Unfortunately the CTRL interface does not allow
certain reserved characters. So lets rename the states and event names
to make them compatible with the CTRL interface.
Change-Id: Id19973b56f9d7b1e3d0b0d7c7d0be7beba5428fc
Related OS#4149
Change-Id: I5ebc9ab5b1456fee29aa4e254fae862dc053f0aa
The parameters described in the docstrings for osmo_plmn_from_bcd() do not match the actual parameter list.
Change-Id: Ic0999dbe096a98418db7482bd110e20497d8e4a5
The identifier of stats item STATS_TCP_REORD_SEEN is tcp:sndbuf_limited,
it should be tcp:reord_seen.
Change-Id: If3539ceb570ae784cc9b6567c59da7afd11acf82
Related: OS#5701
This allows init-passive users to get the configured sizes for the RFCIs
and other similar information once engotiated with the peer.
Realted: OS#1937
Change-Id: I63ee780b4aa162ea097410b234e73984000c0965
When picking the end state, looking only at the path metric
is highly suboptimal because in a tail biting code, we _know_ that
whatever treillis path is correct, it must start and end at the same
state. So we only consider path meeting that condition. We know any
path that doesn't isn't the right one. We only fallback to only
path metric if no path met that condition.
Fixes OS#4508
Signed-off-by: Sylvain Munaut <tnt@246tNt.com>
Change-Id: I87e51d3880c0fe7bf3d6cd08fd46517a424a230c
The function clearly specified in its documentation that the number of
bytes written to the out buffer were being returned. However, the value
returned was "the number of characters (excluding the terminating null
byte) which would have been written to the final string if enough space
had been available.", aka snprintf-style.
The 2 callers of that function were not expecting it, so if a long
enough buffer was passed, the program asserted.
Closes: OS#5383
Change-Id: I8d71bd1a0dad37606acb8302b05c2ae338112e57
The use of an unsinged integer as for loop counter variable doesn't
work when counting down and comparing with >= 0. The existing code
would be an infinite loop if it wasn't for the (data dependent) break
condition:
>>> CID 243259: Control flow issues (NO_EFFECT)
>>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "i >= 0U".
572 for (i = 15; i >= 0; i--) {
573 if (match_mask & (1<<i)) {
574 iui->mode_version = i;
575 break;
576 }
Change-Id: I019d0f0d8f2b167575a2883a13cca692c96961cf
Closes: CID#243259
This is to fix the following compile error on CentOS 7:
[ 74s] stats_tcp.c: In function 'fill_stats':
[ 74s] stats_tcp.c:138:15: error: 'struct tcp_info' has no member named 'tcpi_notsent_bytes'
[ 74s] tcp_info.tcpi_notsent_bytes);
[ 74s] ^
Closes: OS#5374
Change-Id: Icde6651baeb0828477dbf540a02b16a1a5f91797
osmocom applications are deployed in a variety of different situations.
Dependung on the medium that interconnects the network components
unexpected behaviour may occur. To debug problems with the
interconnection between network components it might help to monitor the
health of the related TCP connections.
Change-Id: I1416f95aff2adcf13689646b7574845de169fa3d
Related: SYS#5701
Only support for SMpSDU mode is introduced in this commit.
Not supported explicit list:
- Transparent mode
- ATM/AAL2 based Transport layer
- GTP-U based Transport Layer
- Iu Rate Control procedure
- Time Alignment procedure
APIs are provided to allocate the primitives properly inside the related
msgb. This way primitives can be placed in the headroom, leaving the
data part of the msgb for the IuUP payload, hence allowing re-use of the
msgb and 0 copy of IuUP payload when forwarding data over RNL<->TNL.
Since RNL and TNL primitives relu struct osmo_prim_header, which is not
packed, they cannot be set to packed, and hence proper memory alignment
in the msgb must be done to avoid misaligned accesses (Asan errors about
it otherwise).
Related: SYS#5516
Change-Id: Ibe356fa7b1abaca0091e368db8478e79c09c6cb0
Just like rate_ctr_group_free, osmo_stat_item_group_free should tolerate
it when the argument is NULL
Change-Id: I23323833e7268356a50c4fc6a19639c4ecd2a101
Remove the paragraph about writing to the Free Software Foundation's
mailing address. The FSF has changed addresses in the past, and may do
so again. In 2021 this is not useful, let's rather have a bit less
boilerplate at the start of source files.
Change-Id: I5050285e75cf120407a1d883e99b3c4bcae8ffd7
gsmtap_sendmsg() places the burden of freeing the msgb in case of
erroneous return codes on the caller. A review of existing users
shows that this is overly optimistic and many calls get it wrong,
opening up memory leaks.
Let's add a new function gsmtap_sendmsg_free() which behaves like
gsmtap_sendmsg() but always takes ownership: Either it is sent + freed,
or it is just freed.
Change-Id: I106b09f2a49bf24ce0e8d11fd4d4ee93e9cafdf5
Related: OS#5329
This kind of API will likely cause memory leaks in case the caller fails
to check the return value of the function and knows he must free the
message.
Change-Id: I7e61c19d32a75e28f08b74a8e3d9d63a2d8bf3d7
Related: OS#5329
In case osmo_wqueue_enqueue_quiet() fails, msgb ownership is not
transferred to the queue, but the caller is responsible for freeing
the message buffer that we just failed to enqueue.
Change-Id: I6306e34dc7289864c889e72adf31d74d4581a810
Closes: OS#5328
Related: OS#5329
The log message is very confusing if printed for PtP-BVCI as well. Move
it into the correct if branch.
Change-Id: I0359443ddc52108b492f741005c4699e06b40183
gcc complains because our char might or might not be signed depending on
arch and phase of the moon:
error: array subscript has type 'char' [-Werror=charsubscripts]
Change-Id: I7c76f9a2318c4f0e5eedeea00ec380824b86567e
The bitvec_read_field() is used in performance critical places,
such as the CSN.1 decoder in osmo-pcu. Thus the less conditional
statements we have in the parsing loop, the better.
The bitvec_get_bit_pos() alone is quite a complex function, which
does check the boundaries and even supports the L/H syntax. Even
if it gets inlined by the compiler, we don't really want to run
redundant checks and run bitval2mask() on each iteration.
Change-Id: I438fc82d33ab2edbabd4215ec7bc46afb07d50ab
While running a sanitized version of the bitvec_test I get:
bitvec.c:492:24: runtime error: shift exponent 64 is too large
for 64-bit type 'long unsigned int'
This error is triggered by the following line in the bitvec_test:
_bitvec_read_field(0, 8 * 8 + 1); /* too many bits */
which basically tries to parse more bits (65) than the test vector
actually has (64). The problem is that we don't check if the
given vector has enough data *before* entering the parsing loop,
so we end up doing weird bit-shifts and getting weird values:
bitvec_read_field(idx=0, len=65) => bd5b7ddffdd7b5db (error)
Unfortunately, this problem remained unnoticed so far because in
'tests/testsuite.at' we don't check if stderr is empty. This is
fixed in a follow up change [1].
Rather than checking for errors in every loop iteration, do this
once and return early if the overrun is possible with the given
offset and length arguments.
Change-Id: I4deeabba7ebb720cdbe7c85b37bc011d05bdfa65
Related: [1] Ia82b92eddb18dc596881abcef2f098dc7385538b
This function returns an *unsigned* integer (uint64_t), so returning
a negative value on error is a bad idea. A negative value turns into
a huge positive value, what was demonstrated in the bitvec_test:
bitvec_read_field(idx=512, len=16) => ffffffffffffffea
bitvec_read_field(idx=0, len=65) => ffffffffffffffea
bitvec_read_field(idx=64, len=16) => ffffffffffffffea
The 0xffffffffffffffea above is basically:
(uint64_t) -EINVAL, or
(uint64_t) -22 + 1, or
0xffffffffffffffff - 0x16 + 1.
Let's make use of the errno in order to indicate an error to the caller.
Change-Id: I2cc734caa3365d03c2ae2b3f2cd9544933c25e9e
Related: OS#4388
It's the usual naming for unit test binaries. Without the '_test' endig,
the tdef_vty_test_{config_root,config_subnode,dynamic} binaries do not
match the 'tests/*/*_test' pattern and appear as untracked files in git.
Change-Id: I828fa45132e11a41c527d4b25df850c19871cb75
There might be library code that has rate counters, and if the main
program calls rate_ctr_init() a second time, we can skip the second
initialization.
Change-Id: I6f5342a77518599eb5ac9a0f0605917a78fcc387
alive_timeout_handler() changes the state to RECOVERING which calls
ns2_st_alive_onenter()->ns2_nse_notify_unblocked(unblocked=false)->
ns2_sns_notify_alive(unblocked=false)
When all (signalling) NSVCs have failed and gss->role is SGSN and not
persistent sns_failed() calls gprs_ns2_free_nse() which talloc_free()s
the nse before returning.
The next line in ns2_nse_notify_unblocked() tries to read nse->alive which then causes the
use-after-free.
Change-Id: I0486a77fd3e21fd3904bd19e4e0225ffbf654935
Related: OS#5302
Recent commit introduced the "blocking-io" param to "log stderr" VTY
command, which calls log_target_file_switch_to_{stream,wqueue}.
The VTY command already locks the log_tgt_mutex mutex, since it has to
access the tgt list. However, the functions mention above also want to
lock the same mutex in order to log information. Let's drop the logging
to avoid the double lock, and update its documentation to mention it
must be called with the lock already held, as documented on other
similar functions.
The issue can be spotted when running osmo-trx-uhd:
"""
(gdb) bt
#0 0x00007ffff75d7600 in __lll_lock_wait () from /usr/lib/libpthread.so.0
#1 0x00007ffff75d0503 in pthread_mutex_lock () from /usr/lib/libpthread.so.0
#2 0x00007ffff66314fb in log_tgt_mutex_lock_impl () at /git/libosmocore/src/logging.c:130
#3 0x00007ffff6638e74 in log_check_level (subsys=8, subsys@entry=-1, level=level@entry=3) at /git/libosmocore/src/logging.c:1510
#4 0x00007ffff6639c91 in log_target_file_switch_to_wqueue (target=target@entry=0x611000000320) at /git/libosmocore/src/logging.c:1186
#5 0x00007ffff68565d3 in cfg_log_stderr (self=<optimized out>, vty=0x6140000018a0, argc=0, argv=<optimized out>) at /git/libosmocore/src/vty/logging_vty.c:859
#6 0x00007ffff683db3d in cmd_execute_command_strict (vline=0x60b0000dfe80, vty=vty@entry=0x6140000018a0, cmd=cmd@entry=0x0) at /git/libosmocore/src/vty/command.c:2768
7 0x00007ffff683e396 in config_from_file (vty=vty@entry=0x6140000018a0, fp=fp@entry=0x615000036400) at /git/libosmocore/src/vty/command.c:2880
8 0x00007ffff684cedb in vty_read_config_filep (confp=confp@entry=0x615000036400, priv=priv@entry=0x0) at /git/libosmocore/src/vty/vty.c:1529
9 0x00007ffff684ebfc in vty_read_config_file (file_name=0x7fffffffe7d8 "/build/new/conf/osmo-trx-uhd.cfg", priv=0x0) at /git/libosmocore/src/vty/vty.c:1920
10 0x0000555555565270 in main (argc=3, argv=0x7fffffffe3c8) at /git/osmo-trx/Transceiver52M/osmo-trx.cpp:652
"""
Debugged by rebuilding libosmocore with "LOG_MTX_DEBUG 1":
"""
/libosmocore/src/logging.c:1510 [log_check_level] lock
/libosmocore/src/logging.c:1522 [log_check_level] unlock
/libosmocore/src/vty/logging_vty.c:844 [cfg_log_stderr] lock
/libosmocore/src/logging.c:1510 [log_check_level] lock
"""
Fixes: b72867f0e6
Related: OS#4311
Change-Id: Idb4215fa2f364e28c0bb73fb9975b6c9f50a46f6
Vadim Yanitskiy
Philipp Maier
Harald Welte
Neels Hofmeyr
Pau Espin
Sylvain Munaut
Oliver Smith
Daniel Willmann
Eric Wild