pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/38967?usp=email )
Change subject: abis: Fix reusing bts->*_link while it is being destroyed ......................................................................
abis: Fix reusing bts->*_link while it is being destroyed
Call to e1inp_sign_link_destroy() may trigger a sign_down() callback, which if happening synchronously, could end up reentring the same code path we are in before bts->*_link was set to NULL. Avoid it by marking the pointer as NULL immediatelly before calling e1inp_sign_link_destroy().
Change-Id: Ibc06cdc2d2cd2028b7676fa0c3211ae251cca587 --- M src/common/abis.c 1 file changed, 16 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/67/38967/1
diff --git a/src/common/abis.c b/src/common/abis.c index 3e0f6dc..e619ec8 100644 --- a/src/common/abis.c +++ b/src/common/abis.c @@ -87,10 +87,17 @@
static void reset_oml_link(struct gsm_bts *bts) { + struct e1inp_sign_link *link; + if (bts->oml_link) { struct timespec now;
- e1inp_sign_link_destroy(bts->oml_link); + /* Mark bts->oml_link ptr null before calling sign_link_destroy, + * to avoid a callback triggering this same code path. */ + link = bts->oml_link; + bts->oml_link = NULL; + + e1inp_sign_link_destroy(link);
/* Log a special notice if the OML connection was dropped relatively quickly. */ if (bts->oml_conn_established_timestamp.tv_sec != 0 && clock_gettime(CLOCK_MONOTONIC, &now) == 0 && @@ -100,14 +107,16 @@ "A common error is a mismatch between unit_id configuration parameters of BTS and BSC.\n", (uint64_t) (now.tv_sec - bts->oml_conn_established_timestamp.tv_sec)); } - bts->oml_link = NULL; } memset(&bts->oml_conn_established_timestamp, 0, sizeof(bts->oml_conn_established_timestamp));
/* Same for IPAC_PROTO_OSMO on the same ipa connection: */ if (bts->osmo_link) { - e1inp_sign_link_destroy(bts->osmo_link); + /* Mark bts->osmo_link ptr null before calling sign_link_destroy, + * to avoid a callback triggering this same code path. */ + link = bts->osmo_link; bts->osmo_link = NULL; + e1inp_sign_link_destroy(link); }
} @@ -226,8 +235,11 @@ /* Then iterate over the RSL signalling links */ llist_for_each_entry(trx, &bts->trx_list, list) { if (trx->bb_transc.rsl.link) { - e1inp_sign_link_destroy(trx->bb_transc.rsl.link); + /* Mark link ptr null before calling sign_link_destroy, + * to avoid a callback triggering this same code path. */ + struct e1inp_sign_link *link = trx->bb_transc.rsl.link; trx->bb_transc.rsl.link = NULL; + e1inp_sign_link_destroy(link); if (trx == trx->bts->c0) load_timer_stop(trx->bts); } else {