libcgorup: Fix a chown security issue

From: Balbir Singh <balbir@linux.vnet.ibm.com>

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 <balbir@linux.vnet.ibm.com>
Signed-off-by: Dhaval Giani <dhaval@linux.vnet.ibm.com>



git-svn-id: https://libcg.svn.sourceforge.net/svnroot/libcg/trunk@335 4f4bb910-9a46-0410-90c8-c897d4f1cd53

1 files 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;