From 656665670ca933255242ea7fb1a3a2a952ff7980 Mon Sep 17 00:00:00 2001
From: Andrew Nelson <andrew2nelson@arista.com>
Date: Wed, 27 Nov 2024 11:06:58 -0800
Subject: [PATCH] Refactor order of unshare operations

We ran into an issue where qsort appears to be sorting the namespaces
to unshare differently between glibc and musl. With this change, we
actually just loop through the list of namespaces three times
rather than trying to do a complex sort.
---
 ns.c | 73 ++++++++++++++++++++++--------------------------------------
 ns.h | 14 ++++++------
 2 files changed, 34 insertions(+), 53 deletions(-)

diff --git a/ns.c b/ns.c
index 9b68f36..2e3e747 100644
--- a/ns.c
+++ b/ns.c
@@ -88,31 +88,6 @@ void opts_to_nsactions(const char *shares[], enum nsaction *nsactions)
 	}
 }
 
-static int is_setns(const struct nsid *ns)
-{
-	switch (ns->action) {
-	case NSACTION_UNSHARE:
-	case NSACTION_SHARE_WITH_PARENT:
-		return 0;
-	default:
-		return 1;
-	}
-}
-
-/* cmp_nsids compares two ns IDs in a stable manner, such that
-   namespaces that are entered via setns are sorted before those that
-   are entered via unshare (or not changed at all). */
-static int cmp_nsids(const void *lhs, const void *rhs)
-{
-	int diff = is_setns(rhs) - is_setns(lhs);
-	if (diff != 0) {
-		return diff;
-	}
-	/* Both namespaces are the same kind -- keep ordering intact by comparing
-	   pointer values. */
-	return (int) ((intptr_t) lhs - (intptr_t) rhs);
-}
-
 static void ns_enter_one(struct nsid *ns)
 {
 	switch (ns->action) {
@@ -140,6 +115,8 @@ static void ns_enter_one(struct nsid *ns)
 	}
 }
 
+// Note that for namespaces that want to enter into a specific namespace,
+// we actually setns those before forking.
 static bool is_postfork_ns(struct nsid *ns)
 {
 	/* For now, only the cgroup namespace needs to be unshared postfork */
@@ -151,40 +128,44 @@ void ns_enter_prefork(struct nsid *namespaces, size_t *len)
 	/* Enter all relevant namespaces. It's hard to check in advance which
 	   namespaces are supported, so we unshare them one by one in order. */
 
-	/* If we have CAP_SYS_ADMIN from the get-go, starting by entering
-	   the userns may restrict us from joining additional namespaces, so
-	   we rearrange the order so that we setns into target nsfs files first. */
-	if (capable(BST_CAP_SYS_ADMIN)) {
-		qsort(namespaces, *len, sizeof (namespaces[0]),
-				cmp_nsids);
-	}
-
-	struct nsid *first_postfork = NULL;
+	// First we setns the things that have a specific fd to share into.
 	struct nsid *ns = &namespaces[0];
 	for (; ns < namespaces + *len; ++ns) {
-		if (ns->action != NSACTION_SHARE_WITH_PARENT && is_postfork_ns(ns)) {
-			first_postfork = ns;
-			break;
+		if (ns->action < 0) {
+			continue;
 		}
+		// Note that we also setns the postfork namespaces here. If they
+		// have a specific namespace to share into then we must share into
+		// that namespace while we are still in the user namespace of that
+		// target namespace.
 		ns_enter_one(ns);
 	}
 
-	size_t i = 0;
-	for (; ns < namespaces + *len; ++ns, ++i) {
-		if (first_postfork != NULL && !is_postfork_ns(ns)) {
-			errx(1, "incompatible options: %s namespace must be entered before "
-					"forking, but must be done after %s namespace is entered post-fork.",
-					ns_name(ns->ns),
-					ns_name(first_postfork->ns));
+	// Then setns the things that just need a blanket unshare (postfork
+	// namespaces with NSACTION_UNSHARE need to be shard post-fork).
+	ns = &namespaces[0];
+	for (; ns < namespaces + *len; ++ns) {
+		if (is_postfork_ns(ns)) {
+			continue;
 		}
-		namespaces[i] = *ns;
+		if (ns->action >= 0) {
+			continue;
+		}
+		ns_enter_one(ns);
 	}
-	*len = i;
 }
 
 void ns_enter_postfork(struct nsid *namespaces, size_t len)
 {
 	for (struct nsid *ns = &namespaces[0]; ns < namespaces + len; ++ns) {
+		if (!is_postfork_ns(ns)) {
+			// Already handled in ns_enter_prefork.
+			continue;
+		}
+		if (ns->action >= 0) {
+			// If there is an fd action then we already did this prefork.
+			continue;
+		}
 		ns_enter_one(ns);
 	}
 }
diff --git a/ns.h b/ns.h
index 35fcf9f..4550cc6 100644
--- a/ns.h
+++ b/ns.h
@@ -37,13 +37,13 @@ enum {
 
 enum nstype {
 	NS_CGROUP = 0,
-	NS_IPC,
-	NS_MNT,
-	NS_NET,
-	NS_PID,
-	NS_TIME,
-	NS_USER,
-	NS_UTS,
+	NS_IPC, // 1
+	NS_MNT, // 2
+	NS_NET, // 3
+	NS_PID, // 4
+	NS_TIME,// 5
+	NS_USER,// 6
+	NS_UTS, // 7
 
 	MAX_NS
 };