On 05/16/2011 02:07 PM, Michal Hocko wrote:
> 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).
Why is gid and uid compared together? These two number have nothing in
common!
> The proper fix for this would be to enable file and directory permissions
> setting in configuration file but let's do it later.
Well, fa3d180b is there for a reason, adding 664 everywhere will break
e.g. cgsnapshot. Some parameters are meant only for writing, typically
devices.allow/deny. If you want to fix it properly, then copy user's
permissions to group, i.e. from
--w-------. 1 root root 0 2010-11-02 08:04 devices.allow
to
--w--w----. 1 root root 0 2010-11-02 08:04 devices.allow
Jan
>
> 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
> + * 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);
> + if (error) {
> + fprintf(stderr, "can't change permission "\
> + "of cgroup %s: %s\n",
> cgroup->name,
> + cgroup_strerror(error));
> + cgroup_free(&cgroup);
> + continue;
> + }
> + }
> }
> return error;
> }
------------------------------------------------------------------------------
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
