From: "S.Çağlar Onur" <cag...@10ur.org>
Trying to stop multiple containers concurrently ends up with "cgroup is not
mounted" errors as multiple threads corrupts the shared variables.
Fix that stack corruption and start to use getmntent_r to support stopping
multiple containers concurrently.
---
src/lxc/cgroup.c | 152 +++++++++++++++++++++++++++++++++++------------------
src/lxc/freezer.c | 18 +++++--
src/lxc/state.c | 15 ++++--
3 files changed, 126 insertions(+), 59 deletions(-)
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 812bfb8..7212f01 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -54,6 +54,11 @@ lxc_log_define(lxc_cgroup, lxc);
#define MTAB "/proc/mounts"
+/* In the case of a bind mount, there could be two long pathnames in the
+ * mntent plus options so use large enough buffer size
+ */
+#define LARGE_MAXPATHLEN 4 * MAXPATHLEN
+
/* Check if a mount is a cgroup hierarchy for any subsystem.
* Return the first subsystem found (or NULL if none).
*/
@@ -98,29 +103,31 @@ static char *mount_has_subsystem(const struct mntent
*mntent)
*/
static int get_cgroup_mount(const char *subsystem, char *mnt)
{
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
FILE *file = NULL;
int ret, err = -1;
+ char buf[LARGE_MAXPATHLEN] = {0};
+
file = setmntent(MTAB, "r");
if (!file) {
SYSERROR("failed to open %s", MTAB);
return -1;
}
- while ((mntent = getmntent(file))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
+ if (strcmp(mntent_r.mnt_type, "cgroup") != 0)
continue;
-
+
if (subsystem) {
- if (!hasmntopt(mntent, subsystem))
+ if (!hasmntopt(&mntent_r, subsystem))
continue;
} else {
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
}
- ret = snprintf(mnt, MAXPATHLEN, "%s", mntent->mnt_dir);
+ ret = snprintf(mnt, MAXPATHLEN, "%s", mntent_r.mnt_dir);
if (ret < 0 || ret >= MAXPATHLEN)
goto fail;
@@ -146,22 +153,33 @@ out:
*
* Returns 0 on success, -1 on error.
*
- * The answer is written in a static char[MAXPATHLEN] in this function and
- * should not be freed.
*/
extern int cgroup_path_get(char **path, const char *subsystem, const char
*cgpath)
{
- static char buf[MAXPATHLEN];
- static char retbuf[MAXPATHLEN];
int rc;
+ char *buf = NULL;
+ char *retbuf = NULL;
+
+ buf = malloc(MAXPATHLEN * sizeof(char));
+ if (!buf) {
+ ERROR("malloc failed");
+ goto fail;
+ }
+
+ retbuf = malloc(MAXPATHLEN * sizeof(char));
+ if (!retbuf) {
+ ERROR("malloc failed");
+ goto fail;
+ }
+
/* lxc_cgroup_set passes a state object for the subsystem,
* so trim it to just the subsystem part */
if (subsystem) {
rc = snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("subsystem name too long");
- return -1;
+ goto fail;
}
char *s = index(retbuf, '.');
if (s)
@@ -170,19 +188,28 @@ extern int cgroup_path_get(char **path, const char
*subsystem, const char *cgpat
}
if (get_cgroup_mount(subsystem ? retbuf : NULL, buf)) {
ERROR("cgroup is not mounted");
- return -1;
+ goto fail;
}
rc = snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, cgpath);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("name too long");
- return -1;
+ goto fail;
}
DEBUG("%s: returning %s for subsystem %s", __func__, retbuf, subsystem);
+ if(buf)
+ free(buf);
+
*path = retbuf;
return 0;
+fail:
+ if (buf)
+ free(buf);
+ if (retbuf)
+ free(retbuf);
+ return -1;
}
/*
@@ -290,20 +317,25 @@ static int do_cgroup_set(const char *path, const char
*value)
int lxc_cgroup_set_bypath(const char *cgpath, const char *filename, const char
*value)
{
int ret;
- char *dirpath;
+ char *dirpath = NULL;
char path[MAXPATHLEN];
ret = cgroup_path_get(&dirpath, filename, cgpath);
if (ret)
- return -1;
+ goto fail;
ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
return do_cgroup_set(path, value);
+
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
/*
@@ -321,20 +353,25 @@ int lxc_cgroup_set(const char *name, const char
*filename, const char *value,
const char *lxcpath)
{
int ret;
- char *dirpath;
+ char *dirpath = NULL;
char path[MAXPATHLEN];
ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath);
if (ret)
- return -1;
+ goto fail;
ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
return do_cgroup_set(path, value);
+
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
/*
@@ -361,24 +398,24 @@ int lxc_cgroup_get(const char *name, const char
*filename, char *value,
size_t len, const char *lxcpath)
{
int fd, ret = -1;
- char *dirpath;
+ char *dirpath = NULL;
char path[MAXPATHLEN];
int rc;
ret = lxc_cgroup_path_get(&dirpath, filename, name, lxcpath);
if (ret)
- return -1;
+ goto fail;
rc = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
fd = open(path, O_RDONLY);
if (fd < 0) {
ERROR("open %s : %s", path, strerror(errno));
- return -1;
+ goto fail;
}
if (!len || !value) {
@@ -398,24 +435,28 @@ int lxc_cgroup_get(const char *name, const char
*filename, char *value,
close(fd);
return ret;
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
int lxc_cgroup_nrtasks(const char *cgpath)
{
- char *dpath;
+ char *dirpath = NULL;
char path[MAXPATHLEN];
int pid, ret, count = 0;
FILE *file;
int rc;
- ret = cgroup_path_get(&dpath, NULL, cgpath);
+ ret = cgroup_path_get(&dirpath, NULL, cgpath);
if (ret)
- return -1;
+ goto fail;
- rc = snprintf(path, MAXPATHLEN, "%s/tasks", dpath);
+ rc = snprintf(path, MAXPATHLEN, "%s/tasks", dirpath);
if (rc < 0 || rc >= MAXPATHLEN) {
ERROR("pathname too long");
- return -1;
+ goto fail;
}
file = fopen(path, "r");
@@ -430,6 +471,10 @@ int lxc_cgroup_nrtasks(const char *cgpath)
fclose(file);
return count;
+fail:
+ if(dirpath)
+ free(dirpath);
+ return -1;
}
/*
@@ -470,21 +515,23 @@ static void set_clone_children(const char *mntdir)
static int create_lxcgroups(const char *lxcgroup)
{
FILE *file = NULL;
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
int ret, retv = -1;
char path[MAXPATHLEN];
+ char buf[LARGE_MAXPATHLEN] = {0};
+
file = setmntent(MTAB, "r");
if (!file) {
SYSERROR("failed to open %s", MTAB);
return -1;
}
- while ((mntent = getmntent(file))) {
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
/*
@@ -492,11 +539,11 @@ static int create_lxcgroups(const char *lxcgroup)
* We probably only want to support that for /users/joe
*/
ret = snprintf(path, MAXPATHLEN, "%s/%s",
- mntent->mnt_dir, lxcgroup ? lxcgroup : "lxc");
+ mntent_r.mnt_dir, lxcgroup ? lxcgroup : "lxc");
if (ret < 0 || ret >= MAXPATHLEN)
goto fail;
if (access(path, F_OK)) {
- set_clone_children(mntent->mnt_dir);
+ set_clone_children(mntent_r.mnt_dir);
ret = mkdir(path, 0755);
if (ret == -1 && errno != EEXIST) {
SYSERROR("failed to create '%s' directory",
path);
@@ -542,7 +589,9 @@ char *lxc_cgroup_path_create(const char *lxcgroup, const
char *name)
char *retpath, path[MAXPATHLEN];
char tail[12];
FILE *file = NULL;
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
+
+ char buf[LARGE_MAXPATHLEN] = {0};
if (create_lxcgroups(lxcgroup) < 0)
return NULL;
@@ -559,15 +608,15 @@ again:
else
*tail = '\0';
- while ((mntent = getmntent(file))) {
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
/* find unused mnt_dir + lxcgroup + name + -$i */
- ret = snprintf(path, MAXPATHLEN, "%s/%s/%s%s", mntent->mnt_dir,
+ ret = snprintf(path, MAXPATHLEN, "%s/%s/%s%s", mntent_r.mnt_dir,
lxcgroup ? lxcgroup : "lxc", name, tail);
if (ret < 0 || ret >= MAXPATHLEN)
goto fail;
@@ -607,8 +656,9 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid)
{
char path[MAXPATHLEN];
FILE *file = NULL, *fout;
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
int ret, retv = -1;
+ char buf[LARGE_MAXPATHLEN] = {0};
file = setmntent(MTAB, "r");
if (!file) {
@@ -616,13 +666,13 @@ int lxc_cgroup_enter(const char *cgpath, pid_t pid)
return -1;
}
- while ((mntent = getmntent(file))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
ret = snprintf(path, MAXPATHLEN, "%s/%s/tasks",
- mntent->mnt_dir, cgpath);
+ mntent_r.mnt_dir, cgpath);
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("entering cgroup");
goto out;
@@ -714,23 +764,25 @@ static int lxc_one_cgroup_destroy(struct mntent *mntent,
const char *cgpath)
*/
int lxc_cgroup_destroy(const char *cgpath)
{
- struct mntent *mntent;
+ struct mntent *mntent, mntent_r;
FILE *file = NULL;
int err, retv = 0;
+ char buf[LARGE_MAXPATHLEN] = {0};
+
file = setmntent(MTAB, "r");
if (!file) {
SYSERROR("failed to open %s", MTAB);
return -1;
}
- while ((mntent = getmntent(file))) {
- if (strcmp(mntent->mnt_type, "cgroup"))
+ while ((mntent = getmntent_r(file, &mntent_r, buf, sizeof(buf)))) {
+ if (strcmp(mntent_r.mnt_type, "cgroup"))
continue;
- if (!mount_has_subsystem(mntent))
+ if (!mount_has_subsystem(&mntent_r))
continue;
- err = lxc_one_cgroup_destroy(mntent, cgpath);
+ err = lxc_one_cgroup_destroy(&mntent_r, cgpath);
if (err) // keep trying to clean up the others
retv = -1;
}
diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
index 111bc35..35bf3a7 100644
--- a/src/lxc/freezer.c
+++ b/src/lxc/freezer.c
@@ -120,14 +120,19 @@ out:
static int freeze_unfreeze(const char *name, int freeze, const char *lxcpath)
{
- char *nsgroup;
+ char *nsgroup = NULL;
int ret;
ret = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath);
if (ret)
- return -1;
+ goto fail;
return do_unfreeze(nsgroup, freeze, name, lxcpath);
+
+fail:
+ if (nsgroup)
+ free(nsgroup);
+ return -1;
}
int lxc_freeze(const char *name, const char *lxcpath)
@@ -143,12 +148,17 @@ int lxc_unfreeze(const char *name, const char *lxcpath)
int lxc_unfreeze_bypath(const char *cgpath)
{
- char *nsgroup;
+ char *nsgroup = NULL;
int ret;
ret = cgroup_path_get(&nsgroup, "freezer", cgpath);
if (ret)
- return -1;
+ goto fail;
return do_unfreeze(nsgroup, 0, NULL, NULL);
+
+fail:
+ if (nsgroup)
+ free(nsgroup);
+ return -1;
}
diff --git a/src/lxc/state.c b/src/lxc/state.c
index c50ef00..89e438c 100644
--- a/src/lxc/state.c
+++ b/src/lxc/state.c
@@ -68,7 +68,7 @@ lxc_state_t lxc_str2state(const char *state)
static int freezer_state(const char *name, const char *lxcpath)
{
- char *nsgroup;
+ char *nsgroup = NULL;
char freezer[MAXPATHLEN];
char status[MAXPATHLEN];
FILE *file;
@@ -76,25 +76,30 @@ static int freezer_state(const char *name, const char
*lxcpath)
err = lxc_cgroup_path_get(&nsgroup, "freezer", name, lxcpath);
if (err)
- return -1;
+ goto fail;
err = snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
if (err < 0 || err >= MAXPATHLEN)
- return -1;
+ goto fail;
file = fopen(freezer, "r");
if (!file)
- return -1;
+ goto fail;
err = fscanf(file, "%s", status);
fclose(file);
if (err == EOF) {
SYSERROR("failed to read %s", freezer);
- return -1;
+ goto fail;
}
return lxc_str2state(status);
+
+fail:
+ if (nsgroup)
+ free(nsgroup);
+ return -1;
}
static lxc_state_t __lxc_getstate(const char *name, const char *lxcpath)
--
1.7.10.4
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
