pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmo-sigtran/+/39633?usp=email )
Change subject: asp: Make sure asp->fi is set to NULL when freeing the object
......................................................................
asp: Make sure asp->fi is set to NULL when freeing the object
The asp->fi is freed in lots of places through osmo_fsm_inst_term().
Make sure the pointer is actually set to NULL to catch potential
accesses to it after being freed.
Change-Id: I1e6a25f6db695a16bd05ae4ec481df6e14cf65b5
---
M src/osmo_ss7_asp.c
M src/xua_asp_fsm.c
M src/xua_asp_fsm.h
3 files changed, 41 insertions(+), 18 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran
refs/changes/33/39633/1
diff --git a/src/osmo_ss7_asp.c b/src/osmo_ss7_asp.c
index 44ffa68..ab772ff 100644
--- a/src/osmo_ss7_asp.c
+++ b/src/osmo_ss7_asp.c
@@ -716,8 +716,10 @@
/* (re)start the ASP FSM */
if (asp->fi)
osmo_fsm_inst_term(asp->fi, OSMO_FSM_TERM_REQUEST, NULL);
- asp->fi = xua_asp_fsm_start(asp, asp->cfg.role, LOGL_DEBUG);
-
+ OSMO_ASSERT(!asp->fi);
+ if ((rc = xua_asp_fsm_start(asp, asp->cfg.role, LOGL_DEBUG)) < 0)
+ return rc;
+ OSMO_ASSERT(asp->fi);
return 0;
}
diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c
index 089c31c..12640d6 100644
--- a/src/xua_asp_fsm.c
+++ b/src/xua_asp_fsm.c
@@ -714,7 +714,13 @@
{
struct xua_asp_fsm_priv *xafp = fi->priv;
+ if (!xafp)
+ return;
+
osmo_timer_del(&xafp->t_ack.timer);
+
+ if (xafp->asp)
+ xafp->asp->fi = NULL;
}
static const struct osmo_fsm_state xua_asp_states[] = {
@@ -779,16 +785,16 @@
.cleanup = xua_asp_fsm_cleanup,
};
-static struct osmo_fsm_inst *ipa_asp_fsm_start(struct osmo_ss7_asp *asp,
- enum osmo_ss7_asp_role role, int
log_level);
+static int ipa_asp_fsm_start(struct osmo_ss7_asp *asp,
+ enum osmo_ss7_asp_role role, int log_level);
-/*! \brief Start a new ASP finite stae machine for given ASP
+/*! \brief Start a new ASP finite stae machine for given ASP (stored in
asp->fi)
* \param[in] asp Application Server Process for which to start FSM
* \param[in] role Role (ASP, SG, IPSP) of this FSM
* \param[in] log_level Logging Level for ASP FSM logging
- * \returns FSM instance on success; NULL on error */
-struct osmo_fsm_inst *xua_asp_fsm_start(struct osmo_ss7_asp *asp,
- enum osmo_ss7_asp_role role, int
log_level)
+ * \returns 0 on success; negative on error */
+int xua_asp_fsm_start(struct osmo_ss7_asp *asp,
+ enum osmo_ss7_asp_role role, int log_level)
{
struct osmo_fsm_inst *fi;
struct xua_asp_fsm_priv *xafp;
@@ -802,14 +808,17 @@
xafp = talloc_zero(fi, struct xua_asp_fsm_priv);
if (!xafp) {
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
- return NULL;
+ return -ENOMEM;
}
xafp->role = role;
xafp->asp = asp;
fi->priv = xafp;
- return fi;
+ /* Attach FSM to ASP: */
+ asp->fi = fi;
+
+ return 0;
}
@@ -1124,6 +1133,14 @@
return -1;
}
+static void ipa_asp_fsm_cleanup(struct osmo_fsm_inst *fi, enum
osmo_fsm_term_cause cause)
+{
+ struct ipa_asp_fsm_priv *iafp = fi->priv;
+
+ if (iafp && iafp->asp)
+ iafp->asp->fi = NULL;
+}
+
static const struct osmo_fsm_state ipa_asp_states[] = {
[IPA_ASP_S_DOWN] = {
.in_event_mask = S(XUA_ASP_E_M_ASP_UP_REQ) |
@@ -1200,16 +1217,17 @@
S(XUA_ASP_E_ASPSM_BEAT_ACK) |
S(XUA_ASP_E_AS_ASSIGNED),
.allstate_action = ipa_asp_allstate,
+ .cleanup = ipa_asp_fsm_cleanup,
};
-/*! \brief Start a new ASP finite state machine for given ASP
+/*! \brief Start a new ASP finite state machine for given ASP (stored on
asp->fi)
* \param[in] asp Application Server Process for which to start FSM
* \param[in] role Role (ASP, SG, IPSP) of this FSM
* \param[in] log_level Logging Level for ASP FSM logging
- * \returns FSM instance on success; NULL on error */
-static struct osmo_fsm_inst *ipa_asp_fsm_start(struct osmo_ss7_asp *asp,
- enum osmo_ss7_asp_role role, int
log_level)
+ * \returns 0 on success; negative on error */
+static int ipa_asp_fsm_start(struct osmo_ss7_asp *asp,
+ enum osmo_ss7_asp_role role, int log_level)
{
struct osmo_fsm_inst *fi;
struct ipa_asp_fsm_priv *iafp;
@@ -1223,7 +1241,7 @@
iafp = talloc_zero(fi, struct ipa_asp_fsm_priv);
if (!iafp) {
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
- return NULL;
+ return -ENOMEM;
}
if (as) {
@@ -1251,8 +1269,11 @@
fi->priv = iafp;
+ /* Attach FSM to ASP: */
+ asp->fi = fi;
+
if (can_start && role == OSMO_SS7_ASP_ROLE_ASP)
osmo_fsm_inst_dispatch(fi, XUA_ASP_E_M_ASP_UP_REQ, NULL);
- return fi;
+ return 0;
}
diff --git a/src/xua_asp_fsm.h b/src/xua_asp_fsm.h
index ca44514..ab19e36 100644
--- a/src/xua_asp_fsm.h
+++ b/src/xua_asp_fsm.h
@@ -42,5 +42,5 @@
extern struct osmo_fsm xua_asp_fsm;
extern struct osmo_fsm ipa_asp_fsm;
-struct osmo_fsm_inst *xua_asp_fsm_start(struct osmo_ss7_asp *asp,
- enum osmo_ss7_asp_role role, int
log_level);
+int xua_asp_fsm_start(struct osmo_ss7_asp *asp,
+ enum osmo_ss7_asp_role role, int log_level);
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39633?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I1e6a25f6db695a16bd05ae4ec481df6e14cf65b5
Gerrit-Change-Number: 39633
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pes...@sysmocom.de>
