Attention is currently required from: fixeria, fixeria, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email )
Change subject: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2)
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
@axilirator@gmail.com do you mind giving a try to this new patch I submitted?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df
Gerrit-Change-Number: 32552
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator(a)gmail.com>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <axilirator(a)gmail.com>
Gerrit-Comment-Date: Wed, 25 Oct 2023 13:36:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
pespin has uploaded a new patch set (#3) to the change originally created by fixeria. ( https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email )
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Code-Review-1 by pespin, Verified+1 by Jenkins Builder
Change subject: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2)
......................................................................
trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2)
- If the llist is flushed during rx rsp callback, when the flow is
returned to trx_ctrl_read_cb() it would access tcm which was in the
llist and end up in use-after-free.
- We need to store state on whether code path is inside the read_cb in
order to:
-- Delay transmission of new message if callback calls trx_if_flush()
followed by trx_ctrl_send(), since the trx_ctrl_send() at the end of
trx_ctrl_read_cb would retransmit it again immediatelly.
-- Avoid accessing tcm pointer if the callback called trx_if_flush(),
since it has been freed.
Related: OS#6020
Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df
---
M src/osmo-bts-trx/l1_if.h
M src/osmo-bts-trx/trx_if.c
2 files changed, 49 insertions(+), 7 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/52/32552/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df
Gerrit-Change-Number: 32552
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/34894?usp=email )
Change subject: Revert "trx_if: Allow calling trx_if_flush/close from within TRXC callback"
......................................................................
Revert "trx_if: Allow calling trx_if_flush/close from within TRXC callback"
This reverts commit 4444262a6ab1e1e231ea81c4ec990f1a1f571a1f.
This commit introduced several side effects:
- tcm is left out of the l1h->trx_ctrl_list and hence won't be ever
retransmitted.
- Since tcm is removed before rsp callback, the llist may become empty
and if somehwere in the rsp callback a new message is enqueud it will
be sent immediatelly, and will be retransmitted again when
trx_ctrl_read_cb() calls trx_ctrl_send() at the end.
Change-Id: Ideb2d08ac8a2902bceeabfb055c59c9a13dbe3c0
Related: OS#6020
---
M src/osmo-bts-trx/trx_if.c
1 file changed, 25 insertions(+), 9 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/94/34894/1
diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c
index fef8c22..3f9fc04 100644
--- a/src/osmo-bts-trx/trx_if.c
+++ b/src/osmo-bts-trx/trx_if.c
@@ -721,13 +721,6 @@
rsp.cb = tcm->cb;
- /* Remove command from list, save it to last_acked and remove previous
- * last_acked. Do it before calling callback to avoid user freeing tcm
- * pointer if flushing/closing the iface. */
- llist_del(&tcm->list);
- talloc_free(l1h->last_acked);
- l1h->last_acked = tcm;
-
/* check for response code */
rc = trx_ctrl_rx_rsp(l1h, &rsp, tcm);
if (rc == -EINVAL)
@@ -735,11 +728,15 @@
/* re-schedule last cmd in rc seconds time */
if (rc > 0) {
- if (!llist_empty(&l1h->trx_ctrl_list))
- osmo_timer_schedule(&l1h->trx_ctrl_timer, rc, 0);
+ osmo_timer_schedule(&l1h->trx_ctrl_timer, rc, 0);
return 0;
}
+ /* remove command from list, save it to last_acked and removed previous last_acked */
+ llist_del(&tcm->list);
+ talloc_free(l1h->last_acked);
+ l1h->last_acked = tcm;
+
trx_ctrl_send(l1h);
return 0;
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34894?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ideb2d08ac8a2902bceeabfb055c59c9a13dbe3c0
Gerrit-Change-Number: 34894
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: jolly.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34889?usp=email )
Change subject: ASCI: Control uplink access bursts detection of physical interface
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/34889/comment/49c79392_286d801c
PS1, Line 2448: struct gsm_bts_trx *trx
can we accept `struct gsm_lchan *lchan` directly here? ... and this avoid having to do `gsm_lchan2chan_nr(lchan)` and then `get_lchan_by_chan_nr()` again. Yes, we would still need to do `gsm_lchan2chan_nr(lchan)` here, but this way we avoid unneeded lchan lookups.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34889?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I92d6773a3a463eb747143c85aa149e54c1fda122
Gerrit-Change-Number: 34889
Gerrit-PatchSet: 1
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Wed, 25 Oct 2023 13:31:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/gapk/+/34893?usp=email )
Change subject: dist: ensure the license text is included
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/gapk/+/34893?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: gapk
Gerrit-Branch: master
Gerrit-Change-Id: Ib5c7b479fa66291be987230f102ff391f4902988
Gerrit-Change-Number: 34893
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Oct 2023 13:26:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/gapk/+/34892?usp=email )
Change subject: dist: exclude libgsmhr files downloaded by fetch_sources.py
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
File libgsmhr/Makefile.am:
https://gerrit.osmocom.org/c/gapk/+/34892/comment/4e2cfbd5_e6d4c1ae
PS1, Line 39: -rm -rf $(distdir)/$(REFSRC_PATH)
> > However downloading the sources first even if we don't intend to package them, and then remove the […]
Okay, doesn't seem worth spending more time on. The only user of this is probably the automated release script that runs it once a new release is out.
--
To view, visit https://gerrit.osmocom.org/c/gapk/+/34892?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: gapk
Gerrit-Branch: master
Gerrit-Change-Id: I66e31dec37e53bf1a8c7df948fd9316e1467752c
Gerrit-Change-Number: 34892
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Oct 2023 13:26:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment