Attention is currently required from: daniel, fixeria, laforge, osmith.
Hello Jenkins Builder, daniel, fixeria, laforge, osmith,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+1 by osmith
Change subject: asp: Avoid double-free of received msg if conn is teared down
......................................................................
asp: Avoid double-free of received msg if conn is teared down
"""
20250516192255921 DLSS7 DEBUG IPA_ASP(ipa-asp-loadshare-sender0){WAIT_ID_RESP}: Received Event IPA_CCM_ID_RESP (ipa.c:120)
20250516192255921 DLMI DEBUG Rx IPA CCM ID_RESP: Unit_ID='0/1/2' MAC_Address='' Location_1='' Location_2='' Equipment_Version='' Software_Version='' Unit_Name='mahlzeit' Serial_Number='' (ipa.c:233)
20250516192255921 DLSS7 NOTICE IPA_ASP(ipa-asp-loadshare-sender0){WAIT_ID_RESP}: Cannot find any definition for IPA Unit Name 'mahlzeit' (xua_asp_fsm.c:968)
20250516192255921 DLSS7 INFO ipa-asp-loadshare-sender0: connection closed (ss7_asp.c:1159)
20250516192255921 DLSS7 DEBUG IPA_ASP(ipa-asp-loadshare-sender0){WAIT_ID_RESP}: Received Event SCTP-COMM_DOWN.ind (ss7_asp.c:1165)
20250516192255922 DLSS7 DEBUG IPA_ASP(ipa-asp-loadshare-sender0){WAIT_ID_RESP}: state_chg to ASP_DOWN (xua_asp_fsm.c:1154)
20250516192255922 DLSS7 DEBUG XUA_AS(ipa-as-loadshare-sender){AS_DOWN}: Received Event ASPAS-ASP_DOWN.ind (xua_asp_fsm.c:370)
20250516192255922 DLSS7 DEBUG IPA_ASP(ipa-asp-loadshare-sender0){ASP_DOWN}: No Layer Manager, dropping M-ASP_DOWN.indication (xua_asp_fsm.c:119)
20250516192255922 DLSS7 DEBUG IPA_ASP(ipa-asp-loadshare-sender0){ASP_DOWN}: No Layer Manager, dropping M-SCTP_RELEASE.indication (xua_asp_fsm.c:119)
Program terminated with signal SIGABRT, Aborted.
#0 0x000076bb9898ceec in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#0 0x000076bb9898ceec in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#1 0x000076bb9893dfb2 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#2 0x000076bb98928472 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#3 0x000076bb98ae6496 in ?? () from /lib/x86_64-linux-gnu/libtalloc.so.2
#4 0x000076bb98b1b869 in msgb_free (m=0x5f957de3e750) at ../../../src_copy/libosmocore/src/core/msgb.c:119
#5 0x000076bb98bab8c8 in ipa_rx_msg_ccm (asp=0x5f957de3da50, msg=0x5f957de3e750) at ../../src_copy/libosmo-sigtran/src/ipa.c:137
#6 0x000076bb98bac135 in ipa_rx_msg (asp=0x5f957de3da50, msg=0x5f957de3e750, sls=0 '\000') at ../../src_copy/libosmo-sigtran/src/ipa.c:321
#7 0x000076bb98bca44f in ss7_asp_ipa_srv_conn_rx_cb (conn=0x5f957ddba4a0, res=49, msg=0x5f957de3e750) at ../../src_copy/libosmo-sigtran/src/ss7_asp.c:895
#8 0x000076bb988efcb1 in stream_srv_iofd_read_cb (iofd=0x5f957ddd8e40, res=49, msg=0x5f957de3e750) at ../../src_copy/libosmo-netif/src/stream_srv.c:732
#9 0x000076bb98b23c3c in iofd_handle_segmented_read (iofd=0x5f957ddd8e40, msg=0x5f957de3e750, rc=49) at ../../../src_copy/libosmocore/src/core/osmo_io.c:357
#10 0x000076bb98b23d2b in iofd_handle_recv (iofd=0x5f957ddd8e40, msg=0x5f957de3e750, rc=49, hdr=0x0) at ../../../src_copy/libosmocore/src/core/osmo_io.c:384
#11 0x000076bb98b257b7 in iofd_poll_ofd_cb_recvmsg_sendmsg (ofd=0x5f957ddd8ef0, what=1) at ../../../src_copy/libosmocore/src/core/osmo_io_poll.c:64
#12 0x000076bb98b25b32 in iofd_poll_ofd_cb_dispatch (ofd=0x5f957ddd8ef0, what=1) at ../../../src_copy/libosmocore/src/core/osmo_io_poll.c:136
#13 0x000076bb98b2907b in poll_disp_fds (n_fd=6) at ../../../src_copy/libosmocore/src/core/select.c:419
#14 0x000076bb98b29191 in _osmo_select_main (polling=0) at ../../../src_copy/libosmocore/src/core/select.c:457
#15 0x000076bb98b291ac in osmo_select_main (polling=0) at ../../../src_copy/libosmocore/src/core/select.c:496
#16 0x00005f9553dd9a21 in main (argc=3, argv=0x7ffe754fac38) at ../../src_copy/libosmo-sigtran/stp/stp_main.c:270
"""
Related: OS#6728
Change-Id: I69f80f611c14db2b328dafd4a90247c6f2dac6fd
---
M src/ss7_asp.c
1 file changed, 24 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/27/40327/4
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I69f80f611c14db2b328dafd4a90247c6f2dac6fd
Gerrit-Change-Number: 40327
Gerrit-PatchSet: 4
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-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Attention is currently required from: daniel, fixeria, laforge.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email )
Change subject: asp: Avoid double-free of received msg if conn is teared down
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> API users that may not expect/want this
Then it's a bug in the users of the API.
I don't think it's bad that by default the ctx of the msg is the one configured in osmo_iofd. We already have multiple releases of osmo_iofd with the previous behavior, so yet another reason not to change that.
In any of the cases, this patch is about fixing a msgb coming from *stream* object, not iofd object. Again, it always worked like that since new APIs related to osmo_iofd implementation were introduced, so changing that now is also problematic for something I think is fine as is now.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I69f80f611c14db2b328dafd4a90247c6f2dac6fd
Gerrit-Change-Number: 40327
Gerrit-PatchSet: 3
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-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 27 May 2025 12:28:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40379?usp=email )
Change subject: testenv: fix TESTENV_INSTALL_DIR
......................................................................
testenv: fix TESTENV_INSTALL_DIR
When osmo-dev is used (no --binary-repo arg is set), then set
TESTENV_INSTALL_DIR and --install-prefix for osmo-dev to:
<cache dir>/osmo-ttcn3-testenv/{podman,host}/install
Old --install-prefix:
<cache dir>/osmo-ttcn3-testenv/{podman,host}/usr
Old TESTENV_INSTALL_DIR:
<cache dir>/osmo-ttcn3-testenv/{podman,host}
The old behavior was misleading, because:
* It resulted in configs getting installed into:
<cache dir>/osmo-ttcn3-testenv/{podman,host}/usr/etc/...
* TESTENV_INSTALL_DIR looked like it would point at the install dir
(because there is usr inside that directory), but it was actually
pointing at the top dir of the install dir.
Fixes: 143b1000 ("testenv: add TESTENV_INSTALL_DIR")
Change-Id: Id94936338a6eb74dee0b3f4668cbaca309b269e4
---
M _testenv/README.md
M _testenv/testenv/cmd.py
M _testenv/testenv/osmo_dev.py
3 files changed, 14 insertions(+), 23 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/79/40379/1
diff --git a/_testenv/README.md b/_testenv/README.md
index 53d3440..a1ba73d 100644
--- a/_testenv/README.md
+++ b/_testenv/README.md
@@ -232,8 +232,8 @@
* `TESTENV_INSTALL_DIR`:
The directory into which the SUT binaries and other files are installed. It
is set to `/` for `--podman --binary-repo`,
- `~/.cache/osmo-ttcn3-testenv/podman` for `--podman` and
- `~/.cache/osmo-ttcn3-testenv/host` otherwise. The
+ `~/.cache/osmo-ttcn3-testenv/podman/install` for `--podman` and
+ `~/.cache/osmo-ttcn3-testenv/host/install` otherwise. The
`~/.cache/osmo-ttcn3-testenv` part can be changed with the `--cache`
argument. Different cache directories for podman and for the host are used
as it is very likely that the binary objects from both are incompatible.
@@ -245,10 +245,10 @@
* `OSMO_DEV_MAKE_DIR`:
This variable is unset if `--binary-repo` is used as argument. Otherwise it
- is set to `~/.cache/osmo-ttcn3-testenv/host/make2` or
- `~/.cache/osmo-ttcn3-testenv/podman/make2` (with `--podman`) by default. The
- `~/.cache/osmo-ttcn3-testenv` part can be changed with the `--cache`
- argument.
+ is set to `~/.cache/osmo-ttcn3-testenv/host/make<version>` or
+ `~/.cache/osmo-ttcn3-testenv/podman/make<version>` (with `--podman`) by
+ default. The `~/.cache/osmo-ttcn3-testenv` part can be changed with the
+ `--cache` argument.
* `TESTENV_QEMU_KERNEL`:
Is only set if `-C`/`--custom-kernel` or `-D`/`--debian-kernel` parameters
diff --git a/_testenv/testenv/cmd.py b/_testenv/testenv/cmd.py
index 5da27fe..e65f777 100644
--- a/_testenv/testenv/cmd.py
+++ b/_testenv/testenv/cmd.py
@@ -11,7 +11,7 @@
install_dir = None
make_dir = None
# osmo-dev make dir version, bump when making incompatible changes
-make_dir_version = 2
+make_dir_version = 3
def init_env():
@@ -25,17 +25,17 @@
if testenv.args.binary_repo:
install_dir = "/"
else:
- install_dir = os.path.join(testenv.args.cache, "podman")
+ install_dir = os.path.join(testenv.args.cache, "podman/install")
else:
- install_dir = os.path.join(testenv.args.cache, "host")
+ install_dir = os.path.join(testenv.args.cache, "host/install")
if not testenv.args.binary_repo:
- pkg_config_path = os.path.join(install_dir, "usr/lib/pkgconfig")
+ pkg_config_path = os.path.join(install_dir, "lib/pkgconfig")
if "PKG_CONFIG_PATH" in os.environ:
pkg_config_path += f":{os.environ.get('PKG_CONFIG_PATH')}"
pkg_config_path += ":/usr/lib/pkgconfig"
- ld_library_path = os.path.join(install_dir, "usr/lib")
+ ld_library_path = os.path.join(install_dir, "lib")
if "LD_LIBRARY_PATH" in os.environ:
ld_library_path += f":{os.environ.get('LD_LIBRARY_PATH')}"
ld_library_path += ":/usr/lib"
@@ -83,7 +83,7 @@
path += f":{os.path.join(testenv.testsuite.ttcn3_hacks_dir, testenv.args.testsuite)}"
if install_dir and install_dir != "/":
- path += f":{os.path.join(install_dir, 'usr/bin')}"
+ path += f":{os.path.join(install_dir, 'bin')}"
if podman:
path += ":/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
diff --git a/_testenv/testenv/osmo_dev.py b/_testenv/testenv/osmo_dev.py
index 8b83367..45912ae 100644
--- a/_testenv/testenv/osmo_dev.py
+++ b/_testenv/testenv/osmo_dev.py
@@ -58,17 +58,6 @@
return
extra_opts = []
- if testenv.args.podman:
- extra_opts = [
- "--install-prefix",
- os.path.join(testenv.args.cache, "podman/usr"),
- ]
- else:
- extra_opts = [
- "--install-prefix",
- os.path.join(testenv.args.cache, "host/usr"),
- ]
-
if testenv.args.jobs:
extra_opts += [f"-j{testenv.args.jobs}"]
@@ -77,6 +66,8 @@
"./gen_makefile.py",
"--build-debug",
"--no-make-check",
+ "--install-prefix",
+ testenv.cmd.install_dir,
"--make-dir",
testenv.cmd.make_dir,
"--no-ldconfig",
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40379?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Id94936338a6eb74dee0b3f4668cbaca309b269e4
Gerrit-Change-Number: 40379
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Attention is currently required from: daniel, laforge, pespin.
fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email )
Change subject: asp: Avoid double-free of received msg if conn is teared down
......................................................................
Patch Set 3:
(2 comments)
Patchset:
PS2:
> So what's the root problem according to you? […]
The root problem is that `iofd` code is transferring the talloc ownership to API users that may not expect/want this. As I suggested in the ticket, in my view talloc ownership should be explicitly **taken** by the user of `iofd` (be it netif stuff or whatever else), and not given away by the `iofd` itself.
I am not against fine-grained talloc ownership, and you can still achieve that by doing `talloc_steal()` like you're doing in this patch and did in `lapdm` code in libosmocore. Let's just allocate the `msgb` as a child of the `tall_msgb_ctx` by default and leave it up to the user of `iofd`: whether `talloc_steal()` explicitly (if needed) or keep it there for performance reasons.
Patchset:
PS3:
> ping. This is a fix for a severe bug and has been here for 1 week and a half and almost no feedback.
Please add `Related: OS#6728` and feel free to merge: I am not blocking. But I still see this as a band-aid solution treating the symptoms, but not the "disease". There definitely are other places where we (and even worse, our customers) will be bitten by this again and again. Feels like we're fighting the hydra.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I69f80f611c14db2b328dafd4a90247c6f2dac6fd
Gerrit-Change-Number: 40327
Gerrit-PatchSet: 3
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-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 27 May 2025 10:45:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: daniel, fixeria, laforge, pespin.
osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email )
Change subject: asp: Avoid double-free of received msg if conn is teared down
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I69f80f611c14db2b328dafd4a90247c6f2dac6fd
Gerrit-Change-Number: 40327
Gerrit-PatchSet: 3
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-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 27 May 2025 10:05:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: daniel, fixeria, laforge, osmith.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email )
Change subject: asp: Avoid double-free of received msg if conn is teared down
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
ping. This is a fix for a severe bug and has been here for 1 week and a half and almost no feedback.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/40327?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I69f80f611c14db2b328dafd4a90247c6f2dac6fd
Gerrit-Change-Number: 40327
Gerrit-PatchSet: 3
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-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 27 May 2025 10:02:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
