On 05/20/2011 04:31 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.
>
> This patch adds cg_chmod_path which can be used for non recursive
> permissions setting for path names and cg_owner_mask_chmod_recursive
> owner permissions aware variant of cg_chmod_recursive.
>
> /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 | 22 +++++++++++++
> src/api.c | 73 ++++++++++++++++++++++++++++++++++---------
> src/libcgroup.map | 2 +-
> src/tools/cgcreate.c | 2 +-
> 4 files changed, 81 insertions(+), 18 deletions(-)
>
> diff --git a/include/libcgroup/groups.h b/include/libcgroup/groups.h
> index 61f18a0..ebad462 100644
> --- a/include/libcgroup/groups.h
> +++ b/include/libcgroup/groups.h
> @@ -536,6 +536,28 @@ int cg_chmod_recursive(struct cgroup *cgroup, mode_t
> dir_mode,
> int dirm_change, mode_t file_mode, int filem_change);
>
> /**
> + * Change permission of files and directories of given group and uses
> + * owner permissions as a mask for group and others permissions
> + * @param cgroup The cgroup which permissions should be changed
> + * @param dir_mode The permission mode of group directory
> + * @param dirm_change Denotes whether the directory change should be done
> + * @param file_mode The permission mode of group files
> + * @param filem_change Denotes whether the directory change should be done
> + */
> +int cg_owner_mask_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
> + int dirm_change, mode_t file_mode, int filem_change);
Now that cgroup_create_cgroup can set file/directory permissions when
creating a group, I think it's better to add new public function
cgroup_set_permissions() to set fields in struct cgroup and let
cgroup_create_cgroup() to actually set them on filesystem - just like we
do with cgroup_set_uid_gid(). I have already got bitten enough by
specific functions like cg_chmod_recursive.
> +
> +/** 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);
This function is not published in libcgroup.map and we do not have
external user of it, please put it to libcgroup-internal.h
> +
> +/**
> * @}
> * @}
> */
> diff --git a/src/api.c b/src/api.c
> index eb9f902..d2220c8 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,29 +225,22 @@ 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;
> }
>
> -
> -/*
> - * TODO: Need to decide a better place to put this function.
> - */
> -int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
> - int dirm_change, mode_t file_mode, int filem_change)
> +int __cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
> + int dirm_change, mode_t file_mode, int filem_change,
> + int owner_is_umask)
> {
> int ret = 0;
> int final_ret =0;
> @@ -250,7 +278,7 @@ int cg_chmod_recursive(struct cgroup *cgroup, 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;
> @@ -263,6 +291,19 @@ err:
> return final_ret;
> }
>
> +int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
> + int dirm_change, mode_t file_mode, int filem_change)
> +{
> + return __cg_chmod_recursive(cgroup, dir_mode, dirm_change,
> + file_mode, filem_change, 0);
> +}
> +
> +int cg_owner_mask_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
> + int dirm_change, mode_t file_mode, int filem_change)
> +{
> + return __cg_chmod_recursive(cgroup, dir_mode, dirm_change,
> + file_mode, filem_change, 1);
> +}
>
> static char *cgroup_basename(const char *path)
> {
> @@ -1457,7 +1498,7 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int
> ignore_ownership)
> error = cg_chown_recursive(fts_path,
> cgroup->control_uid, cgroup->control_gid);
> if (!error) {
> - error = cg_chmod_recursive(cgroup,
> + error = cg_owner_mask_chmod_recursive(cgroup,
> cgroup->control_dperm,
> cgroup->control_dperm !=
> NO_PERMS,
> cgroup->control_fperm,
> @@ -1510,7 +1551,7 @@ 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.map b/src/libcgroup.map
> index fb30801..4a5145a 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;
> + cg_owner_mask_chmod_recursive;
> } CGROUP_0.37;
> -
> diff --git a/src/tools/cgcreate.c b/src/tools/cgcreate.c
> index 7312b32..e510dd1 100644
> --- a/src/tools/cgcreate.c
> +++ b/src/tools/cgcreate.c
> @@ -311,7 +311,7 @@ int main(int argc, char *argv[])
> goto err;
> }
> if (dirm_change + filem_change > 0) {
> - ret = cg_chmod_recursive(cgroup, dir_mode, dirm_change,
> + ret = cg_owner_mask_chmod_recursive(cgroup, dir_mode,
> dirm_change,
> file_mode, filem_change);
> if (ret) {
> fprintf(stderr, "%s: can't change permission " \
------------------------------------------------------------------------------
vRanger cuts backup time in half-while increasing security.
With the market-leading solution for virtual backup and recovery,
you get blazing-fast, flexible, and affordable data protection.
Download your free trial now.
http://p.sf.net/sfu/quest-d2dcopy1
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
