[lxc-devel] [lxc/master] tree-wide: improve setgroups() dropping

brauner on Github lxc-bot at linuxcontainers.org
Wed Feb 12 23:17:59 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 460 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200212/5a995f50/attachment-0001.bin>
-------------- next part --------------
From b58214ac30bd8f0a25ef22f4ac5a72676df1f8e7 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 13 Feb 2020 00:16:15 +0100
Subject: [PATCH] tree-wide: improve setgroups() dropping

Drop groups before we change to userns root.

Reported-by: Teddy Reed <teddy.reed at gmail.com>
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/attach.c                |  6 +++---
 src/lxc/cgroups/cgfsng.c        | 14 ++++++--------
 src/lxc/lxccontainer.c          |  5 ++---
 src/lxc/start.c                 | 12 ++++++------
 src/lxc/storage/btrfs.c         |  5 ++---
 src/lxc/storage/storage_utils.c |  5 ++---
 6 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 5c50e1a109..7b07dce440 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -769,6 +769,9 @@ static int attach_child_main(struct attach_clone_payload *payload)
 			goto on_error;
 	}
 
+	if (!lxc_setgroups(0, NULL) && errno != EPERM)
+		goto on_error;
+
 	if (options->namespaces & CLONE_NEWUSER) {
 		/* Check whether nsuid 0 has a mapping. */
 		ns_root_uid = get_ns_uid(0);
@@ -789,9 +792,6 @@ static int attach_child_main(struct attach_clone_payload *payload)
 			goto on_error;
 	}
 
-	if (!lxc_setgroups(0, NULL) && errno != EPERM)
-		goto on_error;
-
 	/* Set {u,g}id. */
 	if (options->uid != LXC_INVALID_UID)
 		new_uid = options->uid;
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 9751fb7612..3e5345b62d 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1027,6 +1027,9 @@ static int cgroup_rmdir_wrapper(void *data)
 	gid_t nsgid = (arg->conf->root_nsgid_map != NULL) ? 0 : arg->conf->init_gid;
 	int ret;
 
+	if (!lxc_setgroups(0, NULL) && errno != EPERM)
+		return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
+
 	ret = setresgid(nsgid, nsgid, nsgid);
 	if (ret < 0)
 		return log_error_errno(-1, errno,
@@ -1039,10 +1042,6 @@ static int cgroup_rmdir_wrapper(void *data)
 				       "Failed to setresuid(%d, %d, %d)",
 				       (int)nsuid, (int)nsuid, (int)nsuid);
 
-	ret = setgroups(0, NULL);
-	if (ret < 0 && errno != EPERM)
-		return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
-
 	return cgroup_rmdir(arg->hierarchies, arg->container_cgroup);
 }
 
@@ -1494,6 +1493,9 @@ static int chown_cgroup_wrapper(void *data)
 	uid_t nsuid = (arg->conf->root_nsuid_map != NULL) ? 0 : arg->conf->init_uid;
 	gid_t nsgid = (arg->conf->root_nsgid_map != NULL) ? 0 : arg->conf->init_gid;
 
+	if (!lxc_setgroups(0, NULL) && errno != EPERM)
+		return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
+
 	ret = setresgid(nsgid, nsgid, nsgid);
 	if (ret < 0)
 		return log_error_errno(-1, errno,
@@ -1506,10 +1508,6 @@ static int chown_cgroup_wrapper(void *data)
 				       "Failed to setresuid(%d, %d, %d)",
 				       (int)nsuid, (int)nsuid, (int)nsuid);
 
-	ret = setgroups(0, NULL);
-	if (ret < 0 && errno != EPERM)
-		return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
-
 	destuid = get_ns_uid(arg->origuid);
 	if (destuid == LXC_INVALID_UID)
 		destuid = 0;
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 15c5ec9031..55e391870a 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -3660,6 +3660,8 @@ static int clone_update_rootfs(struct clone_update_data *data)
 	/* update hostname in rootfs */
 	/* we're going to mount, so run in a clean namespace to simplify cleanup */
 
+	(void)lxc_setgroups(0, NULL);
+
 	if (setgid(0) < 0) {
 		ERROR("Failed to setgid to 0");
 		return -1;
@@ -3670,9 +3672,6 @@ static int clone_update_rootfs(struct clone_update_data *data)
 		return -1;
 	}
 
-	if (setgroups(0, NULL) < 0)
-		WARN("Failed to clear groups");
-
 	if (unshare(CLONE_NEWNS) < 0)
 		return -1;
 
diff --git a/src/lxc/start.c b/src/lxc/start.c
index d70ffdf0e6..b727e3ab9b 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1198,9 +1198,6 @@ static int do_start(void *data)
 		if (!handler->conf->root_nsgid_map)
 			nsgid = handler->conf->init_gid;
 
-		if (!lxc_switch_uid_gid(nsuid, nsgid))
-			goto out_warn_father;
-
 		/* Drop groups only after we switched to a valid gid in the new
 		 * user namespace.
 		 */
@@ -1208,6 +1205,9 @@ static int do_start(void *data)
 		    (handler->am_root || errno != EPERM))
 			goto out_warn_father;
 
+		if (!lxc_switch_uid_gid(nsuid, nsgid))
+			goto out_warn_father;
+
 		ret = prctl(PR_SET_DUMPABLE, prctl_arg(1), prctl_arg(0),
 			    prctl_arg(0), prctl_arg(0));
 		if (ret < 0)
@@ -1447,9 +1447,6 @@ static int do_start(void *data)
 	if (new_gid == nsgid)
 		new_gid = LXC_INVALID_GID;
 
-	if (!lxc_switch_uid_gid(new_uid, new_gid))
-		goto out_warn_father;
-
 	/* If we are in a new user namespace we already dropped all groups when
 	 * we switched to root in the new user namespace further above. Only
 	 * drop groups if we can, so ensure that we have necessary privilege.
@@ -1461,6 +1458,9 @@ static int do_start(void *data)
 			if (!lxc_setgroups(0, NULL))
 				goto out_warn_father;
 
+	if (!lxc_switch_uid_gid(new_uid, new_gid))
+		goto out_warn_father;
+
 	ret = lxc_ambient_caps_down();
 	if (ret < 0) {
 		ERROR("Failed to clear ambient capabilities");
diff --git a/src/lxc/storage/btrfs.c b/src/lxc/storage/btrfs.c
index 160dc6df30..be66aef775 100644
--- a/src/lxc/storage/btrfs.c
+++ b/src/lxc/storage/btrfs.c
@@ -374,14 +374,13 @@ int btrfs_snapshot_wrapper(void *data)
 	const char *src;
 	struct rsync_data_char *arg = data;
 
+	(void)lxc_setgroups(0, NULL);
+
 	if (setgid(0) < 0) {
 		ERROR("Failed to setgid to 0");
 		return -1;
 	}
 
-	if (setgroups(0, NULL) < 0)
-		WARN("Failed to clear groups");
-
 	if (setuid(0) < 0) {
 		ERROR("Failed to setuid to 0");
 		return -1;
diff --git a/src/lxc/storage/storage_utils.c b/src/lxc/storage/storage_utils.c
index 6132113f20..79ee62ecbd 100644
--- a/src/lxc/storage/storage_utils.c
+++ b/src/lxc/storage/storage_utils.c
@@ -465,14 +465,13 @@ int storage_destroy_wrapper(void *data)
 {
 	struct lxc_conf *conf = data;
 
+	(void)lxc_setgroups(0, NULL);
+
 	if (setgid(0) < 0) {
 		SYSERROR("Failed to setgid to 0");
 		return -1;
 	}
 
-	if (setgroups(0, NULL) < 0)
-		SYSWARN("Failed to clear groups");
-
 	if (setuid(0) < 0) {
 		SYSERROR("Failed to setuid to 0");
 		return -1;


More information about the lxc-devel mailing list