[PATCH] libosmo-sccp[master]: ensure valid primary_pc in osmo_ss7_instance

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Wed Jul 26 15:44:11 UTC 2017


Review at  https://gerrit.osmocom.org/3355

ensure valid primary_pc in osmo_ss7_instance

Add osmo_ss7_instance.cfg.primary_pc_valid flag.
Adjust all code paths setting primary_pc to also set primary_pc_valid.
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, but since 0 is apparently a valid point-code and
osmo_ss7_instance.primary_pc is a uint32_t, we have no way to tell whether the
user supplied a point-code or not.

Currently, in osmo_ss7_vty.c we had "if (inst->cfg.primary_pc)" suggesting 0
is invalid, but in struct osmo_sccp_user we have flag pc_valid suggesting 0 is
indeed valid. I chose to adopt a primary_pc_valid flag like osmo_sccp_user.

Change-Id: I7f0f0c89b7335d9da24161bfac8234be214ca00c
---
M include/osmocom/sigtran/osmo_ss7.h
M src/osmo_ss7.c
M src/osmo_ss7_vty.c
M src/sccp_scoc.c
M src/sccp_scrc.c
M src/sccp_user.c
M tests/ss7/ss7_test.c
7 files changed, 28 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/55/3355/1

diff --git a/include/osmocom/sigtran/osmo_ss7.h b/include/osmocom/sigtran/osmo_ss7.h
index 87ace4a..06fb29c 100644
--- a/include/osmocom/sigtran/osmo_ss7.h
+++ b/include/osmocom/sigtran/osmo_ss7.h
@@ -85,6 +85,7 @@
 		char *name;
 		char *description;
 		uint32_t primary_pc;
+		bool primary_pc_valid;
 		/* secondary PCs */
 		/* capability PCs */
 		uint8_t network_indicator;
diff --git a/src/osmo_ss7.c b/src/osmo_ss7.c
index eb5a4ef..ee2b212 100644
--- a/src/osmo_ss7.c
+++ b/src/osmo_ss7.c
@@ -1819,7 +1819,7 @@
 bool osmo_ss7_pc_is_local(struct osmo_ss7_instance *inst, uint32_t pc)
 {
 	OSMO_ASSERT(ss7_initialized);
-	if (pc == inst->cfg.primary_pc)
+	if (inst->cfg.primary_pc_valid && pc == inst->cfg.primary_pc)
 		return true;
 	/* FIXME: Secondary and Capability Point Codes */
 	return false;
diff --git a/src/osmo_ss7_vty.c b/src/osmo_ss7_vty.c
index c859eb9..775baf2 100644
--- a/src/osmo_ss7_vty.c
+++ b/src/osmo_ss7_vty.c
@@ -172,6 +172,7 @@
 	}
 
 	inst->cfg.primary_pc = pc;
+	inst->cfg.primary_pc_valid = true;
 	return CMD_SUCCESS;
 }
 
@@ -1536,7 +1537,7 @@
 	if (inst->cfg.pc_fmt.delimiter != '.')
 		vty_out(vty, " point-code delimiter dash%s", VTY_NEWLINE);
 
-	if (inst->cfg.primary_pc)
+	if (inst->cfg.primary_pc_valid)
 		vty_out(vty, " point-code %s%s",
 			osmo_ss7_pointcode_print(inst, inst->cfg.primary_pc),
 			VTY_NEWLINE);
diff --git a/src/sccp_scoc.c b/src/sccp_scoc.c
index 3d43448..729bc4d 100644
--- a/src/sccp_scoc.c
+++ b/src/sccp_scoc.c
@@ -1671,11 +1671,15 @@
 	struct osmo_ss7_instance *s7i = conn->inst->ss7;
 	struct osmo_sccp_addr *remote_addr;
 	uint32_t local_pc;
+	bool local_pc_valid = false;
 
-	if (conn->user->pc_valid)
+	if (conn->user->pc_valid) {
 		local_pc = conn->user->pc;
-	else
+		local_pc_valid = true;
+	} else if (s7i->cfg.primary_pc_valid) {
 		local_pc = s7i->cfg.primary_pc;
+		local_pc_valid = true;
+	}
 
 	if (conn->incoming)
 		remote_addr = &conn->calling_addr;
@@ -1684,7 +1688,7 @@
 
 	vty_out(vty, "%c %06x %3u %7s ", conn->incoming ? 'I' : 'O',
 		conn->conn_id, conn->user->ssn,
-		osmo_ss7_pointcode_print(s7i, local_pc));
+		local_pc_valid ? osmo_ss7_pointcode_print(s7i, local_pc) : "(no PC)");
 	vty_out(vty, "%16s %06x %3u %7s%s",
 		osmo_fsm_inst_state_name(conn->fi), conn->remote_ref, remote_addr->ssn,
 		osmo_ss7_pointcode_print(s7i, conn->remote_pc),
diff --git a/src/sccp_scrc.c b/src/sccp_scrc.c
index e44201a..a9c1c73 100644
--- a/src/sccp_scrc.c
+++ b/src/sccp_scrc.c
@@ -91,8 +91,14 @@
 	param = &omp->u.transfer;
 	if (sua->mtp.opc)
 		param->opc = sua->mtp.opc;
-	else
+	else {
+		if (!s7i->cfg.primary_pc_valid) {
+			LOGP(DLSCCP, LOGL_ERROR, "SS7 instance %u: no primary point-code set\n",
+			     s7i->cfg.id);
+			return -1;
+		}
 		param->opc = s7i->cfg.primary_pc;
+	}
 	param->dpc = remote_pc;
 	param->sls = sua->mtp.sls;
 	param->sio = MTP_SIO(MTP_SI_SCCP, s7i->cfg.network_indicator);
diff --git a/src/sccp_user.c b/src/sccp_user.c
index c9443a2..ddb777d 100644
--- a/src/sccp_user.c
+++ b/src/sccp_user.c
@@ -277,10 +277,12 @@
 		 * NOTE: This means that the user must set the pointcode to a
 		 * proper value when a cs7 instance is defined via the VTY. */
 		ss7->cfg.primary_pc = pc;
+		ss7->cfg.primary_pc_valid = true;
 		ss7_created = true;
 	}
 	LOGP(DLSCCP, LOGL_NOTICE, "%s: Using SS7 instance %u, pc:%s\n", name,
-	     ss7->cfg.id, osmo_ss7_pointcode_print(ss7, ss7->cfg.primary_pc));
+	     ss7->cfg.id,
+	     ss7->cfg.primary_pc_valid ? osmo_ss7_pointcode_print(ss7, ss7->cfg.primary_pc) : "(no PC)");
 
 	/* There must not be an existing SCCP istance, regarless if the simple
 	 * client has created the SS7 instance or if it was already present.
@@ -302,6 +304,11 @@
 			goto out_ss7;
 		as_created = true;
 
+		if (!ss7->cfg.primary_pc_valid) {
+			LOGP(DLSCCP, LOGL_ERROR, "SS7 instance %u: no primary point-code set\n",
+			     ss7->cfg.id);
+			goto out_ss7;
+		}
 		as->cfg.routing_key.pc = ss7->cfg.primary_pc;
 
 		/* install default route */
@@ -401,6 +408,7 @@
 	if (!ss7)
 		return NULL;
 	ss7->cfg.primary_pc = pc;
+	ss7->cfg.primary_pc_valid = true;
 
 	xs = osmo_ss7_xua_server_create(ss7, prot, local_port, local_ip);
 	if (!xs)
diff --git a/tests/ss7/ss7_test.c b/tests/ss7/ss7_test.c
index 7c51767..6dc2caa 100644
--- a/tests/ss7/ss7_test.c
+++ b/tests/ss7/ss7_test.c
@@ -302,6 +302,7 @@
 
 	/* test osmo_ss7_pc_is_local() */
 	s7i->cfg.primary_pc = 55;
+	s7i->cfg.primary_pc_valid = true;
 	OSMO_ASSERT(osmo_ss7_pc_is_local(s7i, 55) == true);
 	OSMO_ASSERT(osmo_ss7_pc_is_local(s7i, 23) == false);
 

-- 
To view, visit https://gerrit.osmocom.org/3355
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f0f0c89b7335d9da24161bfac8234be214ca00c
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list