From 8af07f821c38224beac5f6d3e4970082c6732fc6 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Sun, 9 Sep 2018 16:30:20 +0200 Subject: [PATCH] utils: make lxc_setgroups() return bool Signed-off-by: Christian Brauner --- src/lxc/attach.c | 3 +-- src/lxc/cmd/lxc_usernsexec.c | 3 +-- src/lxc/start.c | 31 +++++++++++++------------------ src/lxc/storage/rsync.c | 6 ++---- src/lxc/utils.c | 10 +++++----- src/lxc/utils.h | 2 +- 6 files changed, 23 insertions(+), 32 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 8387bbfe1..951d3bb93 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -859,8 +859,7 @@ static int attach_child_main(struct attach_clone_payload *payload) goto on_error; } - ret = lxc_setgroups(0, NULL); - if (ret < 0 && errno != EPERM) + if (!lxc_setgroups(0, NULL) && errno != EPERM) goto on_error; /* Set {u,g}id. */ diff --git a/src/lxc/cmd/lxc_usernsexec.c b/src/lxc/cmd/lxc_usernsexec.c index bdfef0fb2..0b698f86d 100644 --- a/src/lxc/cmd/lxc_usernsexec.c +++ b/src/lxc/cmd/lxc_usernsexec.c @@ -108,8 +108,7 @@ static int do_child(void *vargv) if (ret < 0) return -1; - ret = lxc_setgroups(0, NULL); - if (ret < 0) + if (!lxc_setgroups(0, NULL)) return -1; ret = unshare(CLONE_NEWNS); diff --git a/src/lxc/start.c b/src/lxc/start.c index 951548dc0..8d0e2a1e6 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1046,7 +1046,6 @@ static int do_start(void *data) { int ret; char path[PATH_MAX]; - bool have_cap_setgid; uid_t new_uid; gid_t new_gid; struct lxc_list *iterator; @@ -1132,8 +1131,8 @@ static int do_start(void *data) /* Drop groups only after we switched to a valid gid in the new * user namespace. */ - ret = lxc_setgroups(0, NULL); - if (ret < 0 && (handler->am_root || errno != EPERM)) + if (!lxc_setgroups(0, NULL) && + (handler->am_root || errno != EPERM)) goto out_warn_father; ret = prctl(PR_SET_DUMPABLE, prctl_arg(1), prctl_arg(0), @@ -1356,21 +1355,6 @@ static int do_start(void *data) new_uid = handler->conf->init_uid; new_gid = handler->conf->init_gid; - /* 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. - */ - #if HAVE_LIBCAP - have_cap_setgid = lxc_proc_cap_is_set(CAP_SETGID, CAP_EFFECTIVE); - #else - have_cap_setgid = false; - #endif - if (lxc_list_empty(&handler->conf->id_map) && have_cap_setgid) { - ret = lxc_setgroups(0, NULL); - if (ret < 0) - goto out_warn_father; - } - /* Avoid unnecessary syscalls. */ if (new_uid == nsuid) new_uid = LXC_INVALID_UID; @@ -1382,6 +1366,17 @@ static int do_start(void *data) if (ret < 0) 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. + */ + if (lxc_list_empty(&handler->conf->id_map)) + #if HAVE_LIBCAP + if (lxc_proc_cap_is_set(CAP_SETGID, CAP_EFFECTIVE)) + #endif + if (!lxc_setgroups(0, NULL)) + goto out_warn_father; + ret = lxc_ambient_caps_down(); if (ret < 0) { ERROR("Failed to clear ambient capabilities"); diff --git a/src/lxc/storage/rsync.c b/src/lxc/storage/rsync.c index 83871ae80..e53a538db 100644 --- a/src/lxc/storage/rsync.c +++ b/src/lxc/storage/rsync.c @@ -54,8 +54,7 @@ int lxc_rsync_exec_wrapper(void *data) if (ret < 0) return -1; - ret = lxc_setgroups(0, NULL); - if (ret < 0) + if (!lxc_setgroups(0, NULL)) return -1; return lxc_rsync_exec(args->src, args->dest); @@ -121,8 +120,7 @@ int lxc_rsync(struct rsync_data *data) if (ret < 0) return -1; - ret = lxc_setgroups(0, NULL); - if (ret < 0) + if (!lxc_setgroups(0, NULL)) return -1; src = lxc_storage_get_path(orig->dest, orig->type); diff --git a/src/lxc/utils.c b/src/lxc/utils.c index ff02bba96..9c30dc2ea 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -1377,15 +1377,15 @@ int lxc_switch_uid_gid(uid_t uid, gid_t gid) } /* Simple covenience function which enables uniform logging. */ -int lxc_setgroups(int size, gid_t list[]) +bool lxc_setgroups(int size, gid_t list[]) { if (setgroups(size, list) < 0) { - SYSERROR("Failed to setgroups()."); - return -errno; + SYSERROR("Failed to setgroups()"); + return false; } - NOTICE("Dropped additional groups."); + NOTICE("Dropped additional groups"); - return 0; + return true; } static int lxc_get_unused_loop_dev_legacy(char *loop_name) diff --git a/src/lxc/utils.h b/src/lxc/utils.h index 947b15e16..e6a82978f 100644 --- a/src/lxc/utils.h +++ b/src/lxc/utils.h @@ -362,7 +362,7 @@ extern bool task_blocks_signal(pid_t pid, int signal); * If LXC_INVALID_{G,U}ID is passed then the set{g,u}id() will not be called. */ extern int lxc_switch_uid_gid(uid_t uid, gid_t gid); -extern int lxc_setgroups(int size, gid_t list[]); +extern bool lxc_setgroups(int size, gid_t list[]); /* Find an unused loop device and associate it with source. */ extern int lxc_prepare_loop_dev(const char *source, char *loop_dev, int flags); -- 2.39.2