chmod_file: Introduce intelligent file permissions setting

There is no general rule on which permissions make sense for files in
different subsystems. Nevertheless the kernel creates those files with
the maximum allowed permissions for owner so we should use its part as
an umask for group and others permissions as well.
This means that if we specify 777 for file_mode we will end up having
same permissions as owner what ever they are.

The primary intelligence is implemented in cg_chmod_path function which
takes an argument which says whether user permissions should be used
as a template.

This patch adds and exports cgroup_set_permissions which sets
permissions for control and task files into the cgroup descriptor
and cgroup_create_cgroup does the rest.

Example:
/etc/cgconfig.conf:
mount {
        cpu = /cgroup/cpuctl/;
}

group devel {
        perm {
                task {
                        uid = root;
                        gid = cgroup;
                        fperm = 770;
                }
                admin {
                        uid = root;
                        gid = cgroup;
                        dperm = 775;
                        fperm = 770;
                }
        }
        cpu {
                cpu.shares = 5120;
        }
}

cd /cgroup/cpuctl/devel/
ls -la
drwxrwxr-x 2 root cgroup 0 May 19 16:42 .
drwxr-xr-x 4 root root   0 May 19 16:14 ..
-rw-rw---- 1 root cgroup 0 May 19 16:42 cgroup.clone_children
--w--w---- 1 root cgroup 0 May 19 16:42 cgroup.event_control
-r--r----- 1 root cgroup 0 May 19 16:42 cgroup.procs
-rw-rw---- 1 root cgroup 0 May 19 16:42 cpu.rt_period_us
-rw-rw---- 1 root cgroup 0 May 19 16:42 cpu.rt_runtime_us
-rw-rw---- 1 root cgroup 0 May 19 16:42 cpu.shares
-rw-rw---- 1 root cgroup 0 May 19 16:42 notify_on_release
-rw-rw---- 1 root cgroup 0 May 19 16:42 tasks

Signed-off-by: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Jan Safranek <jsafrane@redhat.com>

diff --git a/include/libcgroup/groups.h b/include/libcgroup/groups.h
index 61f18a09fe7c98775bb4da105e4555105a668ead..86c5b76612d9d4356c2cdb1574af8cbee6467553 100644 (file)
--- a/include/libcgroup/groups.h
+++ b/include/libcgroup/groups.h
@@ -176,7 +176,8 @@ void cgroup_free_controllers(struct cgroup *cgroup);
  * Physically create a control group in kernel. The group is created in all
  * hierarchies, which cover controllers added by cgroup_add_controller().
  * All parameters set by cgroup_add_value_* functions are written.
- * The created groups has owner which was set by cgroup_set_uid_gid().
+ * The created groups has owner which was set by cgroup_set_uid_gid() and
+ * permissions set by cgroup_set_permissions.
  * @param cgroup
  * @param ignore_ownership When nozero, all errors are ignored when setting
  *     owner of the group and/or its tasks file.
@@ -340,6 +341,22 @@ int cgroup_set_uid_gid(struct cgroup *cgroup, uid_t tasks_uid, gid_t tasks_gid,
 int cgroup_get_uid_gid(struct cgroup *cgroup, uid_t *tasks_uid,
                gid_t *tasks_gid, uid_t *control_uid, gid_t *control_gid);
 
+/**
+ * Stores given file permissions of the group's control and tasks files
+ * into the @c cgroup data structure. Use NO_PERMS if permissions shouldn't
+ * be changed or a value which applicable to chmod(2). Please note that
+ * the given permissions are masked with the file owner's permissions.
+ * For example if a control file has permissions 640 and control_fperm is
+ * 471 the result will be 460.
+ * @param cgroup
+ * @param control_dperm Directory permission for the group.
+ * @param control_fperm File permission for the control files.
+ * @param task_fperm File permissions for task file.
+ */
+void cgroup_set_permissions(struct cgroup *cgroup,
+               mode_t control_dperm, mode_t control_fperm,
+               mode_t task_fperm);
+
 /**
  * @}
  * @name Group parameters

diff --git a/src/api.c b/src/api.c
index 0f308af22e524525c5f72f4de3eb5f0fa22d0b6d..66cc2b3dd82e0f2a88803d09cb37c2faee2fbb80 100644 (file)
--- a/src/api.c
+++ b/src/api.c
@@ -175,12 +175,47 @@ static int cg_chown_recursive(char **path, uid_t owner, gid_t group)
        return ret;
 }
 
+int cg_chmod_path(const char *path, mode_t mode, int owner_is_umask)
+{
+       struct stat buf;
+       mode_t mask = -1U;
+
+       if (owner_is_umask) {
+               mode_t umask, gmask, omask;
+
+               /*
+                * Use owner permissions as an umask for group and others
+                * permissions because we trust kernel to initialize owner
+                * permissions to something useful.
+                */
+               if (stat(path, &buf) == -1)
+                       goto fail;
+               umask = S_IRWXU & buf.st_mode;
+               gmask = umask >> 3;
+               omask = gmask >> 3;
+
+               mask = umask|gmask|omask;
+       }
+
+       if (chmod(path, mode & mask))
+               goto fail;
+
+       return 0;
+
+fail:
+       last_errno = errno;
+       return ECGOTHER;
+}
+
 int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t dir_mode,
-       int dirm_change, mode_t file_mode, int filem_change)
+       int dirm_change, mode_t file_mode, int filem_change,
+       int owner_is_umask)
 {
        int ret = 0;
        const char *filename = fts->fts_path;
+
        cgroup_dbg("chmod: seeing file %s\n", filename);
+
        switch (ent->fts_info) {
        case FTS_ERR:
                errno = ent->fts_errno;
@@ -190,20 +225,17 @@ int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t dir_mode,
        case FTS_DNR:
        case FTS_DP:
                if (dirm_change)
-                       ret = chmod(filename, dir_mode);
+                       ret = cg_chmod_path(filename, dir_mode, owner_is_umask);
                break;
        case FTS_F:
        case FTS_NSOK:
        case FTS_NS:
        case FTS_DEFAULT:
                if (filem_change)
-                       ret = chmod(filename, file_mode);
+                       ret = cg_chmod_path(filename, file_mode,
+                                       owner_is_umask);
                break;
        }
-       if (ret < 0) {
-               last_errno = errno;
-               ret = ECGOTHER;
-       }
        return ret;
 }
 
@@ -212,7 +244,8 @@ int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t dir_mode,
  * TODO: Need to decide a better place to put this function.
  */
 static int cg_chmod_recursive_controller(char *path, mode_t dir_mode,
-       int dirm_change, mode_t file_mode, int filem_change)
+               int dirm_change, mode_t file_mode, int filem_change,
+               int owner_is_umask)
 {
        int ret = 0;
        int final_ret =0;
@@ -239,7 +272,7 @@ static int cg_chmod_recursive_controller(char *path, mode_t dir_mode,
                        break;
                }
                ret = cg_chmod_file(fts, ent, dir_mode, dirm_change,
-                       file_mode, filem_change);
+                       file_mode, filem_change, owner_is_umask);
                if (ret) {
                        last_errno = errno;
                        final_ret = ECGOTHER;
@@ -269,7 +302,7 @@ int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
                        break;
                }
                ret = cg_chmod_recursive_controller(path, dir_mode, dirm_change,
-                               file_mode, filem_change);
+                               file_mode, filem_change, 0);
                if (ret)
                        final_ret = ret;
        }
@@ -277,6 +310,15 @@ int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
        return final_ret;
 }
 
+void cgroup_set_permissions(struct cgroup *cgroup,
+               mode_t control_dperm, mode_t control_fperm,
+               mode_t task_fperm)
+{
+       cgroup->control_dperm = control_dperm;
+       cgroup->control_fperm = control_fperm;
+       cgroup->task_fperm = task_fperm;
+}
+
 static char *cgroup_basename(const char *path)
 {
        char *base;
@@ -1475,13 +1517,13 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership)
                        cgroup_dbg("Changing ownership of %s\n", fts_path[0]);
                        error = cg_chown_recursive(fts_path,
                                cgroup->control_uid, cgroup->control_gid);
-                       if (!error) {
+                       if (!error)
                                error = cg_chmod_recursive_controller(fts_path[0],
                                                cgroup->control_dperm,
                                                cgroup->control_dperm != NO_PERMS,
                                                cgroup->control_fperm,
-                                               cgroup->control_fperm != NO_PERMS);
-                       }
+                                               cgroup->control_fperm != NO_PERMS,
+                                               1);
                }
 
                if (error)
@@ -1529,7 +1571,8 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership)
                        error = chown(path, cgroup->tasks_uid,
                                                        cgroup->tasks_gid);
                        if (!error && cgroup->task_fperm != NO_PERMS)
-                               error = chmod(path, cgroup->task_fperm);
+                               error = cg_chmod_path(path, cgroup->task_fperm,
+                                               1);
 
                        if (error) {
                                last_errno = errno;

diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
index 9845cad14f4b64cd64d0edff3e54957f372763bf..de437e19300df6e3f4f4935c1787776316a77583 100644 (file)
--- a/src/libcgroup-internal.h
+++ b/src/libcgroup-internal.h
@@ -251,6 +251,17 @@ extern int cgroup_dictionary_iterator_next(void **handle,
  */
 extern void cgroup_dictionary_iterator_end(void **handle);
 
+/**
+ * Changes permissions for given path. If owner_is_umask is specified
+ * then it uses owner permissions as a mask for group and others permissions.
+ *
+ * @param path Patch to chmod.
+ * @param mode File permissions to set.
+ * @param owner_is_umask Flag whether path owner permissions should be used
+ * as a mask for group and others permissions.
+ */
+int cg_chmod_path(const char *path, mode_t mode, int owner_is_umask);
+
 __END_DECLS
 
 #endif

diff --git a/src/libcgroup.map b/src/libcgroup.map
index fb30801de3eb890fd96c9c2e708c9eb17a3dc706..f1afaf670d5f144516335d44d78372cfb0a1e052 100644 (file)
--- a/src/libcgroup.map
+++ b/src/libcgroup.map
@@ -101,5 +101,5 @@ CGROUP_0.38 {
        cgroup_get_subsys_mount_point_begin;
        cgroup_get_subsys_mount_point_next;
        cgroup_get_subsys_mount_point_end;
+       cgroup_set_permissions;
 } CGROUP_0.37;
-

diff --git a/src/tools/cgcreate.c b/src/tools/cgcreate.c
index 7312b32682213b8aa557efaca7ef3e8da56d7195..d470b9473c7740874ad67b9f572a8eea54feb647 100644 (file)
--- a/src/tools/cgcreate.c
+++ b/src/tools/cgcreate.c
@@ -302,6 +302,9 @@ int main(int argc, char *argv[])
                }
 
                /* all variables set so create cgroup */
+               if (dirm_change + filem_change > 0)
+                       cgroup_set_permissions(cgroup, dir_mode, file_mode,
+                                       file_mode);
                ret = cgroup_create_cgroup(cgroup, 0);
                if (ret) {
                        fprintf(stderr, "%s: "
@@ -310,17 +313,6 @@ int main(int argc, char *argv[])
                        cgroup_free(&cgroup);
                        goto err;
                }
-               if (dirm_change + filem_change > 0) {
-                       ret = cg_chmod_recursive(cgroup, dir_mode, dirm_change,
-                               file_mode, filem_change);
-                       if (ret) {
-                               fprintf(stderr, "%s: can't change permission " \
-                                       "of cgroup %s: %s\n", argv[0],
-                                       cgroup->name, cgroup_strerror(ret));
-                               cgroup_free(&cgroup);
-                               goto err;
-                       }
-               }
                cgroup_free(&cgroup);
        }
 err: