Commit Graph

28 Commits

Author SHA1 Message Date
Neels Hofmeyr d27c1186fe debug fu
Change-Id: I97190e5f8d4ab2f4b4ae8ff8d9359ddae39b9645
2020-01-07 18:17:52 +01:00
Neels Hofmeyr ac729eb5a8 add caller-owns-msgb variant osmo_sccp_user_sap_down_nofree()
Add osmo_sccp_user_sap_down_nofree(), which is identical to
osmo_sccp_user_sap_down(), but doesn't imply a msgb_free().

To implement that, sccp_sclc_user_sap_down_nofree() with the same msgb
semantics is required.

Rationale:

Avoiding msgb leaks is easiest if the caller retains ownership of the msgb.
Take this hypothetical chain where leaks are obviously avoided:

  void send()
  {
  	msg = msgb_alloc();
	dispatch(msg);
	msgb_free(msg);
  }

  void dispatch(msg)
  {
  	osmo_fsm_inst_dispatch(fi, msg);
  }

  void fi_on_event(fi, data)
  {
	if (socket_is_ok)
		socket_write((struct msgb*)data);
  }

  void socket_write(msgb)
  {
  	if (!ok1)
		return;
	if (ok2) {
		if (!ok3)
			return;
		write(sock, msg->data);
	}
  }

However, if the caller passes ownership down to the msgb consumer, things
become nightmarishly complex:

  void send()
  {
  	msg = msgb_alloc();
	rc = dispatch(msg);
	/* dispatching event failed? */
	if (rc)
		msgb_free(msg);
  }

  int dispatch(msg)
  {
  	if (osmo_fsm_inst_dispatch(fi, msg))
		return -1;
	if (something_else())
		return -1; // <-- double free!
  }

  void fi_on_event(fi, data)
  {
	if (socket_is_ok) {
		socket_write((struct msgb*)data);
	else
		/* socket didn't consume? */
		msgb_free(data);
  }

  int socket_write(msgb)
  {
  	if (!ok1)
		return -1; // <-- leak!
	if (ok2) {
		if (!ok3)
			goto out;
		write(sock, msg->data);
	}
  out:
        msgb_free(msg);
	return -2;
  }

If any link in this call chain fails to be aware of the importance to return a
failed RC or to free a msgb if the chain is broken, or to not return a failed
RC if the msgb is consumed, we have a hidden msgb leak or double free.

This is the case with osmo_sccp_user_sap_down(). In new osmo-msc, passing data
through various FSM instances, there is high potential for leak/double-free
bugs. A very large brain is required to track down every msgb path.

osmo_sccp_user_sap_down_nofree() makes this problem trivial to solve even for
humans.

Change-Id: Ic818efa78b90f727e1a94c18b60d9a306644f340
2019-04-12 06:27:10 +02:00
Oliver Smith 49eea32ab4 sccp_scoc: conn_create{,_id}() user arg, not inst
Accept the osmo_sccp_user instead of calling conn_create() and setting
conn->user afterwards. Prepare for generating a local_ref inside
conn_create_id() in the future.

Related: OS#3871
Change-Id: I2fb47c8ba6c0ce7cd92c9ac31f15c67eb67fb66e
2019-04-04 08:33:58 +02:00
Oliver Smith ee0bcf2fca sccp_scoc: move sccp_find_user() up
Move it before sccp_scoc_rx_scrc_rout_fail(), so it can be used in
the latter function to figure out the local_ref from the user (follow
up commit).

Related: OS#3871
Change-Id: Ieabeda3126dcc0349a06c0fc7c9e468b900d7855
2019-04-03 15:06:10 +02:00
Oliver Smith 67b895e156 Cosmetic: sccp_scoc: fix local reference comments
Remove all comments, that claim conn_id is the local reference. This
is only a hack, and should not be something to rely on. A properly
separated local reference will be introduced shortly.

Related: OS#3871
Change-Id: I900124037da76caaaf5ecee323eb82e1c6d2e368
2019-04-03 15:06:02 +02:00
Harald Welte 3f2ff81831 sccp_scoc: Add more comments describing conn_id and local_ref members
Change-Id: I85cabc42621103de1a83282baf210fbc117b63db
2019-04-01 15:50:38 +02:00
Neels Hofmeyr 03209f8eb6 debug log typo fix in sccp_scoc.c
Change-Id: Ic5637700122ef26a44932149994c01ccbfc18ffd
2019-03-06 07:19:36 +01:00
Neels Hofmeyr 6a973bab9c build: move include/{mtp,sccp} to include/osmocom/
Anywhere else in the Osmocom code base, we arrange headers in
include/osmocom/foo/ and pass -I ${root_srcdir}/include/.
This way including an osmocom header always has the format
  #include <osmocom/foo/bar.h>
whether we are including from the local source tree or from $prefix.

For some reason not clear to me, the mtp and sccp folders, even though they are
being installed to $prefix/include/osmocom/, were kept *next* to the osmocom/
dir, instead of inside it. Fix that weird situation.

The motivation is that I wanted to use a definition from sccp_types.h in a
public-API header. That is impossible if it requires
  #include <sccp/sccp_types.h>
in a local build, but
  #include <osmocom/sccp/sccp_types.h>
for any other source tree using libosmo-sccp. After this patch, both are
identical and including works without quirks. (The other patch that needed this
has changed in the meantime on and no longer needs this, but this still makes
sense for future hacking.)

The installed result does not change, since both mtp/*.h and sccp/*.h have
always been installed to $prefix/include/osmocom/{mtp,sccp}/. This merely
changes their position in the source tree.

The most curious situation before this is that any patch #including
<osmocom/sccp/sccp_types.h> might not get a notice that the header didn't
exist, but might instead include an older system-installed file.

Change-Id: I1209a4ecf9f692a8030b5c93cd281fc9dd58d105
2018-10-21 12:35:11 +00:00
Neels Hofmeyr bb6e4bb676 make SCCP timers configurable
The previous hardcoded SCCP timers may cause SCCP connection releases, if the
peer is configured with far lower timers than libosmo-sccp. Testing with a
specific SCCPlite MSC, I experienced an iar of just over three minutes, meaning
that calls would be cut off by the MSC, since the osmo-bsc failed to send an
Inactivity Timer message until seven minutes have passed.

With this patch, SCCP timers are configurable by the user.

Define constant global default timers, and variable user-configurable timers
with each osmo_sccp_instance.

Add VTY UI to configure the timers. Users must call osmo_sccp_vty_init() to get
the sccp-timer config nodes under the 'cs7' node. Show the new UI in
ss7_asp_test.vty.

Note that even though this function is not new at all, until recently, all of
our SCCP users (osmo-bsc, osmo-msc, osmo-sgsn, osmo-hnbgw) failed to call
osmo_sccp_vty_init(), and thus also missed out on the various 'show' commands
defined in sccp_vty.c. In other words, to benefit from the timer
configurability, the patches to call osmo_sccp_vty_init() must first be merged
to the corresponding master branches.

If a 'sccp-timer' config command occurs, the cs7 instance must allocate an SCCP
instance in order to store the timer config. Do that by calling the recently
added osmo_ss7_ensure_sccp() function.

Hence remove the limitation that the SCCP instance must not be populated from
the "simple" setup function. If we want to configure SCCP timers beforehand,
there must be an SCCP instance for that, and there is no hard reason to require
a NULL SCCP instance, besides the desire to prevent this function from being
invoked twice.

Change-Id: I28a7362aa838e648ecc9b26ee53dbcade81a9d65
2018-09-27 17:53:40 +02:00
Neels Hofmeyr 2da2179e82 cosmetic: sccp_scoc.c: fix timers definition units
The SCCP timer units are not what the comments say:
Milliseconds are 1000 * seconds, not 100.
Also, microseconds are 1000000 * seconds.

Interestingly enough, MSEC_TO_US() tried to fix the wrong hundredth-seconds to
microseconds by multiplying by 10, however, it should end up at a factor of a
million, not a thousand, hence would result in wrong sub-second fractions.
But, since none of the current timers use sub-second fractions, none of the
timers are actually affected in practice. Hence this patch is merely cosmetic.

Since all use of the timer constants is wrapped in MSEC_TO_US(), we can
transparently fix all those values and their use:

- define the constants in milliseconds (replace "100" with "1000").
- in MSEC_TO_US(), divide-and-mod by 1000.
- in MSEC_TO_US(), actually calculate microseconds.

BTW, I am about to make the timers VTY configurable, but before I get confused,
I'd rather fix these units first.

Change-Id: Ia6c893f734fbdc88873c4ef80f6cacf01ee7763a
2018-09-26 17:13:06 +02:00
Neels Hofmeyr 8254cf4f2a error log: sccp_scoc.c: log failure to create/resolve conn_id
Tweak the FIXMEs to clarify that we're lacking a reply to the SCCP user besides
log output.

Change-Id: Ib235ff8e264aaf0c2e9794f464a3ba7b54816f3d
2018-01-07 22:40:23 +01:00
Harald Welte b393b3f4cc Add SPDX-License-Identifier + missing copyright statements
Change-Id: I113232bbeaa7a835871df7f9b883ba573d8a2534
2017-11-13 01:25:47 +09:00
Neels Hofmeyr a212398f7c consistency: use OSMO_SS7_PC_INVALID for osmo_sccp_user
A previous patch added ss7_instance primary_pc validity checks by means of
OSMO_SS7_PC_INVALID. To be consistent, also adjust sccp_user accordingly.
(see I7f0f0c89b7335d9da24161bfac8234be214ca00c)

Remove the osmo_sccp_user's pc_valid field, replaced by pc=OSMO_SS7_PC_INVALID.
Adjust all code paths.

Simplify some log printing, using the fact that osmo_ss7_pointcode_print() now
outputs "(no PC)" for unset point codes.

Change-Id: I8684c9b559712072c772012890bbf7efa7c8eb35
2017-08-09 13:54:44 +02:00
Neels Hofmeyr b352ca030d ensure valid primary_pc in osmo_ss7_instance
Initialize osmo_ss7_instance.cfg.primary_pc = OSMO_SS7_PC_INVALID.
Adjust all code paths using primary_pc to ensure it is indeed valid.

Rationale:

It looks like we are going to use the primary point-code of an SS7 instance to
derive a local SCCP address, e.g. for osmo-bsc and osmo-hnbgw.

 cs7-instance 1
  point-code 1.2.3   ! sets osmo_ss7_instance.primary_pc = 1.2.3
  sccp-address msc
   point-code 0.0.1
   routing-indicator PC

 hnb
  iucs
   remote-addr msc   ! derives cs7 instance 1 and local pc 1.2.3

If 'point-code 1.2.3' is omitted, this becomes '0.0.0' without the user
noticing, and this happens for each client that omits it. I would like to barf
when no local PC is set.

Change-Id: I7f0f0c89b7335d9da24161bfac8234be214ca00c
2017-08-09 13:54:44 +02:00
Pau Espin eebbec23a5 sccp_scoc: Fix compilation warning and leave a pragma message
Following warning was being printed:
warning: statement will never be executed [-Wswitch-unreachable]

The code in there seems not to be finished, so better leave the code and a
pragma message to get notified when we compile.

Change-Id: I4e2c482803954c984cb6792b11b4ea0fe674e269
2017-07-08 10:46:31 +00:00
Pau Espin 698045d205 sccp_scoc: Fix trailing whitespace
Change-Id: Ia93bb7d59e4e2c31b693e2c3424d34386762d02e
2017-07-08 10:46:31 +00:00
Neels Hofmeyr 81c0bcab06 Revert "SCOC: When sending a CORE/CR, SUA SRC_ADDR == CallingPartyAddress"
This reverts commit 5527df78ad.

I tried some time to figure out what other changes are needed to make this
commit work and fix a confusion, until I noticed:

The commit's *log message* is correct that SRC == calling, but the *patch*
modifies callED addr to be the SRC, which is wrong. So reverting this commit is
indeed the correct way to fix our addresses.

Change-Id: Ic76aacc81f87f8885fe04121aead5c79a761ef07
2017-06-27 21:25:03 +00:00
Neels Hofmeyr 3468d5dce4 add/tweak various logging to help figure out complex routing
Add function osmo_ss7_point_code_print2() to be able to print two point codes
in the same log message.

Change signatures of two static functions to aid logging:
add invalid ref arg to sccp_scoc_rx_inval_src_ref(),
pass conn instead of inst to sccp_scoc_rx_inval_opc().

Change-Id: Ia3243606d6cad7721f7da7f6caba2caa90ae2bbd
2017-06-25 22:35:01 +02:00
Harald Welte 5527df78ad SCOC: When sending a CORE/CR, SUA SRC_ADDR == CallingPartyAddress
SUA uses different semantics (source / destination) address, while SCCP
uses Calling/CalledParty. This leads to some confusion.  At least in the
CR/CORE case, the CallingParty equals the SRC_ADDR.

Change-Id: I5a3c27b112148dd539f092cce7618b4f62fde73c
2017-05-04 09:07:21 +02:00
Harald Welte 982c0ce337 sccp_scoc: don't pass variable as argument if we know it's NULL
xua will always be NULL in one particular switch case of
scoc_fsm_conn_pend_out(), so let's use NULL directly rather than obscure
it though a variable that might be understood as this being non-NULL in
some cases.

Change-Id: Id6dc56442441489aefc706bcebc49197ca3dae1e
Fixes: coverity CID#166934
2017-04-27 12:17:20 +02:00
Harald Welte 8a1f5a72db scu_gen_encode_and_send(): Fix NULL pointer deref
We were using the 'xua' pointer before checkin if it actually is valid

Change-Id: I5cd3250afc0b787b78683cd8ab6b2512e0d5c69e
Fixes: coverity CID#166945
2017-04-27 11:52:43 +02:00
Harald Welte 4c880a02f4 osmo_sccp_user_sap_down(): Avoid uninitialized pointer deref
When receiving an unknown primitive, we end up de-referencing an
unassigned/uninitialized pointer for 'conn'.  Let's properly catch that
case and print an error message.

Change-Id: Id1f5f293ea9bce8601d45164be670a7062d91802
Fixes: coverity CID#166947
2017-04-27 11:48:15 +02:00
Harald Welte c031536808 SCCP: Add VTY interface for SCCP
Change-Id: I100daaa947dbab6a4528c4e9fbd0d30790288f63
2017-04-14 20:25:50 +02:00
Harald Welte f5a030fac8 sccp_scoc: Memorize if a connection is incoming or outbound
This is not really needed by the state machines internally, so we have
to artificially add it to the sccp_connection.  We don't use it yet, but
upcoming code for VTY introspection of SCCP connections will be able to
use it.

Change-Id: Ic3c85152665abfb613e197b098c97392d16d16bf
2017-04-14 20:25:50 +02:00
Harald Welte dff8f995ef SCCP SCOC: Ensure user primitive msgb->l2h always poinst to tail
In case there is no user data in a CONNECT.conf primitive (or other CO
primitives), we must make sure that msgb->l2h = msgb->tail so that the
SCCP User can use msgb_l2len(msg) == 0 as indicator to verify if user
data is present or not.

Change-Id: Ie512fe063391e3a634097f555b9b0089d2981de9
2017-04-11 21:45:25 +02:00
Harald Welte 996dcf3ba6 Fix for SCCP CC without user data
When sending messages like CC (or SUA COAK) without user data, we must
make sure to not include the optional data part - as opposed to
including one with zero length.

Change-Id: If91edb526cbcd792ec5ebcb4518cf848feb69391
2017-04-11 09:42:09 +02:00
Harald Welte c860f9a41e sigtran: fix various memory leaks (msgb and xua_msg)
The general rule for 'struct xua_msg' is now that it is free'd by the
function that also allocates it in the first place.  Any downstream
consumer of the xua_msg may interpret it, but not hold any references or
free() it.

Change-Id: I708505d129da5824c69b31a13a9c93201929bada
2017-04-10 11:48:35 +02:00
Harald Welte ee350893cf Add new SCCP implementation
This is an implementation of SCCP as specified in ITO-T Q.71x,
particularly the SCRC (routing), SCLC (Connectionless) and SCOC
(Connection Oriented) portions.  the elaborate state machines of
SCOC are implemented using osmo_fsm, with one state machine for each
connection.

Interfaces to the top (user application) are the SCCP-USER-SAP and on
the bottom (network) side the MTP-USER-SAP as provided by osmo_ss7.

Contrary to a straight-forward implementation, the code internally
always uses a SUA representation of all messages (in struct xua_msg).
This enables us to have one common implementation of all related state
machines and use them for both SUA and SCCP.  If used with real SCCP
wire format, all messages are translated from SCCP to SUA on ingress and
translated from SUA to SCCP on egress.  As SUA is a super-set of SCCP,
this can be done "lossless".

Change-Id: I916e895d9a4914b05483fe12ab5251f206d10dee
2017-04-10 11:48:34 +02:00