pespin has submitted this change. ( 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(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved 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: merged Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I1e6a25f6db695a16bd05ae4ec481df6e14cf65b5 Gerrit-Change-Number: 39633 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt;