From 5ef29f580daf9c8d5e6a8af096fb30b1cf9a3aa5 Mon Sep 17 00:00:00 2001 From: Dhaval Giani Date: Sat, 21 Feb 2009 07:28:19 +0000 Subject: libcgorup: Fix a chown security issue From: Balbir Singh Impact: Bug fix causes incorrect chown This patch fixes a potential security issue, we free path and add reallocate it using asprintf, but that breaks chown, since that relies on fts_path[0] and path to point to the same address location. Please review, comment. [dhaval@linux.vnet.ibm.com: Fixed the return checks] Signed-off-by: Balbir Singh Signed-off-by: Dhaval Giani git-svn-id: https://libcg.svn.sourceforge.net/svnroot/libcg/trunk@335 4f4bb910-9a46-0410-90c8-c897d4f1cd53 --- api.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/api.c b/api.c index 91b26e0..1fb0e97 100644 --- a/api.c +++ b/api.c @@ -1211,18 +1211,21 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership) goto err; } - if (!ignore_ownership) + if (!ignore_ownership) { + dbg("Changing ownership of %s\n", fts_path[0]); error = cg_chown_recursive(fts_path, cgroup->control_uid, cgroup->control_gid); + } if (error) goto err; for (j = 0; j < cgroup->controller[k]->index; j++) { - free(path); - ret = asprintf(&path, "%s%s", base, + ret = snprintf(path, FILENAME_MAX, "%s%s", base, cgroup->controller[k]->values[j]->name); - if (ret < 0) { + dbg("setting %s to %s, error %d\n", path, + cgroup->controller[k]->values[j]->name, ret); + if (ret < 0 || ret >= FILENAME_MAX) { last_errno = errno; error = ECGOTHER; goto err; @@ -1245,9 +1248,8 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership) } if (!ignore_ownership) { - free(path); - ret = asprintf(&path, "%s/tasks", base); - if (ret < 0) { + ret = snprintf(path, FILENAME_MAX, "%s/tasks", base); + if (ret < 0 || ret >= FILENAME_MAX) { last_errno = errno; error = ECGOTHER; goto err; -- cgit