[PATCH v2 3/3] pasta: Clean up waiting pasta child on failures
When pasta is invoked with a command rather than an existing namespace to
attach to, it spawns a child process to run a shell or other command. We
create that process during conf(), since we need the namespace to exist for
much of our setup. However, we don't want the specified command to run
until the pasta network interface is ready for use. Therefore,
pasta_spawn_cmd() executing in the child waits before exec()ing. main()
signals the child to continue with SIGUSR1 shortly before entering the
main forwarding loop.
This has the downside that if we exit due to any kind of failure between
conf() and the SIGUSR1, the child process will be around waiting
indefinitely. The user must manually clean this up.
Make this cleaner, by having the child use PR_SET_PDEATHSIG to have
itself killed if the parent dies during this window. Technically
speaking this is racy: if the parent dies before the child can call
the prctl() it will be left zombie-like as before. However, as long
as the parent completes pasta_wait_for_ns() before dying, I wasn't
able to trigger the race. Since the consequences of this going wrong
are merely a bit ugly, I think that's good enough.
Signed-off-by: David Gibson
On Thu, 11 Dec 2025 14:54:36 +1100
David Gibson
When pasta is invoked with a command rather than an existing namespace to attach to, it spawns a child process to run a shell or other command. We create that process during conf(), since we need the namespace to exist for much of our setup. However, we don't want the specified command to run until the pasta network interface is ready for use. Therefore, pasta_spawn_cmd() executing in the child waits before exec()ing. main() signals the child to continue with SIGUSR1 shortly before entering the main forwarding loop.
This has the downside that if we exit due to any kind of failure between conf() and the SIGUSR1, the child process will be around waiting indefinitely. The user must manually clean this up.
Make this cleaner, by having the child use PR_SET_PDEATHSIG to have itself killed if the parent dies during this window. Technically speaking this is racy: if the parent dies before the child can call the prctl() it will be left zombie-like as before. However, as long as the parent completes pasta_wait_for_ns() before dying, I wasn't able to trigger the race. Since the consequences of this going wrong are merely a bit ugly, I think that's good enough.
Signed-off-by: David Gibson
Is this:
Suggested-by: Paul Holzinger
--- pasta.c | 11 +++++++++++ util.c | 1 + 2 files changed, 12 insertions(+)
diff --git a/pasta.c b/pasta.c index 5c693de1..c307b8a8 100644 --- a/pasta.c +++ b/pasta.c @@ -40,6 +40,7 @@ #include
#include #include +#include #include #include @@ -189,6 +190,10 @@ static int pasta_spawn_cmd(void *arg) size_t conf_hostname_len; sigset_t set;
+ /* If the parent dies with an error, so should we */ + if (prctl(PR_SET_PDEATHSIG, SIGKILL)) + die_perror("Couldn't set PR_SET_PDEATHSIG"); + /* We run in a detached PID and mount namespace: mount /proc over */ if (mount("", "/proc", "proc", 0, NULL)) warn_perror("Couldn't mount /proc"); @@ -215,6 +220,12 @@ static int pasta_spawn_cmd(void *arg) sigaddset(&set, SIGUSR1); sigwaitinfo(&set, NULL);
+ /* Once exec()ed this process is more valuable, and easier to see and + * clean up. Let us outlive our parent now. + */ + if (prctl(PR_SET_PDEATHSIG, 0)) + die_perror("Couldn't clear PR_SET_PDEATHSIG"); + execvp(a->exe, a->argv);
die_perror("Failed to start command or shell"); diff --git a/util.c b/util.c index da12c962..27303950 100644 --- a/util.c +++ b/util.c @@ -35,6 +35,7 @@ #include "log.h" #include "pcap.h" #include "epoll_ctl.h" +#include "pasta.h" #ifdef HAS_GETRANDOM #include
#endif
-- Stefano
On Thu, Dec 11, 2025 at 08:16:45AM +0100, Stefano Brivio wrote:
On Thu, 11 Dec 2025 14:54:36 +1100 David Gibson
wrote: When pasta is invoked with a command rather than an existing namespace to attach to, it spawns a child process to run a shell or other command. We create that process during conf(), since we need the namespace to exist for much of our setup. However, we don't want the specified command to run until the pasta network interface is ready for use. Therefore, pasta_spawn_cmd() executing in the child waits before exec()ing. main() signals the child to continue with SIGUSR1 shortly before entering the main forwarding loop.
This has the downside that if we exit due to any kind of failure between conf() and the SIGUSR1, the child process will be around waiting indefinitely. The user must manually clean this up.
Make this cleaner, by having the child use PR_SET_PDEATHSIG to have itself killed if the parent dies during this window. Technically speaking this is racy: if the parent dies before the child can call the prctl() it will be left zombie-like as before. However, as long as the parent completes pasta_wait_for_ns() before dying, I wasn't able to trigger the race. Since the consequences of this going wrong are merely a bit ugly, I think that's good enough.
Signed-off-by: David Gibson
Is this:
Suggested-by: Paul Holzinger
? In any case, Cc'ing him with full quote to be sure he doesn't miss v2.
It is, yes. Sorry, forgot to include that.
--- pasta.c | 11 +++++++++++ util.c | 1 + 2 files changed, 12 insertions(+)
diff --git a/pasta.c b/pasta.c index 5c693de1..c307b8a8 100644 --- a/pasta.c +++ b/pasta.c @@ -40,6 +40,7 @@ #include
#include #include +#include #include #include @@ -189,6 +190,10 @@ static int pasta_spawn_cmd(void *arg) size_t conf_hostname_len; sigset_t set;
+ /* If the parent dies with an error, so should we */ + if (prctl(PR_SET_PDEATHSIG, SIGKILL)) + die_perror("Couldn't set PR_SET_PDEATHSIG"); + /* We run in a detached PID and mount namespace: mount /proc over */ if (mount("", "/proc", "proc", 0, NULL)) warn_perror("Couldn't mount /proc"); @@ -215,6 +220,12 @@ static int pasta_spawn_cmd(void *arg) sigaddset(&set, SIGUSR1); sigwaitinfo(&set, NULL);
+ /* Once exec()ed this process is more valuable, and easier to see and + * clean up. Let us outlive our parent now. + */ + if (prctl(PR_SET_PDEATHSIG, 0)) + die_perror("Couldn't clear PR_SET_PDEATHSIG"); + execvp(a->exe, a->argv);
die_perror("Failed to start command or shell"); diff --git a/util.c b/util.c index da12c962..27303950 100644 --- a/util.c +++ b/util.c @@ -35,6 +35,7 @@ #include "log.h" #include "pcap.h" #include "epoll_ctl.h" +#include "pasta.h" #ifdef HAS_GETRANDOM #include
#endif -- Stefano
-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
On 11/12/2025 04:54, David Gibson wrote:
When pasta is invoked with a command rather than an existing namespace to attach to, it spawns a child process to run a shell or other command. We create that process during conf(), since we need the namespace to exist for much of our setup. However, we don't want the specified command to run until the pasta network interface is ready for use. Therefore, pasta_spawn_cmd() executing in the child waits before exec()ing. main() signals the child to continue with SIGUSR1 shortly before entering the main forwarding loop.
This has the downside that if we exit due to any kind of failure between conf() and the SIGUSR1, the child process will be around waiting indefinitely. The user must manually clean this up.
Make this cleaner, by having the child use PR_SET_PDEATHSIG to have itself killed if the parent dies during this window. Technically speaking this is racy: if the parent dies before the child can call the prctl() it will be left zombie-like as before. However, as long as the parent completes pasta_wait_for_ns() before dying, I wasn't able to trigger the race. Since the consequences of this going wrong are merely a bit ugly, I think that's good enough.
Signed-off-by: David Gibson
Reviewed-by: Paul Holzinger --- pasta.c | 11 +++++++++++ util.c | 1 + 2 files changed, 12 insertions(+) diff --git a/pasta.c b/pasta.c index 5c693de1..c307b8a8 100644 --- a/pasta.c +++ b/pasta.c @@ -40,6 +40,7 @@ #include
#include #include +#include #include #include @@ -189,6 +190,10 @@ static int pasta_spawn_cmd(void *arg) size_t conf_hostname_len; sigset_t set;
+ /* If the parent dies with an error, so should we */ + if (prctl(PR_SET_PDEATHSIG, SIGKILL)) + die_perror("Couldn't set PR_SET_PDEATHSIG"); + /* We run in a detached PID and mount namespace: mount /proc over */ if (mount("", "/proc", "proc", 0, NULL)) warn_perror("Couldn't mount /proc"); @@ -215,6 +220,12 @@ static int pasta_spawn_cmd(void *arg) sigaddset(&set, SIGUSR1); sigwaitinfo(&set, NULL);
+ /* Once exec()ed this process is more valuable, and easier to see and + * clean up. Let us outlive our parent now. + */ + if (prctl(PR_SET_PDEATHSIG, 0)) + die_perror("Couldn't clear PR_SET_PDEATHSIG"); + execvp(a->exe, a->argv);
die_perror("Failed to start command or shell"); diff --git a/util.c b/util.c index da12c962..27303950 100644 --- a/util.c +++ b/util.c @@ -35,6 +35,7 @@ #include "log.h" #include "pcap.h" #include "epoll_ctl.h" +#include "pasta.h" #ifdef HAS_GETRANDOM #include
#endif
-- Paul Holzinger
participants (3)
-
David Gibson
-
Paul Holzinger
-
Stefano Brivio