[lxc-devel] [lxc/master] 2016 03 02/cgfs.rmperms
hallyn on Github
lxc-bot at linuxcontainers.org
Thu Mar 3 00:26:22 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 685 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160303/6af91d48/attachment.bin>
-------------- next part --------------
From 6a9e0f26fe57c00a852d78751a8a35db3c1396fc Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at ubuntu.com>
Date: Wed, 2 Mar 2016 14:00:13 -0800
Subject: [PATCH 1/4] cgfs: switch to userns when removing cgroup
Otherwise unprivileged users may not have the privilege needed to
remove their cgroups.
Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
src/lxc/cgfs.c | 52 ++++++++++++++++++++++++++++++++++++----------------
src/lxc/cgmanager.c | 2 +-
src/lxc/cgroup.c | 2 +-
src/lxc/cgroup.h | 2 +-
4 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index 071060b..37ba342 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -132,7 +132,8 @@ static void lxc_cgroup_mount_point_free(struct cgroup_mount_point *mp);
static void lxc_cgroup_hierarchy_free(struct cgroup_hierarchy *h);
static bool is_valid_cgroup(const char *name);
static int create_cgroup(struct cgroup_mount_point *mp, const char *path);
-static int remove_cgroup(struct cgroup_mount_point *mp, const char *path, bool recurse);
+static int remove_cgroup(struct cgroup_mount_point *mp, const char *path, bool recurse,
+ struct lxc_conf *conf);
static char *cgroup_to_absolute_path(struct cgroup_mount_point *mp, const char *path, const char *suffix);
static struct cgroup_process_info *find_info_for_subsystem(struct cgroup_process_info *info, const char *subsystem);
static int do_cgroup_get(const char *cgroup_path, const char *sub_filename, char *value, size_t len);
@@ -150,7 +151,8 @@ static struct cgroup_meta_data *lxc_cgroup_put_meta(struct cgroup_meta_data *met
/* free process membership information */
static void lxc_cgroup_process_info_free(struct cgroup_process_info *info);
-static void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info);
+static void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info,
+ struct lxc_conf *conf);
static struct cgroup_ops cgfs_ops;
@@ -223,6 +225,20 @@ static int cgroup_rmdir(char *dirname)
return failed ? -1 : 0;
}
+static int rmdir_wrapper(void *data)
+{
+ char *path = data;
+
+ if (setresgid(0,0,0) < 0)
+ SYSERROR("Failed to setgid to 0");
+ if (setresuid(0,0,0) < 0)
+ SYSERROR("Failed to setuid to 0");
+ if (setgroups(0, NULL) < 0)
+ SYSERROR("Failed to clear groups");
+
+ return cgroup_rmdir(path);
+}
+
static struct cgroup_meta_data *lxc_cgroup_load_meta()
{
const char *cgroup_use = NULL;
@@ -919,7 +935,7 @@ static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const c
* In that case, remove the cgroup from all previous hierarchies
*/
for (j = 0, info_ptr = base_info; j < i && info_ptr; info_ptr = info_ptr->next, j++) {
- r = remove_cgroup(info_ptr->designated_mount_point, info_ptr->created_paths[info_ptr->created_paths_count - 1], false);
+ r = remove_cgroup(info_ptr->designated_mount_point, info_ptr->created_paths[info_ptr->created_paths_count - 1], false, NULL);
if (r < 0)
WARN("could not clean up cgroup we created when trying to create container");
free(info_ptr->created_paths[info_ptr->created_paths_count - 1]);
@@ -1077,7 +1093,7 @@ static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const c
out_initial_error:
saved_errno = errno;
free(path_so_far);
- lxc_cgroup_process_info_free_and_remove(base_info);
+ lxc_cgroup_process_info_free_and_remove(base_info, NULL);
lxc_free_array((void **)new_cgroup_paths, free);
lxc_free_array((void **)new_cgroup_paths_sub, free);
lxc_free_array((void **)cgroup_path_components, free);
@@ -1221,7 +1237,7 @@ void lxc_cgroup_process_info_free(struct cgroup_process_info *info)
}
/* free process membership information and remove cgroups that were created */
-void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info)
+void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info, struct lxc_conf *conf)
{
struct cgroup_process_info *next;
char **pp;
@@ -1237,7 +1253,7 @@ void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info)
* '/lxc' cgroup in this container but another container
* is still running (for example)
*/
- (void)remove_cgroup(mp, info->cgroup_path, true);
+ (void)remove_cgroup(mp, info->cgroup_path, true, conf);
}
for (pp = info->created_paths; pp && *pp; pp++);
for ((void)(pp && --pp); info->created_paths && pp >= info->created_paths; --pp) {
@@ -1248,7 +1264,7 @@ void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info)
free(info->cgroup_path);
free(info->cgroup_path_sub);
free(info);
- lxc_cgroup_process_info_free_and_remove(next);
+ lxc_cgroup_process_info_free_and_remove(next, conf);
}
static char *lxc_cgroup_get_hierarchy_path_data(const char *subsystem, struct cgfs_data *d)
@@ -1802,7 +1818,8 @@ static bool is_valid_cgroup(const char *name)
}
static int create_or_remove_cgroup(bool do_remove,
- struct cgroup_mount_point *mp, const char *path, int recurse)
+ struct cgroup_mount_point *mp, const char *path, int recurse,
+ struct lxc_conf *conf)
{
int r, saved_errno = 0;
char *buf = cgroup_to_absolute_path(mp, path, NULL);
@@ -1813,9 +1830,12 @@ static int create_or_remove_cgroup(bool do_remove,
if (do_remove) {
if (!dir_exists(buf))
return 0;
- if (recurse)
- r = cgroup_rmdir(buf);
- else
+ if (recurse) {
+ if (conf && !lxc_list_empty(&conf->id_map))
+ r = userns_exec_1(conf, rmdir_wrapper, buf);
+ else
+ r = cgroup_rmdir(buf);
+ } else
r = rmdir(buf);
} else
r = mkdir(buf, 0777);
@@ -1827,13 +1847,13 @@ static int create_or_remove_cgroup(bool do_remove,
static int create_cgroup(struct cgroup_mount_point *mp, const char *path)
{
- return create_or_remove_cgroup(false, mp, path, false);
+ return create_or_remove_cgroup(false, mp, path, false, NULL);
}
static int remove_cgroup(struct cgroup_mount_point *mp,
- const char *path, bool recurse)
+ const char *path, bool recurse, struct lxc_conf *conf)
{
- return create_or_remove_cgroup(true, mp, path, recurse);
+ return create_or_remove_cgroup(true, mp, path, recurse, conf);
}
static char *cgroup_to_absolute_path(struct cgroup_mount_point *mp,
@@ -2311,14 +2331,14 @@ static void *cgfs_init(const char *name)
return NULL;
}
-static void cgfs_destroy(void *hdata)
+static void cgfs_destroy(void *hdata, struct lxc_conf *conf)
{
struct cgfs_data *d = hdata;
if (!d)
return;
free(d->name);
- lxc_cgroup_process_info_free_and_remove(d->info);
+ lxc_cgroup_process_info_free_and_remove(d->info, conf);
lxc_cgroup_put_meta(d->meta);
free(d);
}
diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index 937c36e..7a35d03 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -558,7 +558,7 @@ static void *cgm_init(const char *name)
}
/* Called after a failed container startup */
-static void cgm_destroy(void *hdata)
+static void cgm_destroy(void *hdata, struct lxc_conf *conf)
{
struct cgm_data *d = hdata;
char **slist = subsystems;
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 8ee0629..5d67bd3 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -71,7 +71,7 @@ bool cgroup_init(struct lxc_handler *handler)
void cgroup_destroy(struct lxc_handler *handler)
{
if (ops) {
- ops->destroy(handler->cgroup_data);
+ ops->destroy(handler->cgroup_data, handler->conf);
handler->cgroup_data = NULL;
}
}
diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
index 63dcc92..9919486 100644
--- a/src/lxc/cgroup.h
+++ b/src/lxc/cgroup.h
@@ -41,7 +41,7 @@ struct cgroup_ops {
const char *name;
void *(*init)(const char *name);
- void (*destroy)(void *hdata);
+ void (*destroy)(void *hdata, struct lxc_conf *conf);
bool (*create)(void *hdata);
bool (*enter)(void *hdata, pid_t pid);
bool (*create_legacy)(void *hdata, pid_t pid);
From 4fee80f9d7d5105b4729d3691437536110f3c20b Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at ubuntu.com>
Date: Wed, 2 Mar 2016 15:23:33 -0800
Subject: [PATCH 2/4] cgfs: be less verbose
don't always warn about unused cgroups, it's noisy and not helpful
Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
src/lxc/cgfs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index 37ba342..0eb47a1 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -1150,7 +1150,6 @@ static struct cgroup_process_info *lxc_cgroup_get_container_info(const char *nam
path = lxc_cmd_get_cgroup_path(name, lxcpath, h->subsystems[0]);
if (!path) {
h->used = false;
- WARN("Not attaching to cgroup %s unknown to %s %s", h->subsystems[0], lxcpath, name);
continue;
}
From 77afbedf09ed41592817ecd6491ed8ada3babba9 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at ubuntu.com>
Date: Wed, 2 Mar 2016 16:11:14 -0800
Subject: [PATCH 3/4] cgfs: don't try to remove cgroups we haven't created
info_ptr->created_paths_count can be 0, so don't blindly dereference
info_ptr->created_paths[ created_paths_count - 1]. Apparently we never
used to have 0 at the cleanup_name_on_this_level before, but now that
we can fail with -eperm and not just -eexist, we do.
Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
src/lxc/cgfs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index 0eb47a1..05e7bcf 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -935,6 +935,8 @@ static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const c
* In that case, remove the cgroup from all previous hierarchies
*/
for (j = 0, info_ptr = base_info; j < i && info_ptr; info_ptr = info_ptr->next, j++) {
+ if (info_ptr->created_paths_count < 1)
+ continue;
r = remove_cgroup(info_ptr->designated_mount_point, info_ptr->created_paths[info_ptr->created_paths_count - 1], false, NULL);
if (r < 0)
WARN("could not clean up cgroup we created when trying to create container");
From a17d94a593b0ca096690d70a25cf3d434fbbc813 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at ubuntu.com>
Date: Wed, 2 Mar 2016 16:17:17 -0800
Subject: [PATCH 4/4] lxc-test-unpriv: try to start the container a second time
We have nothing else testing this, and it was a real regression in lp
bug 1552355.
Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
src/tests/lxc-test-unpriv | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/tests/lxc-test-unpriv b/src/tests/lxc-test-unpriv
index 141f509..bb3660e 100755
--- a/src/tests/lxc-test-unpriv
+++ b/src/tests/lxc-test-unpriv
@@ -125,15 +125,21 @@ run_cmd mkdir -p $HDIR/.cache/lxc
chown -R $TUSER: $HDIR/.cache/lxc
run_cmd lxc-create -t download -n c1 -- -d ubuntu -r trusty -a $ARCH
-run_cmd lxc-start -n c1 -d
-p1=$(run_cmd lxc-info -n c1 -p -H)
-[ "$p1" != "-1" ] || { echo "Failed to start container c1"; false; }
+# Make sure we can start it - twice
-run_cmd lxc-info -n c1
-run_cmd lxc-attach -n c1 -- /bin/true
+for count in `seq 1 2`; do
+ run_cmd lxc-start -n c1 -d
+
+ p1=$(run_cmd lxc-info -n c1 -p -H)
+ [ "$p1" != "-1" ] || { echo "Failed to start container c1 (run $count)"; false; }
+
+ run_cmd lxc-info -n c1
+ run_cmd lxc-attach -n c1 -- /bin/true
+
+ run_cmd lxc-stop -n c1
+done
-run_cmd lxc-stop -n c1
run_cmd lxc-copy -s -n c1 -N c2
run_cmd lxc-start -n c2 -d
p1=$(run_cmd lxc-info -n c2 -p -H)
More information about the lxc-devel
mailing list