From: Balbir Singh <[email protected]> cgroup_create_cgroup() can return -1, which causes cgroup_strerror() to assert. This patch fixes that problem and another one where we incorrectly return -1 if we fail to open /proc/mounts. We should return 0 there.
The patch also fixes a potential security issue. We were freeing path and using asprintf, but path is set to fts_path[0], which we use for changing permissions recursively. I don't understand why that was done or why we use asprintf there in the first place. Please review, this is urgent. Signed-off-by: Balbir Singh <[email protected]> --- api.c | 30 ++++++++++++++++++++++-------- 1 files changed, 22 insertions(+), 8 deletions(-) diff --git a/api.c b/api.c index 72f8c69..8e35a51 100644 --- a/api.c +++ b/api.c @@ -138,6 +138,10 @@ static int cg_chown_file(FTS *fts, FTSENT *ent, uid_t owner, gid_t group) break; } fail_chown: + if (ret < 0) { + last_errno = errno; + ret = ECGOTHER; + } return ret; } @@ -680,7 +684,7 @@ static int cg_test_mounted_fs() proc_mount = fopen("/proc/mounts", "r"); if (proc_mount == NULL) { - return -1; + return 0; } temp_ent = (struct mntent *) malloc(sizeof(struct mntent)); @@ -937,15 +941,20 @@ static int cg_mkdir_p(const char *path) i = j; ret = chdir(wd); if (ret) { - printf("could not chdir to child directory (%s)\n", wd); + last_errno = errno; + ret = ECGOTHER; + dbg("could not chdir to child directory (%s)\n", wd); break; } free(wd); } while (real_path[i]); ret = chdir(buf); - if (ret) - printf("could not go back to old directory (%s)\n", cwd); + if (ret) { + last_errno = errno; + ret = ECGOTHER; + dbg("could not go back to old directory (%s)\n", cwd); + } done: free(real_path); @@ -1192,6 +1201,8 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership) cgroup->controller[k]->name)) continue; + dbg("creating path %s, controller %s\n", path, + cgroup->controller[k]->name); error = cg_create_control_group(path); if (error) goto err; @@ -1208,12 +1219,13 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership) error = cg_chown_recursive(fts_path, cgroup->control_uid, cgroup->control_gid); + dbg("changing ownership for %s, error %d\n", fts_path[0], + error); if (error) goto err; for (j = 0; j < cgroup->controller[k]->index; j++) { - free(path); - ret = asprintf(&path, "%s%s", base, + ret = sprintf(path, "%s%s", base, cgroup->controller[k]->values[j]->name); if (ret < 0) { last_errno = errno; @@ -1222,6 +1234,9 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership) } error = cg_set_control_value(path, cgroup->controller[k]->values[j]->value); + dbg("setting name %s, value %s, error %d\n", + path, cgroup->controller[k]->values[j]->value, + error); /* * Should we undo, what we've done in the loops above? * An error should not be treated as fatal, since we @@ -1238,8 +1253,7 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership) } if (!ignore_ownership) { - free(path); - ret = asprintf(&path, "%s/tasks", base); + ret = sprintf(path, "%s/tasks", base); if (ret < 0) { last_errno = errno; error = ECGOTHER; -- Balbir ------------------------------------------------------------------------------ Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H _______________________________________________ Libcg-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/libcg-devel
