On 06/07/2011 02:34 PM, Michal Hocko wrote:
> 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 of all cgroup files according to cgroup data structure. It
> uses cg_chmod_recursive_controller and cg_chmod_path internally.
>
> 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 <mho...@suse.cz>
> ---
> include/libcgroup/groups.h | 8 ++++
> src/api.c | 80
> +++++++++++++++++++++++++++++++++-----------
> src/libcgroup-internal.h | 11 ++++++
> src/libcgroup.map | 2 +-
> src/tools/cgcreate.c | 5 ++-
> 5 files changed, 83 insertions(+), 23 deletions(-)
>
> diff --git a/include/libcgroup/groups.h b/include/libcgroup/groups.h
> index 61f18a0..3271c52 100644
> --- a/include/libcgroup/groups.h
> +++ b/include/libcgroup/groups.h
> @@ -341,6 +341,14 @@ int cgroup_get_uid_gid(struct cgroup *cgroup, uid_t
> *tasks_uid,
> gid_t *tasks_gid, uid_t *control_uid, gid_t *control_gid);
>
> /**
> + * Sets file permissions of the group's control and tasks files according
> + * to the configuration stored in the given @c cgroup structure.
> + * @param path Path to the group.
> + * @param cgroup
> + */
> +int cgroup_set_permissions(const char *path, struct cgroup *cgroup);
I got probably misunderstood, what I wanted was
cgroup_set_permissions(struct cgroup *cgroup,
mode_t control_fperm,
mode_t control_dperm,
mode_t task_fperm);
which would just set the values in the struct cgroup and
cgroup_create_cgroup() would just use them, similarly to
cgroup_set_uid_gid() in wrapper.c.
> +
> +/**
> * @}
> * @name Group parameters
> * These are functions can read or modify parameter of a group.
> diff --git a/src/api.c b/src/api.c
> index 23772f3..c3d5a9d 100644
> --- 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,16 @@ 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 +243,7 @@ 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 +270,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 +300,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 +308,23 @@ int cg_chmod_recursive(struct cgroup *cgroup, mode_t
> dir_mode,
> return final_ret;
> }
>
> +int cgroup_set_permissions(const char *path, struct cgroup *cgroup)
> +{
> + int error;
> +
> + error = cg_chmod_recursive_controller(path,
> + cgroup->control_dperm,
> + cgroup->control_dperm != NO_PERMS,
> + cgroup->control_fperm,
> + cgroup->control_fperm != NO_PERMS,
> + 1);
api.c:320:4: warning: passing argument 1 of
'cg_chmod_recursive_controller' discards 'const' qualifier from pointer
target type [enabled by default]
> +
> + if (!error && cgroup->task_fperm != NO_PERMS)
> + error = cg_chmod_path(path, cgroup->task_fperm, 1);
> +
> + return error;
> +
> +}
> static char *cgroup_basename(const char *path)
> {
> char *base;
> @@ -1475,13 +1523,8 @@ 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) {
> - error =
> cg_chmod_recursive_controller(fts_path[0],
> -
> cgroup->control_dperm,
> -
> cgroup->control_dperm != NO_PERMS,
> -
> cgroup->control_fperm,
> -
> cgroup->control_fperm != NO_PERMS);
> - }
> + if (!error)
> + error = cgroup_set_permissions(fts_path[0],
> cgroup);
> }
>
> if (error)
> @@ -1528,9 +1571,6 @@ 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);
> -
> if (error) {
> last_errno = errno;
> error = ECGOTHER;
> diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
> index e79c038..b57a887 100644
> --- a/src/libcgroup-internal.h
> +++ b/src/libcgroup-internal.h
> @@ -250,6 +250,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 fb30801..f1afaf6 100644
> --- 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 7312b32..96f65da 100644
> --- a/src/tools/cgcreate.c
> +++ b/src/tools/cgcreate.c
> @@ -311,8 +311,9 @@ int main(int argc, char *argv[])
> goto err;
> }
> if (dirm_change + filem_change > 0) {
> - ret = cg_chmod_recursive(cgroup, dir_mode, dirm_change,
> - file_mode, filem_change);
> + cgroup->control_dperm = dir_mode;
> + cgroup->control_fperm = cgroup->task_fperm = file_mode;
> + ret = cgroup_set_permissions(cgroup_list[i]->path,
> cgroup);
> if (ret) {
> fprintf(stderr, "%s: can't change permission " \
> "of cgroup %s: %s\n", argv[0],
------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
