On Fri, Feb 20, 2009 at 09:37:20AM +0530, Balbir Singh wrote: > > 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.
Can you please split this patch into two (or three), there are too many things happening in it. :( > > 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; > } > Good catch! > @@ -680,7 +684,7 @@ static int cg_test_mounted_fs() > > proc_mount = fopen("/proc/mounts", "r"); > if (proc_mount == NULL) { > - return -1; > + return 0; And this one as well > } > > 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); snprintf? > if (ret < 0) { > last_errno = errno; > error = ECGOTHER; > > -- > Balbir -- regards, Dhaval ------------------------------------------------------------------------------ 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
