From b3b889d8a24340249f668df9e68f5e95646cc004 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 29 Apr 2020 12:32:34 +0200 Subject: rootlessport: use two different channels The same channel is written to by two different goroutines. Use a different channel for each of them so to avoid writing to a closed channel. Closes: https://github.com/containers/libpod/issues/6018 Signed-off-by: Giuseppe Scrivano (cherry picked from commit 6d545bb2f773ff996ce28e0b6608380206835004) --- pkg/rootlessport/rootlessport_linux.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/rootlessport/rootlessport_linux.go b/pkg/rootlessport/rootlessport_linux.go index 1c1ed39df..c686d80fc 100644 --- a/pkg/rootlessport/rootlessport_linux.go +++ b/pkg/rootlessport/rootlessport_linux.go @@ -102,25 +102,27 @@ func parent() error { return err } - sigC := make(chan os.Signal, 1) - signal.Notify(sigC, unix.SIGPIPE) - defer func() { - // dummy signal to terminate the goroutine - sigC <- unix.SIGKILL - }() + exitC := make(chan os.Signal, 1) + defer close(exitC) + go func() { + sigC := make(chan os.Signal, 1) + signal.Notify(sigC, unix.SIGPIPE) defer func() { signal.Stop(sigC) close(sigC) }() - s := <-sigC - if s == unix.SIGPIPE { - if f, err := os.OpenFile("/dev/null", os.O_WRONLY, 0755); err == nil { - unix.Dup2(int(f.Fd()), 1) // nolint:errcheck - unix.Dup2(int(f.Fd()), 2) // nolint:errcheck - f.Close() + select { + case s := <-sigC: + if s == unix.SIGPIPE { + if f, err := os.OpenFile("/dev/null", os.O_WRONLY, 0755); err == nil { + unix.Dup2(int(f.Fd()), 1) // nolint:errcheck + unix.Dup2(int(f.Fd()), 2) // nolint:errcheck + f.Close() + } } + case <-exitC: } }() -- cgit v1.2.3-54-g00ecf From efe956249ef5a04e3bc28aa6f15c7b5d5a854351 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 29 Apr 2020 11:01:56 +0200 Subject: rootless: move ns open before fork commit 788fdc685b00dee5ccb594bef845204250c4c123 introduced a race where the target process dies before the child process opens the namespace files. Move the open before the fork so if it fails the parent process can attempt to join a different container instead of failing. Signed-off-by: Giuseppe Scrivano (cherry picked from commit 89d4940a3787ccc871c92950a79347efc0d5c58c) --- pkg/rootless/rootless_linux.c | 48 ++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/pkg/rootless/rootless_linux.c b/pkg/rootless/rootless_linux.c index 72d461cdc..716db81dc 100644 --- a/pkg/rootless/rootless_linux.c +++ b/pkg/rootless/rootless_linux.c @@ -535,32 +535,30 @@ create_pause_process (const char *pause_pid_file_path, char **argv) } } -static void -join_namespace_or_die (int pid_to_join, const char *ns_file) +static int +open_namespace (int pid_to_join, const char *ns_file) { char ns_path[PATH_MAX]; int ret; - int fd; ret = snprintf (ns_path, PATH_MAX, "/proc/%d/ns/%s", pid_to_join, ns_file); if (ret == PATH_MAX) { fprintf (stderr, "internal error: namespace path too long\n"); - _exit (EXIT_FAILURE); + return -1; } - fd = open (ns_path, O_CLOEXEC | O_RDONLY); - if (fd < 0) - { - fprintf (stderr, "cannot open: %s\n", ns_path); - _exit (EXIT_FAILURE); - } - if (setns (fd, 0) < 0) + return open (ns_path, O_CLOEXEC | O_RDONLY); +} + +static void +join_namespace_or_die (const char *name, int ns_fd) +{ + if (setns (ns_fd, 0) < 0) { - fprintf (stderr, "cannot set namespace to %s: %s\n", ns_path, strerror (errno)); + fprintf (stderr, "cannot set %s namespace\n", name); _exit (EXIT_FAILURE); } - close (fd); } int @@ -570,6 +568,8 @@ reexec_userns_join (int pid_to_join, char *pause_pid_file_path) char gid[16]; char **argv; int pid; + int mnt_ns = -1; + int user_ns = -1; char *cwd = getcwd (NULL, 0); sigset_t sigset, oldsigset; @@ -589,14 +589,28 @@ reexec_userns_join (int pid_to_join, char *pause_pid_file_path) _exit (EXIT_FAILURE); } + user_ns = open_namespace (pid_to_join, "user"); + if (user_ns < 0) + return user_ns; + mnt_ns = open_namespace (pid_to_join, "mnt"); + if (mnt_ns < 0) + { + close (user_ns); + return mnt_ns; + } + pid = fork (); if (pid < 0) fprintf (stderr, "cannot fork: %s\n", strerror (errno)); if (pid) { - /* We passed down these fds, close them. */ int f; + + /* We passed down these fds, close them. */ + close (user_ns); + close (mnt_ns); + for (f = 3; f < open_files_max_fd; f++) if (open_files_set == NULL || FD_ISSET (f % FD_SETSIZE, &(open_files_set[f / FD_SETSIZE]))) close (f); @@ -634,8 +648,10 @@ reexec_userns_join (int pid_to_join, char *pause_pid_file_path) _exit (EXIT_FAILURE); } - join_namespace_or_die (pid_to_join, "user"); - join_namespace_or_die (pid_to_join, "mnt"); + join_namespace_or_die ("user", user_ns); + join_namespace_or_die ("mnt", mnt_ns); + close (user_ns); + close (mnt_ns); if (syscall_setresgid (0, 0, 0) < 0) { -- cgit v1.2.3-54-g00ecf