On 05/31/2011 03:10 PM, Ivana Hutarova Varekova wrote:
> On 05/31/2011 01:50 PM, Jan Safranek wrote:
>> The function changes permissions only in the first hierarchy, but it should
>> change it in all of them.
>>
>> Reproducer:
>> 1) have cpuacct and freezer mounted separately, i.e.:
>> $ lssubsys -m
>> cpuacct /sys/fs/cgroup/cpuacct
>> freezer /sys/fs/cgroup/freezer
>>
>> 2) create a group with specific permissions:
>> $ cgcreate -f 700 -g freezer,cpuacct:/test
>>
>> Result:
>> $ ls -la /sys/fs/cgroup/freezer/test/
>> -rwx------. 1 jsafrane jsafrane 0 May 31 09:16 cgroup.clone_children
>> -rwx------. 1 jsafrane jsafrane 0 May 31 09:16 cgroup.event_control
>> (-> first controller is fine)
>>
>> $ ls -la /sys/fs/cgroup/cpuacct/test/
>> -rw-r--r--. 1 jsafrane jsafrane 0 May 31 09:16 cgroup.clone_children
>> --w--w--w-. 1 jsafrane jsafrane 0 May 31 09:16 cgroup.event_control
>> (-> second controller is wrong, it should be -rwx------)
>>
>> Changelog:
>> - fixed return code of cg_chmod_recursive_controller when fts_read fails
>>
> I have a problem with cg_build_path function which can lead to buffer
> overflow now, othervise ok.
There is one more thing to be fixed.
>> Signed-off-by: Jan Safranek<jsafr...@redhat.com>
>> ---
>>
>> src/api.c | 48 ++++++++++++++++++++++++++++++------------------
>> 1 files changed, 30 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/api.c b/src/api.c
>> index 53c76e8..1c0b872 100644
>> --- a/src/api.c
>> +++ b/src/api.c
>> @@ -211,42 +211,31 @@ int cg_chmod_file(FTS *fts, FTSENT *ent, mode_t
>> dir_mode,
>> /*
>> * TODO: Need to decide a better place to put this function.
>> */
>> -int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
>> +static int cg_chmod_recursive_controller(char *path, mode_t dir_mode,
>> int dirm_change, mode_t file_mode, int filem_change)
>> {
>> int ret = 0;
>> int final_ret =0;
>> FTS *fts;
>> char *fts_path[2];
>> - char *path = NULL;
>>
>> - fts_path[0] = (char *)malloc(FILENAME_MAX);
>> - if (!fts_path[0]) {
>> - last_errno = errno;
>> - return ECGOTHER;
>> - }
>> + fts_path[0] = path;
>> fts_path[1] = NULL;
>> - path = fts_path[0];
>> cgroup_dbg("chmod: path is %s\n", path);
>>
>> - if (!cg_build_path(cgroup->name, path,
>> - cgroup->controller[0]->name)) {
>> - final_ret = ECGFAIL;
>> - goto err;
>> - }
>> -
>> fts = fts_open(fts_path, FTS_PHYSICAL | FTS_NOCHDIR |
>> FTS_NOSTAT, NULL);
>> if (fts == NULL) {
>> last_errno = errno;
>> - final_ret = ECGOTHER;
>> - goto err;
>> + return ECGOTHER;
>> }
>> while (1) {
>> FTSENT *ent;
>> ent = fts_read(fts);
>> if (!ent) {
>> cgroup_dbg("fts_read failed\n");
>> + last_errno = errno;
>> + final_ret = ECGOTHER;
>> break;
>> }
>> ret = cg_chmod_file(fts, ent, dir_mode, dirm_change,
>> @@ -257,13 +246,36 @@ int cg_chmod_recursive(struct cgroup *cgroup, mode_t
>> dir_mode,
>> }
>> }
>> fts_close(fts);
>> + return final_ret;
>> +}
>> +
>> +int cg_chmod_recursive(struct cgroup *cgroup, mode_t dir_mode,
>> + int dirm_change, mode_t file_mode, int filem_change)
>> +{
>> + int i;
>> + char *path;
>> + int final_ret = 0;
>> + int ret;
>>
>> + path = malloc(FILENAME_MAX);
>> + if (!path)
>> + return ECGFAIL;
<- here should be used ECGOTHER with proper errno
>> + for (i = 0; i< cgroup->index; i++) {
>> + if (!cg_build_path(cgroup->name, path,
>> + cgroup->controller[i]->name)) {
> <- the size of buffer needed by cg_build_path is not limited nor checked
> thus here can happen overflow - the bug have to be fixed either on
> cg_build_path side or in this tool and allocate sufficient space (from
> my point of view on cg_build path side).
>> + final_ret = ECGFAIL;
>> + goto err;
>> + }
>> + ret = cg_chmod_recursive_controller(path, dir_mode, dirm_change,
>> + file_mode, filem_change);
>> + if (ret)
>> + final_ret = ret;
>> + }
>> err:
>> - free(fts_path[0]);
>> + free(path);
>> return final_ret;
>> }
>>
>> -
>> static char *cgroup_basename(const char *path)
>> {
>> char *base;
>>
>>
>> ------------------------------------------------------------------------------
>> Simplify data backup and recovery for your virtual environment with vRanger.
>> Installation's a snap, and flexible recovery options mean your data is safe,
>> secure and there when you need it. Data protection magic?
>> Nope - It's vRanger. Get your free trial download today.
>> http://p.sf.net/sfu/quest-sfdev2dev
>> _______________________________________________
>> Libcg-devel mailing list
>> Libcg-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/libcg-devel
>
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with vRanger.
> Installation's a snap, and flexible recovery options mean your data is safe,
> secure and there when you need it. Data protection magic?
> Nope - It's vRanger. Get your free trial download today.
> http://p.sf.net/sfu/quest-sfdev2dev
> _______________________________________________
> Libcg-devel mailing list
> Libcg-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/libcg-devel
------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
