mkdir: don't do it as setresid

First of all the code was wrong anyway.

Secondly, doing mkdir under setres{ug}id won't work because that
won't respect privilege that callers in user namespaces may have.

We check the permissions meticulously anyway, so do the mkdir as
root and then chown the new cgroup.

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>

diff --git a/cgfs.c b/cgfs.c
index 98f7c147aa91019ce41abe3f850d17a9413df9e0..bf4322b7659d0327ab300af3b5f29ddc98a61522 100644 (file)
--- a/cgfs.c
+++ b/cgfs.c
@@ -351,20 +351,20 @@ bool cgfs_set_value(const char *controller, const char *cgroup, const char *file
        return write_string(fnam, value);
 }
 
-bool cgfs_create(const char *controller, const char *cg)
+int cgfs_create(const char *controller, const char *cg)
 {
        size_t len;
        char *dirnam, *tmpc = find_mounted_controller(controller);
 
        if (!tmpc)
-               return false;
+               return -EINVAL;
        /* basedir / tmpc / cg \0 */
        len = strlen(basedir) + strlen(tmpc) + strlen(cg) + 3;
        dirnam = alloca(len);
        snprintf(dirnam, len, "%s/%s/%s", basedir,tmpc, cg);
-       if (mkdir(dirnam, 0755) < 0 && errno != EEXIST)
-               return false;
-       return true;
+       if (mkdir(dirnam, 0755) < 0)
+               return -errno;
+       return 0;
 }
 
 static bool recursive_rmdir(const char *dirname)
@@ -452,20 +452,20 @@ bool cgfs_chmod_file(const char *controller, const char *file, mode_t mode)
        return true;
 }
 
-bool cgfs_chown_file(const char *controller, const char *file, uid_t uid, gid_t gid)
+int cgfs_chown_file(const char *controller, const char *file, uid_t uid, gid_t gid)
 {
        size_t len;
        char *pathname, *tmpc = find_mounted_controller(controller);
 
        if (!tmpc)
-               return false;
+               return -EINVAL;
        /* basedir / tmpc / file \0 */
        len = strlen(basedir) + strlen(tmpc) + strlen(file) + 3;
        pathname = alloca(len);
        snprintf(pathname, len, "%s/%s/%s", basedir, tmpc, file);
        if (chown(pathname, uid, gid) < 0)
-               return false;
-       return true;
+               return -errno;
+       return 0;
 }
 
 FILE *open_pids_file(const char *controller, const char *cgroup)

diff --git a/cgfs.h b/cgfs.h
index a6f077d38b62c6311f12c9cb768ea08aadc371f4..84f1a29cb3551d8d6b9c89ba158c4ede286e9f9b 100644 (file)
--- a/cgfs.h
+++ b/cgfs.h
@@ -6,10 +6,10 @@ char *must_copy_string(const char *str);
 
 bool cgfs_set_value(const char *controller, const char *cgroup, const char *file,
                const char *value);
-bool cgfs_create(const char *controller, const char *cg);
+int cgfs_create(const char *controller, const char *cg);
 bool cgfs_remove(const char *controller, const char *cg);
 bool cgfs_chmod_file(const char *controller, const char *file, mode_t mode);
-bool cgfs_chown_file(const char *controller, const char *cg, uid_t uid, gid_t gid);
+int cgfs_chown_file(const char *controller, const char *cg, uid_t uid, gid_t gid);
 FILE *open_pids_file(const char *controller, const char *cgroup);
 bool cgfs_list_children(const char *controller, const char *cgroup, char ***list);
 bool cgfs_get_value(const char *controller, const char *cgroup, const char *file,

diff --git a/lxcfs.c b/lxcfs.c
index 342e777ea887358d09c76ea733f5c4591564b676..19c781dfbe247c83dfc8bb1094a0886c7d2fec93 100644 (file)
--- a/lxcfs.c
+++ b/lxcfs.c
@@ -1483,12 +1483,7 @@ int cg_chown(const char *path, uid_t uid, gid_t gid)
                goto out;
        }
 
-       if (!cgfs_chown_file(controller, cgroup, uid, gid)) {
-               ret = -EINVAL;
-               goto out;
-       }
-
-       ret = 0;
+       ret = cgfs_chown_file(controller, cgroup, uid, gid);
 
 out:
        free_key(k);
@@ -1599,30 +1594,12 @@ int cg_mkdir(const char *path, mode_t mode)
                goto out;
        }
 
-       if (fc->uid == 0 && fc->gid == 0) {
-               if (!cgfs_create(controller, cgroup)) {
-                       ret = -EINVAL;
-                       goto out;
-               }
-       } else {
-               if (setresuid(fc->uid, fc->gid, 0) < 0) { // bail
-                       fprintf(stderr, "ERROR - DANGER - setresuid failed!\n");
-                       exit(1);
-               }
-
-               bool bret = cgfs_create(controller, cgroup);
-
-               if (setresuid(0, 0, 0) < 0) {
-                       fprintf(stderr, "ERROR - failed to restore uids!\n");
-                       exit(1);
-               }
-               if (!bret) {
-                       ret = -EINVAL;
-                       goto out;
-               }
-       }
+       ret = cgfs_create(controller, cgroup);
+       if (ret)
+               goto out;
 
-       ret = 0;
+       if (fc->uid != 0 || fc->gid != 0)
+               ret = cgfs_chown_file(controller, cgroup, fc->uid, fc->gid);
 
 out:
        free(cgdir);