neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37147?usp=email )
Change subject: prevent use-after-free after FSM instance termination ......................................................................
prevent use-after-free after FSM instance termination
- Set osmo_fsm_set_dealloc_ctx(OTC_SELECT) in osmo-hnbgw's main(). - Only dispatch RANAP when FSM instances aren't terminated.
This way we possibly pre-empt use-after-free crashes for deallocating FSM "nests" for obscure corner cases.
Use-after-free is a general problem for FSM design. For this, we created osmo_fsm_set_dealloc_ctx(): When an FSM is terminated, move it to a separate talloc context, instead of being deallocated.
An actual use-after-free was observed as described in OS#6484, but that needs a separate, orthogonal fix:
When the Iuh link is lost while the CN link is waiting for SCCP CC or CREF -- the better solution is described in OS#6085: don't wait for CC at all, just dispatch DISCONN to SCCP-SCOC.
So even though the code where a crash was observed will be removed, this patch is a general safeguard against corner case crashes, improving general stability.
Related: OS#6484 Change-Id: Ib41e1a996aaa03221e73643636140947ac8f99e2 --- M src/osmo-hnbgw/context_map_rua.c M src/osmo-hnbgw/context_map_sccp.c M src/osmo-hnbgw/osmo_hnbgw_main.c 3 files changed, 42 insertions(+), 0 deletions(-)
Approvals: fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve
diff --git a/src/osmo-hnbgw/context_map_rua.c b/src/osmo-hnbgw/context_map_rua.c index 3c6ea1e..7286906 100644 --- a/src/osmo-hnbgw/context_map_rua.c +++ b/src/osmo-hnbgw/context_map_rua.c @@ -158,6 +158,11 @@ static int handle_rx_rua(struct osmo_fsm_inst *fi, struct msgb *ranap_msg) { struct hnbgw_context_map *map = fi->priv; + + /* If the FSM instance has already terminated, don't dispatch anything. */ + if (fi->proc.terminating) + return 0; + if (!msg_has_l2_data(ranap_msg)) return 0;
diff --git a/src/osmo-hnbgw/context_map_sccp.c b/src/osmo-hnbgw/context_map_sccp.c index 10ddaf2..511577c 100644 --- a/src/osmo-hnbgw/context_map_sccp.c +++ b/src/osmo-hnbgw/context_map_sccp.c @@ -198,6 +198,10 @@ struct hnbgw_context_map *map = fi->priv; int rc;
+ /* If the FSM instance has already terminated, don't dispatch anything. */ + if (fi->proc.terminating) + return 0; + /* When there was no message received along with the received event, then there is nothing to forward to RUA. */ if (!msg_has_l2_data(ranap_msg)) return 0; diff --git a/src/osmo-hnbgw/osmo_hnbgw_main.c b/src/osmo-hnbgw/osmo_hnbgw_main.c index 2f71251..3e715b0 100644 --- a/src/osmo-hnbgw/osmo_hnbgw_main.c +++ b/src/osmo-hnbgw/osmo_hnbgw_main.c @@ -360,6 +360,8 @@ signal(SIGUSR2, &signal_handler); osmo_init_ignore_signals();
+ osmo_fsm_set_dealloc_ctx(OTC_SELECT); + while (1) { rc = osmo_select_main_ctx(0); if (rc < 0)