- gerrit-log - lists.osmocom.org

Change in osmo-uecups[master]: Fix use-after-free by tun thread after tun obj destroyed
by osmith 12 Apr '22

12 Apr '22

Attention is currently required from: pespin, lynxis lazus. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27739 ) Change subject: Fix use-after-free by tun thread after tun obj destroyed ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File daemon/tun_device.c: https://gerrit.osmocom.org/c/osmo-uecups/+/27739/comment/77838668_648a07a4 PS3, Line 200: pthread_cleanup_pop(1); > Sorry we discussed this through jabber since it's a bit of a complex topic. […] Ack, makes sense now. -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27739 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: Idf005359afb41d3413b09281a9ff937d5eafcc7c Gerrit-Change-Number: 27739 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Tue, 12 Apr 2022 09:30:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

Change in osmo-uecups[master]: main.c: Improve logging
by pespin 12 Apr '22

12 Apr '22

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27738 ) Change subject: main.c: Improve logging ...................................................................... main.c: Improve logging Change-Id: I6d9a355fef78906cee1ddf65ebb00a738b43c596 --- M daemon/main.c 1 file changed, 4 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/daemon/main.c b/daemon/main.c index 328d074..76aeab5 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -72,6 +72,8 @@ /* kill the specified subprocess and forget about it */ static void subprocess_destroy(struct subprocess *p, int signal) { + LOGCC(p->cups_client, LOGL_DEBUG, "Kill subprocess pid %llu with signal %u\n", + (unsigned long long)p->pid, signal); kill(p->pid, signal); llist_del(&p->list); talloc_free(p); @@ -478,6 +480,7 @@ struct subprocess *p, *p2; json_t *jres; + LOGCC(cc, LOGL_DEBUG, "Destroying all tunnels\n"); pthread_rwlock_wrlock(&d->rwlock); llist_for_each_entry_safe(t, t2, &d->gtp_tunnels, list) { _gtp_tunnel_destroy(t); @@ -485,6 +488,7 @@ pthread_rwlock_unlock(&d->rwlock); /* no locking needed as this list is only used by main thread */ + LOGCC(cc, LOGL_DEBUG, "Destroying all subprocesses\n"); llist_for_each_entry_safe(p, p2, &d->subprocesses, list) { subprocess_destroy(p, SIGKILL); } -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27738 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: I6d9a355fef78906cee1ddf65ebb00a738b43c596 Gerrit-Change-Number: 27738 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

Change in osmo-uecups[master]: main: Remove duplicate call to child_terminated()
by pespin 12 Apr '22

12 Apr '22

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27737 ) Change subject: main: Remove duplicate call to child_terminated() ...................................................................... main: Remove duplicate call to child_terminated() The same pid will be handled below tyhrough waitpid. This can be seen when running PGW_Tests: """ 20220411134309656 DUECUPS main.c:343 SIGCHLD receive from pid 24; status=0 20220411134309656 DUECUPS main.c:95 r=172.18.18.202:9999<->l=172.18.18.20:4268: JSON Tx '{"program_term_ind": {"exit_code": 0, "pid": 24}}' 20220411134309656 DUECUPS main.c:343 SIGCHLD receive from pid 24; status=0 """ Change-Id: I348b91097fe9bf78b2c7e33d4f3eaf316618068e --- M daemon/main.c 1 file changed, 0 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve daniel: Looks good to me, approved diff --git a/daemon/main.c b/daemon/main.c index 39d60e6..328d074 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -367,8 +367,6 @@ OSMO_ASSERT(fdsi->ssi_signo == SIGCHLD); - child_terminated(d, fdsi->ssi_pid, fdsi->ssi_status); - /* it is known that classic signals coalesce: If you get multiple signals of the * same type before a process is scheduled, the subsequent signals are dropped. This * makes sense for SIGINT or something like this, but for SIGCHLD carrying the PID of -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27737 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: I348b91097fe9bf78b2c7e33d4f3eaf316618068e Gerrit-Change-Number: 27737 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-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

Change in docker-playground[master]: ttcn3-pgw: Pass config file to osmo-uecups
by pespin 12 Apr '22

12 Apr '22

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/27734 ) Change subject: ttcn3-pgw: Pass config file to osmo-uecups ...................................................................... Patch Set 1: (1 comment) File ttcn3-pgw-test/jenkins.sh: https://gerrit.osmocom.org/c/docker-playground/+/27734/comment/21d4571f_ef3… PS1, Line 74: -c /data/osmo-uecups-daemon.cfg > Have you checked that it actually works? The current osmo-uecups master does not parse command like […] Ah good to know,I changed the config file and I saw it applied, but I didn't think about the fact that it was taking it due to $WORKDIR. -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/27734 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: I714172ca063c76a3104daf06f52b5823f304dffe Gerrit-Change-Number: 27734 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 12 Apr 2022 09:26:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

Change in osmo-uecups[master]: main: Remove duplicate call to child_terminated()
by daniel 12 Apr '22

12 Apr '22

Attention is currently required from: laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27737 ) Change subject: main: Remove duplicate call to child_terminated() ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27737 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: I348b91097fe9bf78b2c7e33d4f3eaf316618068e Gerrit-Change-Number: 27737 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: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 12 Apr 2022 09:20:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in osmo-uecups[master]: Fix use-after-free by tun thread after tun obj destroyed
by pespin 12 Apr '22

12 Apr '22

Attention is currently required from: osmith, lynxis lazus. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27739 ) Change subject: Fix use-after-free by tun thread after tun obj destroyed ...................................................................... Patch Set 3: (1 comment) File daemon/tun_device.c: https://gerrit.osmocom.org/c/osmo-uecups/+/27739/comment/cfa1f163_2d0db5d9 PS3, Line 200: pthread_cleanup_pop(1); > Daniel already asked, but it's not answered and I don't see it: how do you reach this, since there's […] Sorry we discussed this through jabber since it's a bit of a complex topic. In summary, check "man pthread_cleanup_push". push/pop can be implemented as a while loop, as specified by POSIX. And if you check /usr/include/pthread.h, it is actually implemented that way in linux. This adds further restrictions, like having to put both push/pop, and having to put them in the same function block. So in reality the function is not called, and we don't need it called, but it's needed to make the "do while" block. -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27739 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: Idf005359afb41d3413b09281a9ff937d5eafcc7c Gerrit-Change-Number: 27739 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Tue, 12 Apr 2022 09:18:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: comment

1 0

Change in osmo-uecups[master]: Fix use-after-free by tun thread after tun obj destroyed
by osmith 12 Apr '22

12 Apr '22

Attention is currently required from: pespin, lynxis lazus. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27739 ) Change subject: Fix use-after-free by tun thread after tun obj destroyed ...................................................................... Patch Set 3: (1 comment) File daemon/tun_device.c: https://gerrit.osmocom.org/c/osmo-uecups/+/27739/comment/083b9452_9c6d4c7a PS3, Line 200: pthread_cleanup_pop(1); Daniel already asked, but it's not answered and I don't see it: how do you reach this, since there's no break in the while loop? -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27739 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: Idf005359afb41d3413b09281a9ff937d5eafcc7c Gerrit-Change-Number: 27739 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Tue, 12 Apr 2022 09:15:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in osmo-uecups[master]: Fix use-after-free by tun thread after tun obj destroyed
by daniel 12 Apr '22

12 Apr '22

Attention is currently required from: osmith, pespin, lynxis lazus. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27739 ) Change subject: Fix use-after-free by tun thread after tun obj destroyed ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File daemon/tun_device.c: https://gerrit.osmocom.org/c/osmo-uecups/+/27739/comment/6bbf091c_06a7e8d2 PS2, Line 204: pthread_cleanup_pop(tun_device_pthread_cleanup_routine); > man pthread_cleanup_pop reads: […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27739 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: Idf005359afb41d3413b09281a9ff937d5eafcc7c Gerrit-Change-Number: 27739 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Tue, 12 Apr 2022 08:57:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

Change in osmo-uecups[master]: Fix use-after-free by tun thread after tun obj destroyed
by pespin 12 Apr '22

12 Apr '22

Attention is currently required from: osmith, pespin, lynxis lazus. Hello osmith, Jenkins Builder, fixeria, daniel, lynxis lazus, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-uecups/+/27739 to look at the new patch set (#3). Change subject: Fix use-after-free by tun thread after tun obj destroyed ...................................................................... Fix use-after-free by tun thread after tun obj destroyed The main thread calls pthread_cancel before freeing the tun object. However, pthread_cancel doesn't kill the thread synchronously (man pthread_cancel). Hence, the tun thread may still be running for a while after the tun object is/has been(ing) freed. Let's avoid this by making sure the thread is stopped before freeing the object. To accomplish it, we must wait for the thread to be cancelled. A cleanup routie is added which will signal the "tun_released" message to the main thread through an osmo_itq, which will then free the object (since talloc context is managed by the main thread). Related: SYS#5523 Change-Id: Idf005359afb41d3413b09281a9ff937d5eafcc7c --- M daemon/daemon_vty.c M daemon/internal.h M daemon/main.c M daemon/tun_device.c 4 files changed, 84 insertions(+), 15 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-uecups refs/changes/39/27739/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27739 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: Idf005359afb41d3413b09281a9ff937d5eafcc7c Gerrit-Change-Number: 27739 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: newpatchset

1 0

Change in osmo-uecups[master]: Fix use-after-free by tun thread after tun obj destroyed
by daniel 12 Apr '22

12 Apr '22

Attention is currently required from: osmith, pespin, lynxis lazus. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/27739 ) Change subject: Fix use-after-free by tun thread after tun obj destroyed ...................................................................... Patch Set 2: Code-Review-1 (1 comment) File daemon/tun_device.c: https://gerrit.osmocom.org/c/osmo-uecups/+/27739/comment/c8cde187_2997d548 PS2, Line 204: pthread_cleanup_pop(tun_device_pthread_cleanup_routine); man pthread_cleanup_pop reads: """The pthread_cleanup_pop() function removes the routine at the top of the stack of clean-up handlers, and optionally executes it if execute is nonzero.""" So it will always pop the last cleanup handler, the only choice you have is if you want it to be executed or not. I also don't see how you can reach this code (at the moment), there's no break in the while loop. -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/27739 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: Idf005359afb41d3413b09281a9ff937d5eafcc7c Gerrit-Change-Number: 27739 Gerrit-PatchSet: 2 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Tue, 12 Apr 2022 06:41:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2026

2025

2024

2023

2022

gerrit-log