[PATCH] libosmo-sccp[master]: ensure valid primary_pc in osmo_ss7_instance
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/3355
to look at the new patch set (#4).
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
---
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
5 files changed, 19 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/55/3355/4
diff --git a/src/osmo_ss7.c b/src/osmo_ss7.c
index 6db3f14..f82f952 100644
--- a/src/osmo_ss7.c
+++ b/src/osmo_ss7.c
@@ -354,6 +354,8 @@
if (!inst)
return NULL;
+ inst->cfg.primary_pc = OSMO_SS7_PC_INVALID;
+
inst->cfg.id = id;
LOGSS7(inst, LOGL_INFO, "Creating SS7 Instance\n");
@@ -1825,7 +1827,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 (osmo_ss7_pc_is_valid(inst->cfg.primary_pc) && 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 476aedf..164b7f2 100644
--- a/src/osmo_ss7_vty.c
+++ b/src/osmo_ss7_vty.c
@@ -166,7 +166,7 @@
{
struct osmo_ss7_instance *inst = vty->index;
int pc = osmo_ss7_pointcode_parse(inst, argv[0]);
- if (pc < 0) {
+ if (pc < 0 || !osmo_ss7_pc_is_valid((uint32_t)pc)) {
vty_out(vty, "Invalid point code (%s)%s", argv[0], VTY_NEWLINE);
return CMD_WARNING;
}
@@ -1536,7 +1536,7 @@
if (inst->cfg.pc_fmt.delimiter != '.')
vty_out(vty, " point-code delimiter dash%s", VTY_NEWLINE);
- if (inst->cfg.primary_pc)
+ if (osmo_ss7_pc_is_valid(inst->cfg.primary_pc))
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..9820c40 100644
--- a/src/sccp_scoc.c
+++ b/src/sccp_scoc.c
@@ -1670,11 +1670,11 @@
{
struct osmo_ss7_instance *s7i = conn->inst->ss7;
struct osmo_sccp_addr *remote_addr;
- uint32_t local_pc;
+ uint32_t local_pc = OSMO_SS7_PC_INVALID;
if (conn->user->pc_valid)
local_pc = conn->user->pc;
- else
+ else if (osmo_ss7_pc_is_valid(s7i->cfg.primary_pc))
local_pc = s7i->cfg.primary_pc;
if (conn->incoming)
diff --git a/src/sccp_scrc.c b/src/sccp_scrc.c
index e44201a..2afd696 100644
--- a/src/sccp_scrc.c
+++ b/src/sccp_scrc.c
@@ -91,8 +91,14 @@
param = >u.transfer;
if (sua->mtp.opc)
param->opc = sua->mtp.opc;
- else
+ else {
+ if (!osmo_ss7_pc_is_valid(s7i->cfg.primary_pc)) {
+ 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 71b3262..d49da29 100644
--- a/src/sccp_user.c
+++ b/src/sccp_user.c
@@ -366,6 +366,11 @@
goto out_ss7;
as_created = true;
+ if (!osmo_ss7_pc_is_valid(ss7->cfg.primary_pc)) {
+ 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 */
--
To view, visit https://gerrit.osmocom.org/3355
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f0f0c89b7335d9da24161bfac8234be214ca00c
Gerrit-PatchSet: 4
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
[PATCH] libosmo-sccp[master]: ensure valid primary_pc in osmo_ss7_instance
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 = >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 = >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;
