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);