if (ptr)
msgb_free(ptr)
extends to:
if (ptr)
talloc_free(ptr)
And according to the talloc documentation a talloc_free(NULL)
will not crash: "... Likewise, if "ptr" is NULL, then the function
will make no modifications and returns -1."
Handling 7-bit coding is a little different for USSD, as TS 03.38
states:
To avoid the situation where the receiving entity confuses 7 binary
zero pad bits as the @ character, the carriage return or <CR>
character shall be used for padding in this situation [...].
If <CR> is intended to be the last character and the message
(including the wanted <CR>) ends on an octet boundary, then another
<CR> must be added together with a padding bit 0. The receiving entity
will perform the carriage return function twice, but this will not
result in misoperation as the definition of <CR> [...] is identical to
the definition of <CR><CR>.
The receiving entity shall remove the final <CR> character where the
message ends on an octet boundary with <CR> as the last character.
Jacob has verified the fix with fakeBTS and the wireshark dissector.
Fixes: OW#947
Reviewed-by: Jacob Erlbeck <jerlbeck@sysmocom.de>
Compiled with ubuntu 1204 (precise), where -Wformat-security is enabled by
-Wall.
Test yields ok, but the current implementation doesn't properly support
multi-character separators and end strings. So the test output is truncated.
Addresses:
utils.c: In function 'vty_cmd_string_from_valstr':
utils.c:84:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:84:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:108:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:108:2: warning: format not a string literal and no format arguments [-Wformat-security]
The code most likely wanted to check the result of argv_concat.
To do this we need to dereference the dptr.
Logically dead code (DEADCODE)
dead_error_line: Execution cannot reach this statement "return 1;
Fixes: Coverity CID 1040675
lapdm.c takes the re-establishment message and forwards it to lapd_core.c,
so we can assume that msgb is set at primitive. In case there is data in
the re-establishment msg, it is moved into send_buffer. In case of no
data (0 length), it must be freed.
Fixes an issue spotted by Coverity Scan.
This reverts commit f996b05dbd
and 2b0cac4ef8. A detailed
explanation can be found here:
http://lists.osmocom.org/pipermail/openbsc/2013-July/004737.html
The short description is that:
1.) The API should return (as out parameter) the number of
octets used.
2.) The handling for the <CR> encoding only applies to USSD
and it is incomplete. On top of that it broke the SMS test.
To avoid the situation where the receiving entity confuses 7 binary zero pad
bits as the @ character, the carriage return or <CR> character shall be used
for padding in this situation.
If the datalink fails or if handover or assignment to a new channel fails,
it is re-establised by sending SABM again. The length of establish message
is 0 in this case. The length is used to differentiate between
re-establishment and contention resolution, which has to be handled
differently.
See TS 04.06 Chapter 5.4.2.1
It is impossible that the snprintf will fill the entire namebuf
but just follow the idiom to make sure it is null terminated.
Related: Coverity CID 1040676
These routines were not freeing vectors used for the lookup. On
review it is fixing another path not detected by coverity.
The danger is a double free in tab completion now. It is difficult
to test this.
Fixes: Coverity CID 23037, CID 23038
The &buf[3] is unlikely to be aligned properly. Use memcpy instead
of an assignment. Add a small testcase that verifies that I didn't
mess up the conversion.
Alignment trap: osmo-nitb (3293) PC=0x492b7094 Instr=0xe5803003 Address=0xbeb259db FSR 0x801
This already came up during review but now that Coverity complains
about it as well, let us just remove it. The variable is unsigned
so it can never be < 0.
Fixes: Coverity CID 1040669.
If the BTS tells us to not send any data at all anymore (bucket leak
rate of 0 bits per second), then we should respect this and not run into
a divide-by-zero. However, as this indicates complete overload, we
print a log message to that regard.
When a SABM(E) frame arrives, we have to trim the L2 padding (0x2b for
gsm) before handing the data off to L3, just like we do with I frames.
Also, we should use mggb_trim() or even msgb_l3trim() instead of
manually fiddling with msgb->length and ->tail pointers.
After reception of SABM, the network responds with UA and enters the
establised multiframe state. If UA is not received by mobile, the SABM
is transmitted again, and the network must respond with UA again, unless
it is from a different mobile.
Add LAPDm collision test (contention resolution on network side).
Only the Gb library relies on having undefined references to a
symbol that needs to be provided by the host application. For
all other libraries we can link with -no-undefined.
Seems the script I used to parse those had a bug where range of
bits in the 'decreasing' direction ( like 6..0 ) were not processed
properly.
Thanks to Andreas for noticing this !
Signed-off-by: Sylvain Munaut <tnt@246tNt.com>
Commit cd6ed82d1f made "EVERYTHING"
map to LOGL_DEBUG but when writing out the configuration the following
would be written:
logging level all unknown 0x0
This happend because no string was found for the value 0. Address it
by adding a legacy check for 0 and write out the str from the index
0. Currently this is "EVERYTHING".
The log target can be used via log alarms and show alarms.
Why? This feature was proposed/requested at
http://openbsc.osmocom.org/trac/wiki/Tasks/ErrorLogTarget
All messages use the same amount of space, prioritizing simplicity.
The log target can be used via log alarms and show alarms.
Why? This feature was proposed/requested at
http://openbsc.osmocom.org/trac/wiki/Tasks/ErrorLogTarget
All messages use the same amount of space, prioritizing simplicity.
As Holger points out "logging level XXX everything" wasn't working, as
it sets category->loglevel to 0, which is checked in osmo_vlogp() and
will never get logged.
This hides HIDDEN or DEPRECATED commands from showing up when the
full list of commands is inquired with '?' at any given point in the
command tree. Only if the hidden/deprecated command is already typed
in partially, then it will still tab-complete.
this will avoid printing 'abis_nm.c' as the filename in the log, which
is pretty useless during debugging. We want to know where
abis_nm_debugp_foh() is being used from, not where it is implemented.
We used 1ULL at one place and not the other ... at the same time,
we now use (uintXX_t) so that the proper type is used each time.
Signed-off-by: Sylvain Munaut <tnt@246tNt.com>
getaddrinfo returns EAI_SERVICE (-8) if that combination is used.
More information available in here:
http://sourceware.org/bugzilla/show_bug.cgi?id=15015
Reported by Holger Hans Peter Freyther.
While at it, this patch also removes hints.ai_flags = 0 as memset
to zero already happened just a bit before that.
This is essentially http://patchwork.diac24.net/patch/271/ forward
ported to libosmovty
Original-by: Paul Jakma <paul@quagga.net>
Signed-off-by: Sylvain Munaut <tnt@246tNt.com>
The second loop in osmo_revbytebits_buf() in src/bits.c grabs
4 bytes each iteration, which can easily go past the supplied
input in some cases.
Compiled with -fstack-protector , I get a "stack smashing detected"
in the bits test.
From: Nils O. Selåsdal <noselasd@fiane.dyndns.org>
Signed-off-by: Sylvain Munaut <tnt@246tNt.com>
This was found while implementing handover on a sysmobts. When we
receive a channel release request for a channel that was never really
activated (set_lapdm_context() was not called) we segfault in
lapd_recv_dlsap().
We now return early with -EINVAL in rslms_rx_rll() if we receive a
message that assumes set_lapdm_context() was already called.
These are:
* RSL_MT_UNIT_DATA_REQ
* RSL_MT_DATA_REQ
* RSL_MT_SUSP_REQ
* RSL_MT_REL_REQ
A test case was added to trigger the issue.
GCC 4.7.2 was already smart enough to see that the table is const
so there is no change in the generated assembly code. For some reason
the dispatch is still going through one relocation.
When OpenBSC is handling more than one message at a time it is difficult
to see which log message belongs to which SMR instance. Introduce a
uint64_t id that can be set to the row_id/message_id and prefix all
log messages with SMR(ID).
This change is ABI and API incompatible with previous versions of
libosmogsm.
When OpenBSC is handling more than one message at a time it is difficult
to see which log message belongs to which SMC. Introduce a uint64_t id
that can be set to the row_id/message_id and prefix all log messages
with SMC(ID).
This change is ABI and API incompatible with previous versions of
libosmogsm.
Example:
SMC(100) instance created
SMC(100) message MNSMS-EST-REQ received in state IDLE
When the connection may not released print the name of the current
state to ease with debugging and verification that this is not a
valid state transition.
Use FreeBSD struct iphdr definition for OSX also. From the commentary in
the source file:
On BSD the IPv4 struct is called struct ip and instead of iXX
the members are called ip_XX. One could change this code to use
struct ip but that would require to define _BSD_SOURCE and that
might have other complications. Instead make sure struct iphdr
is present on FreeBSD.
Prior to this fix, a persistent file or syslog log configuration
didn't work across an application re-start, as the
"logging filter all 1" was never saved and thus no messages were
logged.
Introduce a print_filename attribute for each logtarget. Initialize it
with 1 to be backward compatible with earlier versions. The bit is taken
from an existint bitfield. There were at least six bits left of the byte.
Include ctype.h to have a declaration of tolower.
utils.c: In function 'vty_cmd_string_from_valstr':
utils.c:95:6: warning: implicit declaration of function 'tolower' [-Wimplicit-function-declaration]
The comment explains why we don't care about the content of z,
stop storing it.
gsm_utils.c: In function 'gsm_7bit_encode':
gsm_utils.c:253:13: warning: variable 'z' set but not used [-Wunused-but-set-variable]
Applications should keep the log area in a static const area. Mark
the pointer as const to address compiler warnings in OpenBSC, cast
the const away for the osmo_log_info as it is not declared as const.
* add more comments on units of struct members
* make sure to parsre FC-BVC message correctly
* add error message in case user passes PDU larger than bucket size
* add new function to initialize flow control struct
This code is supposed to implement the BSSGP flow control algorithm,
both for the per-BSS and for the per-MS flow control.
The code currently has no test cases, they will come in a separate
commit.
This was fixed in 9c3dc90d16a40789081c84e46620f4d66689fec1 of
openbsc.git, after the sms code had been migrated here:
introduce HAVE_TM_GMTOFF_IN_TM
Not all architectures have the tm.tm_gmtoff member. This fixes cygwin builds.
Holger Hans Peter Freyther
Harald Welte
Sylvain Munaut