- gerrit-log - lists.osmocom.org

[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
by osmith 28 Nov '24

28 Nov '24

Attention is currently required from: pespin. osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email ) Change subject: abis: Fix reusing bts->*_link while it is being destroyed ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0 Gerrit-Change-Number: 38968 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 28 Nov 2024 08:34:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in libosmo-abis[master]: ipaccess: Convert BTS OML & RSL link to use stream_cli
by fixeria 27 Nov '24

27 Nov '24

Attention is currently required from: daniel, pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email ) Change subject: ipaccess: Convert BTS OML & RSL link to use stream_cli ...................................................................... Patch Set 1: Code-Review+1 (6 comments) File src/input/ipaccess.c: https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/7df669a7_82556089… : PS1, Line 494: OSMO_ASSERT(cli); Why are you so assert()ive? ;) `ipaccess_bts_e1i_ts_stream_cli()` does assert() `cli` is not `NULL`. Either this one or the one inside `ipaccess_bts_e1i_ts_stream_cli()` is redundant. https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/a3508b43_e02a7d98… : PS1, Line 545: osmo_stream_cli_send Why not returning `rc` of this function? https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/49fd0510_bb6c10d3… : PS1, Line 928: *msgb_put(nmsg2, 1) = IPAC_MSGT_ID_ACK; `msgb_v_put(nmsg2, IPAC_MSGT_ID_ACK)` https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/f8adf616_5ab18d59… : PS1, Line 944: uint8_t *data = msgb_l2(msg); `const`? https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/b6751ff9_415a8f80… : PS1, Line 992: err: : return -1; `goto`s can be replaced with `return -1`? https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/a56686ff_a3988cdd… : PS1, Line 1014: e1i_ts->line `s/e1i_ts->line/line/` -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I35c214fbe930c695a1475d8b4bc3dc44dff83eea Gerrit-Change-Number: 38927 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Wed, 27 Nov 2024 22:53:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
by pespin 27 Nov '24

27 Nov '24

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder 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: Ice71b3143f167482e4a23759012b58e9ee13bfc0 --- M src/osmo-bsc/bts_ipaccess_nanobts.c 1 file changed, 13 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/68/38968/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0 Gerrit-Change-Number: 38968 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder

1 0

[S] Change in libosmo-abis[master]: e1_input: Guard against crash by users calling reentrant e1inp_sign_l...
by pespin 27 Nov '24

27 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38969?usp=email ) Change subject: e1_input: Guard against crash by users calling reentrant e1inp_sign_link_destroy() ...................................................................... e1_input: Guard against crash by users calling reentrant e1inp_sign_link_destroy() Related: osmo-bsc.git Change-Id Ice71b3143f167482e4a23759012b58e9ee13bfc0 Related: osmo-bts.git Change-Id Ibc06cdc2d2cd2028b7676fa0c3211ae251cca587 Change-Id: Ib761cece68f1d809e27194bc5cc2eb15ad2ae4eb --- M src/e1_input.c 1 file changed, 11 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/69/38969/1 diff --git a/src/e1_input.c b/src/e1_input.c index 76adbd9..066a572 100644 --- a/src/e1_input.c +++ b/src/e1_input.c @@ -810,7 +810,17 @@ { struct msgb *msg; - llist_del(&link->list); + /* Catch upper layers behaving wrongly, calling e1inp_sign_link_destroy() + * and then during the driver->close() callback below ending up calling + * this function again deeper in the call stack in the same code path. */ + if (llist_empty(&link->list)) { + LOGP(DLINP, LOGL_ERROR, "Fix your application, " + "caught reentrant e1inp_sign_link_destroy(%p)!\n", link); + return; + } + /* Use llist_del_init() to catch double destroy above: */ + llist_del_init(&link->list); + while (!llist_empty(&link->tx_list)) { msg = msgb_dequeue(&link->tx_list); msgb_free(msg); -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38969?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Ib761cece68f1d809e27194bc5cc2eb15ad2ae4eb Gerrit-Change-Number: 38969 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
by pespin 27 Nov '24

27 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38968?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: Ice71b3143f167482e4a23759012b58e9ee13bfc0 --- M src/osmo-bsc/bts_ipaccess_nanobts.c 1 file changed, 13 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/68/38968/1 diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c index b0532e5..b823dfc 100644 --- a/src/osmo-bsc/bts_ipaccess_nanobts.c +++ b/src/osmo-bsc/bts_ipaccess_nanobts.c @@ -508,12 +508,17 @@ /* These are exported because they are used by the VTY interface. */ void ipaccess_drop_rsl(struct gsm_bts_trx *trx, const char *reason) { + struct e1inp_sign_link *link; + if (!trx->rsl_link_primary) return; LOG_TRX(trx, DLINP, LOGL_NOTICE, "Dropping RSL link: %s\n", reason); - e1inp_sign_link_destroy(trx->rsl_link_primary); + /* Mark bts->rsl_link_primar ptr null before calling sign_link_destroy, + * to avoid a callback triggering this same code path. */ + link = trx->rsl_link_primary; trx->rsl_link_primary = NULL; + e1inp_sign_link_destroy(link); osmo_stat_item_dec(osmo_stat_item_group_get_item(trx->bts->bts_statg, BTS_STAT_RSL_CONNECTED), 1); if (trx->bts->c0 == trx) @@ -529,6 +534,7 @@ uint8_t i; struct timespec tp; int rc; + struct e1inp_sign_link *link; /* First of all, remove deferred drop if enabled */ osmo_timer_del(&bts->oml_drop_link_timer); @@ -537,8 +543,11 @@ return; LOG_BTS(bts, DLINP, LOGL_NOTICE, "Dropping OML link: %s\n", reason); - 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); rc = osmo_clock_gettime(CLOCK_MONOTONIC, &tp); bts->updowntime = (rc < 0) ? 0 : tp.tv_sec; /* we don't need sub-second precision for downtime */ osmo_stat_item_dec(osmo_stat_item_group_get_item(bts->bts_statg, BTS_STAT_OML_CONNECTED), 1); @@ -546,8 +555,9 @@ /* Also drop the associated OSMO link */ OSMO_ASSERT(bts->osmo_link); - e1inp_sign_link_destroy(bts->osmo_link); + link = bts->osmo_link; bts->osmo_link = NULL; + e1inp_sign_link_destroy(link); bts_setup_ramp_remove(bts); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0 Gerrit-Change-Number: 38968 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-ttcn3-hacks[master]: bsc: osmo-bsc.cfg: Listen IPA Abis and CTRL interfaces on 127.0.0.1
by fixeria 27 Nov '24

27 Nov '24

Attention is currently required from: pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38963?usp=email ) Change subject: bsc: osmo-bsc.cfg: Listen IPA Abis and CTRL interfaces on 127.0.0.1 ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38963?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I56c8c6af16502e8f98b28902a251e190da0c0490 Gerrit-Change-Number: 38963 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 27 Nov 2024 19:09:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in osmo-bts[master]: abis: Fix reusing bts->*_link while it is being destroyed
by fixeria 27 Nov '24

27 Nov '24

Attention is currently required from: pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bts/+/38967?usp=email ) Change subject: abis: Fix reusing bts->*_link while it is being destroyed ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38967?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ibc06cdc2d2cd2028b7676fa0c3211ae251cca587 Gerrit-Change-Number: 38967 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 27 Nov 2024 19:08:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-bts[master]: abis: Log line and ts nr of signal
by fixeria 27 Nov '24

27 Nov '24

Attention is currently required from: pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bts/+/38966?usp=email ) Change subject: abis: Log line and ts nr of signal ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38966?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I322633a90566dbd4fae10ab6b1fbbedf55907e8b Gerrit-Change-Number: 38966 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 27 Nov 2024 19:00:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-bts[master]: abis: Log line and ts nr of signal
by pespin 27 Nov '24

27 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/38966?usp=email ) Change subject: abis: Log line and ts nr of signal ...................................................................... abis: Log line and ts nr of signal Change-Id: I322633a90566dbd4fae10ab6b1fbbedf55907e8b --- M src/common/abis.c 1 file changed, 5 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/66/38966/1 diff --git a/src/common/abis.c b/src/common/abis.c index 5629cf2..3e0f6dc 100644 --- a/src/common/abis.c +++ b/src/common/abis.c @@ -476,8 +476,11 @@ return 0; struct input_signal_data *isd = signal_data; - DEBUGP(DABIS, "Input Signal %s received for link_type=%s\n", - get_value_string(e1inp_signal_names, signal), e1inp_signtype_name(isd->link_type)); + DEBUGP(DABIS, "Input Signal %s received for ts-%u-%u link_type=%s\n", + get_value_string(e1inp_signal_names, signal), + isd->line ? isd->line->num : -1, + isd->ts_nr, + e1inp_signtype_name(isd->link_type)); return 0; } -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38966?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I322633a90566dbd4fae10ab6b1fbbedf55907e8b Gerrit-Change-Number: 38966 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-bts[master]: abis: Fix reusing bts->*_link while it is being destroyed
by pespin 27 Nov '24

27 Nov '24

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 { -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38967?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ibc06cdc2d2cd2028b7676fa0c3211ae251cca587 Gerrit-Change-Number: 38967 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log