From 54cff3d8f9855e52ede42ee986bfb23efe2a6fee Mon Sep 17 00:00:00 2001 From: "Franklin \"Snaipe\" Mathieu" Date: Thu, 28 Nov 2024 12:23:04 +0100 Subject: [PATCH] enter: pass nsactions array rather than path array Before this commit, we used to construct shared namespace paths purely from string operations; that is, --share pid,mnt=/foo would be reconstructed as --share pid=/foo/pid --share mnt=/foo/mnt. The problem with this approach is that this doesn't work well with magic links to file descriptors in /proc. Instead, we now open the directory path and use its file descriptor to open further nsfs files. --- enter.c | 7 ++-- enter.h | 10 +---- main.c | 115 +++++++++++++++++++++++++++++++++++++++++++------------- ns.c | 29 +------------- ns.h | 2 +- 5 files changed, 95 insertions(+), 68 deletions(-) diff --git a/enter.c b/enter.c index c0ce055..eb43650 100644 --- a/enter.c +++ b/enter.c @@ -142,7 +142,7 @@ int enter(struct entry_settings *opts) } int timens_offsets = -1; - if (opts->shares[NS_TIME] != SHARE_WITH_PARENT) { + if (opts->nsactions[NS_TIME] != NSACTION_SHARE_WITH_PARENT) { /* Because this process is privileged, /proc/self/timens_offsets is unfortunately owned by root and not ourselves, so we have @@ -158,14 +158,13 @@ int enter(struct entry_settings *opts) /* The kernel evidently doesn't support time namespaces yet. Don't try to open the time namespace file with --share-all=, or try to unshare or setns the time namespace below. */ - opts->shares[NS_TIME] = SHARE_WITH_PARENT; + opts->nsactions[NS_TIME] = NSACTION_SHARE_WITH_PARENT; } reset_capabilities(); } - enum nsaction nsactions[MAX_NS]; - opts_to_nsactions(opts->shares, nsactions); + enum nsaction *nsactions = opts->nsactions; if (nsactions[NS_NET] != NSACTION_UNSHARE && opts->nnics > 0) { errx(1, "cannot create NICs when not in a network namespace"); diff --git a/enter.h b/enter.h index a85c575..31a5d33 100644 --- a/enter.h +++ b/enter.h @@ -48,15 +48,9 @@ enum { MAX_CLOSE_FDS = 4096, }; -/* SHARE_WITH_PARENT is a special value for entry_settings.shares[ns]. */ -# define SHARE_WITH_PARENT ((char *) -1) - struct entry_settings { - /* shares[] is indexed by SHARE_CGROUP, etc. Legal values are: - NULL: unshare. - SHARE_WITH_PARENT: special marker meaning don't unshare or setns. - filename: setns to the given namespace file. */ - const char *shares[MAX_NS]; + /* nsactions[] is indexed by NS_CGROUP, etc. */ + enum nsaction nsactions[MAX_NS]; const char *persist[MAX_NS]; const char *pathname; diff --git a/main.c b/main.c index f1fc7ab..f3c4fac 100644 --- a/main.c +++ b/main.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -73,25 +74,25 @@ enum { OPTION_NO_COPY_HARD_RLIMITS, }; -static void process_nslist_entry(const char **out, const char *share, const char *path, int append_nsname) +/* SHARE_WITH_PARENT is a special value for the path passed to process_nslist_entry + * that means sharing with the parent. */ +# define SHARE_WITH_PARENT ((char *) -1) + +static enum nstype find_nstype_by_name(const char *nsname) { - if (!strcmp(share, "network")) { - share = "net"; + if (!strcmp(nsname, "network")) { + nsname = "net"; } - if (!strcmp(share, "mount")) { - share = "mnt"; + if (!strcmp(nsname, "mount")) { + nsname = "mnt"; } for (enum nstype ns = 0; ns < MAX_NS; ns++) { - if (!strcmp(share, ns_name(ns))) { - if (append_nsname) { - out[ns] = strdup(makepath("%s/%s", path, ns_name(ns))); - } else { - out[ns] = path; - } - return; + if (!strcmp(nsname, ns_name(ns))) { + return ns; } } - fprintf(stderr, "namespace `%s` does not exist.\n", share); + + fprintf(stderr, "namespace `%s` does not exist.\n", nsname); fprintf(stderr, "valid namespaces are: "); for (enum nstype ns = 0; ns < MAX_NS; ns++) { fprintf(stderr, "%s%s", ns == 0 ? "" : ", ", ns_name(ns)); @@ -100,9 +101,56 @@ static void process_nslist_entry(const char **out, const char *share, const char exit(2); } +static void process_nslist_entry(enum nsaction *nsactions, const char *nsname, int dirfd, const char *path) +{ + enum nstype ns = find_nstype_by_name(nsname); + enum nsaction *action = &nsactions[ns]; + + if (*action >= 0) { + close(*action); + } + + if (path == NULL) { + *action = NSACTION_UNSHARE; + } else if (path == SHARE_WITH_PARENT) { + *action = NSACTION_SHARE_WITH_PARENT; + } else { + /* nsname might not be the canonical filename, so fix it */ + nsname = ns_name(ns); + + int fd = openat(dirfd, nsname, O_RDONLY | O_CLOEXEC); + if (fd == -1) { + err(1, "open /%s", nsname); + } + + /* If the nsfd refers to the same namespace as the one we are + currently in, treat as for SHARE_WITH_PARENT. + + We want to give semantics to --share =/proc/self/ns/ + being the same as --share . */ + if (is_nsfd_current(fd, nsname)) { + close(fd); + fd = NSACTION_SHARE_WITH_PARENT; + } + + *action = fd; + } +} + +static void build_ns_path(const char **out, const char *nsname, const char *path, int append_nsname) +{ + enum nstype ns = find_nstype_by_name(nsname); + + if (append_nsname) { + out[ns] = strdup(makepath("%s/%s", path, ns_name(ns))); + } else { + out[ns] = path; + } +} + #define ALL_NAMESPACES "cgroup,ipc,mnt,net,pid,time,user,uts" -static void process_share(const char **out, const char *optarg) +static void process_share(enum nsaction *nsactions, const char *optarg) { char *nsnames = strtok((char *) optarg, "="); char *path = strtok(NULL, ""); @@ -119,18 +167,27 @@ static void process_share(const char **out, const char *optarg) } size_t nsnames_len = strlen(nsnames); - const char *share = strtok(nsnames, ","); + const char *nsname = strtok(nsnames, ","); + + int dirfd = AT_FDCWD; - /* Only append the ns name to the path if there is more than one - namespace specified. */ - bool append_nsname = share + strlen(share) != nsnames + nsnames_len; if (path == NULL) { path = SHARE_WITH_PARENT; - append_nsname = false; + } else if (nsname + strlen(nsname) != nsnames + nsnames_len) { + /* We have more that one ns name specified, which means the path + refers to a directory of nsfs files */ + dirfd = open(path, O_PATH | O_DIRECTORY | O_CLOEXEC); + if (dirfd == -1) { + err(1, "open %s", path); + } } - for (; share != NULL; share = strtok(NULL, ",")) { - process_nslist_entry(out, share, path, append_nsname); + for (; nsname != NULL; nsname = strtok(NULL, ",")) { + process_nslist_entry(nsactions, nsname, dirfd, path); + } + + if (dirfd != AT_FDCWD) { + close(dirfd); } } @@ -158,11 +215,11 @@ static void process_persist(const char **out, const char *optarg) bool multiple = share + strlen(share) != nsnames + nsnames_len; for (; share != NULL; share = strtok(NULL, ",")) { - process_nslist_entry(out, share, path, multiple); + build_ns_path(out, share, path, multiple); } } -static void process_unshare(const char **out, char *nsnames) +static void process_unshare(enum nsaction *nsactions, char *nsnames) { /* Similar rules as for process_share, but we do not take in paths. */ @@ -172,7 +229,7 @@ static void process_unshare(const char **out, char *nsnames) } for (const char *ns = strtok(nsnames, ","); ns != NULL; ns = strtok(NULL, ",")) { - process_nslist_entry(out, ns, NULL, false); + process_nslist_entry(nsactions, ns, 0, NULL); } } @@ -279,6 +336,10 @@ int main(int argc, char *argv[], char *envp[]) opts.umask = (mode_t) -1; opts.cgroup_driver = (enum cgroup_driver) -1; + for (enum nstype ns = 0; ns < MAX_NS; ns++) { + opts.nsactions[ns] = NSACTION_UNSHARE; + } + static struct option options[] = { { "help", no_argument, NULL, 'h' }, { "root", required_argument, NULL, 'r' }, @@ -431,11 +492,11 @@ int main(int argc, char *argv[], char *envp[]) break; case OPTION_SHARE: - process_share(opts.shares, optarg); + process_share(opts.nsactions, optarg); break; case OPTION_UNSHARE: - process_unshare(opts.shares, optarg); + process_unshare(opts.nsactions, optarg); break; case OPTION_PERSIST: @@ -872,7 +933,7 @@ int main(int argc, char *argv[], char *envp[]) /* Use our own default init if we unshare the pid namespace, and no --init has been specified. */ - if (opts.shares[NS_PID] == NULL && opts.init == NULL) { + if (opts.nsactions[NS_PID] == NSACTION_UNSHARE && opts.init == NULL) { opts.init = BINDIR "/bst-init"; } diff --git a/ns.c b/ns.c index 9b68f36..e95ab98 100644 --- a/ns.c +++ b/ns.c @@ -40,7 +40,7 @@ int ns_cloneflag(enum nstype ns) return flags[ns].flag; } -static bool is_nsfd_current(int nsfd, const char *name) +bool is_nsfd_current(int nsfd, const char *name) { const char *path = makepath("/proc/self/ns/%s", name); @@ -61,33 +61,6 @@ static bool is_nsfd_current(int nsfd, const char *name) return self.st_ino == stat.st_ino; } -void opts_to_nsactions(const char *shares[], enum nsaction *nsactions) -{ - for (enum nstype ns = 0; ns < MAX_NS; ns++) { - const char *share = shares[ns]; - if (share == NULL) { - nsactions[ns] = NSACTION_UNSHARE; - } else if (share == SHARE_WITH_PARENT) { - nsactions[ns] = NSACTION_SHARE_WITH_PARENT; - } else { - nsactions[ns] = open(share, O_RDONLY | O_CLOEXEC); - if (nsactions[ns] < 0) { - err(1, "open %s", share); - } - - /* If the nsfd refers to the same namespace as the one we are - currently in, treat as for SHARE_WITH_PARENT. - - We want to give semantics to --share =/proc/self/ns/ - being the same as --share . */ - if (is_nsfd_current(nsactions[ns], ns_name(ns))) { - close(nsactions[ns]); - nsactions[ns] = NSACTION_SHARE_WITH_PARENT; - } - } - } -} - static int is_setns(const struct nsid *ns) { switch (ns->action) { diff --git a/ns.h b/ns.h index 35fcf9f..2d5656b 100644 --- a/ns.h +++ b/ns.h @@ -61,7 +61,7 @@ struct nsid { const char *ns_name(enum nstype); int ns_cloneflag(enum nstype); -void opts_to_nsactions(const char *shares[], enum nsaction *nsactions); +bool is_nsfd_current(int nsfd, const char *name); void ns_enter_prefork(struct nsid *namespaces, size_t *len); void ns_enter_postfork(struct nsid *namespaces, size_t len);