Commit Graph

8 Commits

Author SHA1 Message Date
Harald Welte f71096a2b0 spelling fixes in comments
Change-Id: I4ecd9a1c5241cfd3a3e1daf05f7826876371369f
2021-11-15 18:05:04 +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
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
Harald Welte b393b3f4cc Add SPDX-License-Identifier + missing copyright statements
Change-Id: I113232bbeaa7a835871df7f9b883ba573d8a2534
2017-11-13 01:25:47 +09:00
Harald Welte 6fa1933178 sclc_rx_cldr(): Don't try to dereference user data_ie without check
While the SUA / SCCP2SUA code is ensuring that mandatory information
elements such as the user data IE in a CLD$ message, we might still have
current or future callers of sclc_rx_cldr() that don't comply with that.
So let's make sure data_ie is valid before dereferencing it.

Change-Id: I7c1010b0ac82ee0b7bd5e2c7413899695eae0070
Fixes: coverity CID#166940
2017-04-27 12:09:29 +02:00
Harald Welte 36a0ca83ab sclc_rx_cldt(): Don't try to dereference user data_ie without check
While the SUA / SCCP2SUA code is ensuring that mandatory information
elements such as the user data IE in a CLDT message, we might still have
current or future callers of sclc_rx_cldt() that don't comply with that.
So let's make sure data_ie is valid before dereferencing it.

Change-Id: Ia102f6c4cd5c6c3f823cb219635c42b9a87765f8
Fixes: coverity CID#166942
2017-04-27 12:02:47 +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