On Tue 17-05-11 11:55:32, Balbir Singh wrote:
> * Michal Hocko <mho...@suse.cz> [2011-05-16 14:07:05]:
[...]
> > 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.
I have a half baked patchset already - need to do some cleanup and think
how to handle files that are not to be changed. I will probably post it
at the end of the week because I am traveling now.
Anyway, I was wondering, would be such a change acceptable for the
stable release? That is the reason for this workaround in the first
place.
> Could you please mention with an example or actual output the effect
> this change brings about to permissions.
I am not sure whether you refer to this patch or the grammar
enhancement.
>
> >
> > 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
OK
>
> > + * 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
OK. What about the patch below? Or does this workaround doesn't make any
sense at all?
---
>From 5a18942312a6c0ced11c829663392d9a2ca52796 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.
Signed-off-by: Michal Hocko <mho...@suse.cz>
---
src/config.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/src/config.c b/src/config.c
index f1873ea..76f189b 100644
--- a/src/config.c
+++ b/src/config.c
@@ -480,6 +480,26 @@ 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,
+ S_IRWXU|S_IRWXG|S_IROTH|S_IXOTH, 1,
+
S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH, 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;
}
--
1.7.4.4
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
------------------------------------------------------------------------------
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
