* Michal Hocko <mho...@suse.cz> [2011-05-16 14:07:05]:
> The patch has been waiting in for the administrator approval for some
> days so I have subscribed. Here is the reposted patch.
>
> On Fri 13-05-11 14:57:21, Michal Hocko wrote:
> > On Fri 13-05-11 10:52:50, Michal Hocko wrote:
> > > Hi,
> > > I consider the patch below to be a bug fix (or better said usability
> > > fix). As the changelog says, the proper fix would be to allow
> > > permissions setting in the per configuration section (I have half baked
> > > patch already but I need to clean it up). I still think that this should
> > > be pushed to the stable releases where bigger - permission in config -
> > > would be a problem.
> >
> > I am sorry, the previous patch misses cgroup_free bad type fix that
> > didn't get into the commit. Here we go:
> ---
> >From 2a9068fde5db17f42d0fe8a16bb5d036a9c100c2 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mho...@suse.cz>
> Date: Thu, 12 May 2011 21:43:22 +0200
> Subject: [PATCH] cgconfigparser: Change permissions if tasks or admin gid is
> different from uid
>
> There is no possibility to set permissions for files and directories in
> the configuration file so cgconfigparser doesn't care about file
> permissions at all.
> This is a bit unfortunate because if user sets gid for either tasks or
> admin files he expects that a group member will be allowed to modify
> those files. This is especially true for tasks file because gid setting
> basically says who can attach to that group.
> We used to set file permissions but this has been changed by fa3d180b
> which removed permissions setting from control_group_create
> code path.
> The patch checks whether uid differs from gid either for task or admin
> and automatically changes the file and directory permissions to be user
> and group read&write (and executable for directories).
> The proper fix for this would be to enable file and directory permissions
> setting in configuration file but let's do it later.
Yes, that is the path I prefer. Enhance the grammar to allow it. Could
you please mention with an example or actual output the effect this
change brings about to permissions.
>
> Signed-off-by: Michal Hocko <mho...@suse.cz>
> ---
> src/config.c | 17 +++++++++++++++++
> 1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/src/config.c b/src/config.c
> index f1873ea..2735b9f 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -480,6 +480,23 @@ static int cgroup_config_create_groups(void)
> error);
> if (error)
> return error;
> +
> + /* Change permissions if we decide to to have a different
Comment style is broken
> + * group either for tasks or admin files.
> + * It could be done separately for admin and tasks files but
> + * cg_chmod_recursive cannot exclude files.
> + */
> + if (cgroup->control_uid != cgroup->control_gid ||
> + cgroup->tasks_uid != cgroup->tasks_gid) {
> + error = cg_chmod_recursive(cgroup, 00775, 1, 00664, 1);
Please don't hard code these values, use S_I* from fcntl.h or use a
well defined macro that builds the permissions we need and use it
here, please
> + if (error) {
> + fprintf(stderr, "can't change permission "\
> + "of cgroup %s: %s\n",
> cgroup->name,
> + cgroup_strerror(error));
> + cgroup_free(&cgroup);
> + continue;
> + }
> + }
> }
> return error;
--
Three Cheers,
Balbir
------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
