OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/4] Devices accessibility control group (v3, release candidate)
items: [PATCH 1/4] Some changes in the kobject mapper
items: [PATCH 2/4] The character devices layer changes
items: [PATCH 3/4] The block devices layer changes
items: [PATCH 4/4] The control group itself
items: Re: [PATCH 0/4] Devices accessibility control group (v3, release candidate)
items: Re: [PATCH 4/4] The control group itself

[PATCH 0/4] Devices accessibility control group (v3, release candidate)
type: item
description: Changes from v2: * Fixed problems pointed out by Sukadev with permissions revoke. Now we have to perform kobject re-lookup on each char device open, just like for block ones, so I think this is OK. The /proc/devices tune is still in TODO list, as I have problems with getting majors _in_a_simple_manner_ from a map, that contains a mix of major/minor pairs in arbitrary order. The second version is here: http://openvz.org/pipermail/devel/2008-January/010160.html Changes from v1: * Added the block devices support :) It turned out to be a bit simpler than the char one (or I missed something significant); * Now we can enable/disable not just individual devices, but the whole major with all its minors (see the TODO list beyond as well); * Added the ability to restrict the read/write permissions to devices, not just visible/invisible state. The first version was here: http://openvz.org/pipermail/devel/2007-September/007647.html I still don't pay much attention to split this set well, so this will most likely won't work with git-bisect, but I think this is OK for now. I will sure split it better when I send it to Andrew. The set is prepared against the 2.6.24-rc8-mm1. To play with it - run a standard procedure: # mount -t container none /cont/devs -o devices # mkdir /cont/devs/0 # echo -n $$ > /cont/devs/0/tasks and tune device permissions. Thanks, Pavel _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2008-02-07T12:56:18-00:00

[PATCH 1/4] Some changes in the kobject mapper
type: item
description: The main thing that I want from the kobj mapper is to add the mode_t on the struct kobj_map that reflects with permissions are associated with this particular map. This mode is to be returned via the kobj_lookup. I use the FMODE_XXX flags to handle the permissions bits, as I will compare these ones to the file->f_mode later. By default all bits are set (for the initial container). The additional things I need are kobj_remap() to change that permission and kobj_iterate() to walk the map. The kobj_map_fini() is the roll-back for the kobj_map_init(). Signed-off-by: Pavel Emelyanov <xemul@openvz.org> --- diff --git a/drivers/base/map.c b/drivers/base/map.c index e87017f..1aa2b58 100644 --- a/drivers/base/map.c +++ b/drivers/base/map.c @@ -15,11 +15,13 @@ #include <linux/kdev_t.h> #include <linux/kobject.h> #include <linux/kobj_map.h> +#include <linux/fs.h> struct kobj_map { struct probe { struct probe *next; dev_t dev; + mode_t mode; unsigned long range; struct module *owner; kobj_probe_t *get; @@ -29,9 +31,9 @@ struct kobj_map { struct mutex *lock; }; -int kobj_map(struct kobj_map *domain, dev_t dev, unsigned long range, - struct module *module, kobj_probe_t *probe, - int (*lock)(dev_t, void *), void *data) +static int __kobj_map(struct kobj_map *domain, dev_t dev, mode_t mode, + unsigned long range, struct module *module, + kobj_probe_t *probe, int (*lock)(dev_t, void *), void *data) { unsigned n = MAJOR(dev + range - 1) - MAJOR(dev) + 1; unsigned index = MAJOR(dev); @@ -53,8 +55,10 @@ int kobj_map(struct kobj_map *domain, dev_t dev, unsigned long range, p->dev = dev; p->range = range; p->data = data; + /* we allow these ones always by default */ + p->mode = mode | FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE; } - mutex_lock(domain->lock); + for (i = 0, p -= n; i < n; i++, p++, index++) { struct probe **s = &domain->probes[index % 255]; while (*s && (*s)->range < range) @@ -62,10 +66,57 @@ int kobj_map(struct kobj_map *domain, dev_t dev, unsigned long range, p->next = *s;…
creator: Pavel Emelianov
date: 2008-02-07T12:57:23-00:00

[PATCH 2/4] The character devices layer changes
type: item
description: These changes include the API for the control group to map/remap/unmap the devices with their permissions and one important thing. The fact is that the struct cdev is cached in the inode for faster access, so once we looked one up we go through the fast path and omit the kobj_lookup() call. This is no longer good when we restrict the access to cdevs. Another problem is that different char devices may use one struct cdev object, so having an access to one of them grants us access to another, so we have to re-lookup the kobj map each open. Signed-off-by: Pavel Emelyanov <xemul@openvz.org> --- diff --git a/fs/char_dev.c b/fs/char_dev.c index 2c7a8b5..ddacab7 100644 --- a/fs/char_dev.c +++ b/fs/char_dev.c @@ -22,6 +22,8 @@ #include <linux/mutex.h> #include <linux/backing-dev.h> +#include <linux/devscontrol.h> + #ifdef CONFIG_KMOD #include <linux/kmod.h> #endif @@ -362,17 +364,31 @@ int chrdev_open(struct inode * inode, struct file * filp) struct cdev *p; struct cdev *new = NULL; int ret = 0; + struct kobj_map *map; + + map = task_cdev_map(current); + if (map == NULL) + map = cdev_map; spin_lock(&cdev_lock); p = inode->i_cdev; - if (!p) { + if (!p || map != cdev_map) { struct kobject *kobj; int idx; + mode_t mode; + spin_unlock(&cdev_lock); - kobj = kobj_lookup(cdev_map, inode->i_rdev, &idx); + kobj = kobj_lookup(map, inode->i_rdev, &mode, &idx); if (!kobj) return -ENXIO; new = container_of(kobj, struct cdev, kobj); + BUG_ON(p != NULL && p != new); + + if ((filp->f_mode & mode) != filp->f_mode) { + cdev_put(new); + return -EACCES; + } + spin_lock(&cdev_lock); p = inode->i_cdev; if (!p) { @@ -461,6 +477,53 @@ int cdev_add(struct cdev *p, dev_t dev, unsigned count) return kobj_map(cdev_map, dev, count, NULL, exact_match, exact_lock, p); } +#ifdef CONFIG_CGROUP_DEVS +int cdev_add_to_map(struct kobj_map *map, dev_t dev, int all, mode_t mode) +{ + int tmp; + struct kobject *k; + struct cdev *c; + + k = kobj_lookup(cdev_map, dev, NULL, &tmp); + if (k == NULL) + return -ENODEV;…
creator: Pavel Emelianov
date: 2008-02-07T12:59:04-00:00

[PATCH 3/4] The block devices layer changes
type: item
description: They are the same as for the character layer, but the good news is that there are no caching in this case. So this patch is smaller and easier to understand as compared to the previous one. Signed-off-by: Pavel Emelyanov <xemul@openvz.org> --- diff --git a/block/genhd.c b/block/genhd.c index 5e4ab4b..6f9ef48 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -8,6 +8,7 @@ #include <linux/kdev_t.h> #include <linux/kernel.h> #include <linux/blkdev.h> +#include <linux/devscontrol.h> #include <linux/init.h> #include <linux/spinlock.h> #include <linux/seq_file.h> @@ -195,6 +196,57 @@ void unlink_gendisk(struct gendisk *disk) disk->minors); } +#ifdef CONFIG_CGROUP_DEVS +int bdev_add_to_map(struct kobj_map *map, dev_t dev, int all, mode_t mode) +{ + int tmp; + struct kobject *kobj; + struct device *d; + struct gendisk *disk; + + kobj = kobj_lookup(bdev_map, dev, NULL, &tmp); + if (kobj == NULL) + return -ENODEV; + + d = kobj_to_dev(kobj); + disk = dev_to_disk(d); + tmp = kobj_remap(map, dev, mode, all ? MINORBITS : 1, NULL, + exact_match, exact_lock, disk); + if (tmp < 0) { + put_disk(disk); + return tmp; + } + + return 0; +} + +int bdev_del_from_map(struct kobj_map *map, dev_t dev, int all) +{ + int tmp; + struct kobject *kobj; + struct device *d; + struct gendisk *disk; + + kobj = kobj_lookup(bdev_map, dev, NULL, &tmp); + if (kobj == NULL) + return -ENODEV; + + d = kobj_to_dev(kobj); + disk = dev_to_disk(d); + kobj_unmap(map, dev, all ? MINORBITS : 1); + + put_disk(disk); + put_disk(disk); + return 0; +} + +void bdev_iterate_map(struct kobj_map *map, + int (*fn)(dev_t, int, mode_t, void *), void *x) +{ + kobj_map_iterate(map, fn, x); +} +#endif + /** * get_gendisk - get partitioning information for a given device * @dev: device to get partitioning information for @@ -202,10 +254,18 @@ void unlink_gendisk(struct gendisk *disk) * This function gets the structure containing partitioning * information for the given device @dev. */ -struct gendisk *get_gendisk(dev_t devt, int *…
creator: Pavel Emelianov
date: 2008-02-07T12:59:48-00:00

[PATCH 4/4] The control group itself
type: item
description: Each new group will have its own maps for char and block layers. The devices access list is tuned via the devices.permissions file. One may read from the file to get the configured state. The top container isn't initialized, so that the char and block layers will use the global maps to lookup their devices. I did that not to export the static maps to the outer world. Good news is that this patch now contains more comments and Documentation file :) Signed-off-by: Pavel Emelyanov <xemul@openvz.org> --- diff --git a/Documentation/controllers/devices.txt b/Documentation/controllers/devices.txt new file mode 100644 index 0000000..dbd0c7a --- /dev/null +++ b/Documentation/controllers/devices.txt @@ -0,0 +1,61 @@ + + Devices visibility controller + +This controller allows to tune the devices accessibility by tasks, +i.e. grant full access for /dev/null, /dev/zero etc, grant read-only +access to IDE devices and completely hide SCSI disks. + +Tasks still can call mknod to create device files, regardless of +whether the particular device is visible or accessible, but they +may not be able to open it later. + +This one hides under CONFIG_CGROUP_DEVS option. + + +Configuring + +The controller provides a single file to configure itself -- the +devices.permissions one. To change the accessibility level for some +device write the following string into it: + +[cb] <major>:(<minor>|*) [r-][w-] + ^ ^ ^ + | | | + | | +--- access rights (1) + | | + | +-- device major and minor numbers (2) + | + +-- device type (character / block) + +1) The access rights set to '--' remove the device from the group's +access list, so that it will not even be shown in this file later. + +2) Setting the minor to '*' grants access to all the minors for +particular major. + +When reading from it, one may see something like + + c 1:5 rw + b 8:* r- + +Security issues, concerning who may grant access to what are governed +at the cgroup infrastructure level. + + +Examples: + +1. Grand full access to /dev/null +…
creator: Pavel Emelianov
date: 2008-02-07T13:01:47-00:00

Re: [PATCH 0/4] Devices accessibility control group (v3, release candidate)
type: item
description: Quoting Pavel Emelyanov (xemul@openvz.org): > Changes from v2: > * Fixed problems pointed out by Sukadev with permissions > revoke. Now we have to perform kobject re-lookup on > each char device open, just like for block ones, so I > think this is OK. > > The /proc/devices tune is still in TODO list, as I have > problems with getting majors _in_a_simple_manner_ from a > map, that contains a mix of major/minor pairs in > arbitrary order. Thanks for posting, Pavel. I'm sorry, I'm *trying* to review these, but my eyes are glazing over. (Not because of your patches I'll start by just testing it a bit either this afternoon or monday. thanks, -serge > The second version is here: > http://openvz.org/pipermail/devel/2008-January/010160.html > Changes from v1: > > * Added the block devices support :) It turned out to > be a bit simpler than the char one (or I missed > something significant); > * Now we can enable/disable not just individual devices, > but the whole major with all its minors (see the TODO > list beyond as well); > * Added the ability to restrict the read/write permissions > to devices, not just visible/invisible state. > > The first version was here: > http://openvz.org/pipermail/devel/2007-September/007647.html > > I still don't pay much attention to split this set well, so > this will most likely won't work with git-bisect, but I think > this is OK for now. I will sure split it better when I send > it to Andrew. > > The set is prepared against the 2.6.24-rc8-mm1. > > To play with it - run a standard procedure: > > # mount -t container none /cont/devs -o devices > # mkdir /cont/devs/0 > # echo -n $$ > /cont/devs/0/tasks > > and tune device permissions. > > Thanks, > Pavel _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2008-02-08T16:12:17-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Quoting Pavel Emelyanov (xemul@openvz.org): > Each new group will have its own maps for char and block > layers. The devices access list is tuned via the > devices.permissions file. One may read from the file to get > the configured state. > > The top container isn't initialized, so that the char > and block layers will use the global maps to lookup > their devices. I did that not to export the static maps > to the outer world. > > Good news is that this patch now contains more comments > and Documentation file :) Seems to work as advertised :) I can't reproduce Suka's null/zero bug. You're relying fully on uid-0 to stop writes into the devices.permissions files. Would you mind adding a check for CAP_SYS_ADMIN (or CAP_NS_OVERRIDE+CAP_MKNOD)? Or were you really counting on using the filesystem visibility cgroup to stop a container from making changes to its device access whitelist? thanks, -serge > Signed-off-by: Pavel Emelyanov <xemul@openvz.org> > > --- > > diff --git a/Documentation/controllers/devices.txt b/Documentation/controllers/devices.txt > new file mode 100644 > index 0000000..dbd0c7a > --- /dev/null > +++ b/Documentation/controllers/devices.txt > @@ -0,0 +1,61 @@ > + > + Devices visibility controller > + > +This controller allows to tune the devices accessibility by tasks, > +i.e. grant full access for /dev/null, /dev/zero etc, grant read-only > +access to IDE devices and completely hide SCSI disks. > + > +Tasks still can call mknod to create device files, regardless of > +whether the particular device is visible or accessible, but they > +may not be able to open it later. > + > +This one hides under CONFIG_CGROUP_DEVS option. > + > + > +Configuring > + > +The controller provides a single file to configure itself -- the > +devices.permissions one. To change the accessibility level for some > +device write the following string into it: > + > +[cb] <major>:(<minor>|*) [r-][w-] > + ^ ^ ^ > + | | | > + | | +--- access rights (1) > + | | > + | +-- device major…
creator: serue
date: 2008-02-11T17:38:30-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: This patchset does fix the problem I was having before with null and zero devices. Overall, it looks like pretty good. I am still reviewing the patches. Just some nits I came across: Pavel Emelianov [xemul@openvz.org] wrote: | Each new group will have its own maps for char and block | layers. The devices access list is tuned via the | devices.permissions file. One may read from the file to get | the configured state. | | The top container isn't initialized, so that the char | and block layers will use the global maps to lookup | their devices. I did that not to export the static maps | to the outer world. | | Good news is that this patch now contains more comments | and Documentation file :) | | Signed-off-by: Pavel Emelyanov <xemul@openvz.org> | | --- | | diff --git a/Documentation/controllers/devices.txt b/Documentation/controllers/devices.txt | new file mode 100644 | index 0000000..dbd0c7a | --- /dev/null | +++ b/Documentation/controllers/devices.txt | @@ -0,0 +1,61 @@ | + | + Devices visibility controller | + | +This controller allows to tune the devices accessibility by tasks, | +i.e. grant full access for /dev/null, /dev/zero etc, grant read-only | +access to IDE devices and completely hide SCSI disks. | + | +Tasks still can call mknod to create device files, regardless of | +whether the particular device is visible or accessible, but they | +may not be able to open it later. | + | +This one hides under CONFIG_CGROUP_DEVS option. | + | + | +Configuring | + | +The controller provides a single file to configure itself -- the | +devices.permissions one. To change the accessibility level for some | +device write the following string into it: | + | +[cb] <major>:(<minor>|*) [r-][w-] | + ^ ^ ^ | + | | | | + | | +--- access rights (1) | + | | | + | +-- device major and minor numbers (2) | + | | + +-- device type (character / block) | + | +1) The access rights set to '--' remove the device from the group's | +access list, so that it will not even be shown in this file…
creator: Sukadev Bhattiprolu
date: 2008-02-12T07:42:17-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: sukadev@us.ibm.com wrote: > This patchset does fix the problem I was having before with null and > zero devices. Overall, it looks like pretty good. > > I am still reviewing the patches. Just some nits I came across: > > > Pavel Emelianov [xemul@openvz.org] wrote: > | Each new group will have its own maps for char and block > | layers. The devices access list is tuned via the > | devices.permissions file. One may read from the file to get > | the configured state. > | > | The top container isn't initialized, so that the char > | and block layers will use the global maps to lookup > | their devices. I did that not to export the static maps > | to the outer world. > | > | Good news is that this patch now contains more comments > | and Documentation file :) > | > | Signed-off-by: Pavel Emelyanov <xemul@openvz.org> > | > | --- > | > | diff --git a/Documentation/controllers/devices.txt b/Documentation/controllers/devices.txt > | new file mode 100644 > | index 0000000..dbd0c7a > | --- /dev/null > | +++ b/Documentation/controllers/devices.txt > | @@ -0,0 +1,61 @@ > | + > | + Devices visibility controller > | + > | +This controller allows to tune the devices accessibility by tasks, > | +i.e. grant full access for /dev/null, /dev/zero etc, grant read-only > | +access to IDE devices and completely hide SCSI disks. > | + > | +Tasks still can call mknod to create device files, regardless of > | +whether the particular device is visible or accessible, but they > | +may not be able to open it later. > | + > | +This one hides under CONFIG_CGROUP_DEVS option. > | + > | + > | +Configuring > | + > | +The controller provides a single file to configure itself -- the > | +devices.permissions one. To change the accessibility level for some > | +device write the following string into it: > | + > | +[cb] <major>:(<minor>|*) [r-][w-] > | + ^ ^ ^ > | + | | | > | + | | +--- access rights (1) > | + | | > | + | +-- device major and minor numbers (2) > | + | > | + +-- device type (character / bl…
creator: Pavel Emelianov
date: 2008-02-12T07:51:24-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Serge E. Hallyn wrote: > Quoting Pavel Emelyanov (xemul@openvz.org): >> Each new group will have its own maps for char and block >> layers. The devices access list is tuned via the >> devices.permissions file. One may read from the file to get >> the configured state. >> >> The top container isn't initialized, so that the char >> and block layers will use the global maps to lookup >> their devices. I did that not to export the static maps >> to the outer world. >> >> Good news is that this patch now contains more comments >> and Documentation file :) > > Seems to work as advertised :) I can't reproduce Suka's null/zero > bug. > > You're relying fully on uid-0 to stop writes into the > devices.permissions files. Would you mind adding a check for > CAP_SYS_ADMIN (or CAP_NS_OVERRIDE+CAP_MKNOD)? Or were you really > counting on using the filesystem visibility cgroup to stop a container > from making changes to its device access whitelist? Yup. I strongly believe that a controller itself should not bring any security policy of its own, but the infrastructure should take care of this. However, I'm open to change my mind if I see good explanation of why it is wrong. > thanks, > -serge > >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org> >> >> --- >> >> diff --git a/Documentation/controllers/devices.txt b/Documentation/controllers/devices.txt >> new file mode 100644 >> index 0000000..dbd0c7a >> --- /dev/null >> +++ b/Documentation/controllers/devices.txt >> @@ -0,0 +1,61 @@ >> + >> + Devices visibility controller >> + >> +This controller allows to tune the devices accessibility by tasks, >> +i.e. grant full access for /dev/null, /dev/zero etc, grant read-only >> +access to IDE devices and completely hide SCSI disks. >> + >> +Tasks still can call mknod to create device files, regardless of >> +whether the particular device is visible or accessible, but they >> +may not be able to open it later. >> + >> +This one hides under CONFIG_CGROUP_DEVS option. >> + >> + >> +Configur…
creator: Pavel Emelianov
date: 2008-02-12T10:28:02-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Quoting Pavel Emelyanov (xemul@openvz.org): > Serge E. Hallyn wrote: > > Quoting Pavel Emelyanov (xemul@openvz.org): > >> Each new group will have its own maps for char and block > >> layers. The devices access list is tuned via the > >> devices.permissions file. One may read from the file to get > >> the configured state. > >> > >> The top container isn't initialized, so that the char > >> and block layers will use the global maps to lookup > >> their devices. I did that not to export the static maps > >> to the outer world. > >> > >> Good news is that this patch now contains more comments > >> and Documentation file :) > > > > Seems to work as advertised :) I can't reproduce Suka's null/zero > > bug. > > > > You're relying fully on uid-0 to stop writes into the > > devices.permissions files. Would you mind adding a check for > > CAP_SYS_ADMIN (or CAP_NS_OVERRIDE+CAP_MKNOD)? Or were you really > > counting on using the filesystem visibility cgroup to stop a container > > from making changes to its device access whitelist? > > Yup. I strongly believe that a controller itself should not bring > any security policy of its own, but the infrastructure should > take care of this. That would be ok if the controller gave the infrastructure some way of knowing what sort of thing the controller does. I.e. I wouldn't mind having the cgroup infrastructe check for capabilities, but I suspect some cgroups will really be best represented by different capabilities. Paul (actually both Menage and Jackson :) do you have an opinion on this? Are there sites which eg do 'chown -R some_user_id /cgroup/cpusets/' to have some non-root user be able to dole out cpusets? Is there any way it would be ok to have cgroup_file_write() check for CAP_SYS_ADMIN? > However, I'm open to change my mind if I see good explanation of > why it is wrong. Well the thing is that it currently leaves no way to keep root in another namespace from manipulating it. I don't think we can even use SELinux unless we'r…
creator: serue
date: 2008-02-12T17:21:34-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: On Feb 12, 2008 9:21 AM, Serge E. Hallyn <serue@us.ibm.com> wrote: > > Paul (actually both Menage and Jackson :) do you have an opinion on > this? Are there sites which eg do 'chown -R some_user_id /cgroup/cpusets/' > to have some non-root user be able to dole out cpusets? We (Google) currently only do that for the tasks file, to allow users to move their processes between cpusets/groups that they own. > Is there any > way it would be ok to have cgroup_file_write() check for CAP_SYS_ADMIN? Sure, we could have flags in the subsys object for this kind of thing that let particular subsystems request this. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-02-13T02:17:14-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Serge wrote: > Paul (actually both Menage and Jackson :) do you have an opinion on > this? Are there sites which eg do 'chown -R some_user_id /cgroup/cpusets/' > to have some non-root user be able to dole out cpusets? Is there any > way it would be ok to have cgroup_file_write() check for CAP_SYS_ADMIN? I don't know what my users actually do here ... I'm a couple layers removed from that reality. But certainly I've recommended that they sometimes do things like having the batch scheduler chown the files of each jobs cpuset to the uid of the user running that job, so that the job can manipulate its own cpuset allocate resources in finer detail. One of the more elaborate ways of doing this nests a pair of cpusets, with the parent owned by the batch scheduler confining the child owned by the individual job. The job can actually do things like write its own cpus and mems files, but is confined by the parent cpuset to only specify cpus and mems assigned to that job. As to how this affects your question ... I'm not sure. However I suspect that an added requirement for CAP_SYS_ADMIN would cause breakage and not be a good idea. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.940.382.4214 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Jackson
date: 2008-02-13T02:32:15-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Paul M wrote: > > Is there any > > way it would be ok to have cgroup_file_write() check for CAP_SYS_ADMIN? > > Sure, we could have flags in the subsys object for this kind of thing > that let particular subsystems request this. That sounds like it would work fine ... the cpuset subsystem would just not request this. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.940.382.4214 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Jackson
date: 2008-02-13T02:42:15-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Quoting Paul Jackson (pj@sgi.com): > Paul M wrote: > > > Is there any > > > way it would be ok to have cgroup_file_write() check for CAP_SYS_ADMIN? > > > > Sure, we could have flags in the subsys object for this kind of thing > > that let particular subsystems request this. > > That sounds like it would work fine ... the cpuset subsystem would > just not request this. Cool I'll keep it in mind then - though really right now it seems like it just amounts to slightly more complicated approach than just having the subsystems do the check in _write... thanks, -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2008-02-14T17:17:18-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Quoting Paul Jackson (pj@sgi.com): > Serge wrote: > > Paul (actually both Menage and Jackson :) do you have an opinion on > > this? Are there sites which eg do 'chown -R some_user_id /cgroup/cpusets/' > > to have some non-root user be able to dole out cpusets? Is there any > > way it would be ok to have cgroup_file_write() check for CAP_SYS_ADMIN? > > I don't know what my users actually do here ... I'm a couple layers > removed from that reality. But certainly I've recommended that they > sometimes do things like having the batch scheduler chown the files > of each jobs cpuset to the uid of the user running that job, so that > the job can manipulate its own cpuset allocate resources in finer > detail. > > One of the more elaborate ways of doing this nests a pair of cpusets, > with the parent owned by the batch scheduler confining the child > owned by the individual job. The job can actually do things like > write its own cpus and mems files, but is confined by the parent > cpuset to only specify cpus and mems assigned to that job. > > As to how this affects your question ... I'm not sure. However I > suspect that an added requirement for CAP_SYS_ADMIN would cause > breakage and not be a good idea. Yeah, I guess a more general mechanism to couple a user namespace's connection to a mount is the right way to go. If we can just specify that root in this namespace is not root in that namespace (or any other userid we've chowned the files to), we've got what we need. thanks, -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2008-02-14T17:18:57-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: On Thu, Feb 7, 2008 at 5:01 AM, Pavel Emelyanov <xemul@openvz.org> wrote: > + > +[cb] <major>:(<minor>|*) [r-][w-] > + ^ ^ ^ > + | | | > + | | +--- access rights (1) > + | | > + | +-- device major and minor numbers (2) > + | > + +-- device type (character / block) > ... > +When reading from it, one may see something like > + > + c 1:5 rw > + b 8:* r- > + In the interest of avoiding proliferating cgroup control file formats, I'm wondering if we can abstract out the general form of the data being presented here and maybe simplify it in such a way that we can hopefully reuse the format for other control files in the future? For example, one way to represent this would be a map from device strings such c:1:5 to permission strings such as rw. Or maybe numerical device ids to numerical permission values. The alternative might be to accept that there are two kinds of control files - those which are likely to be programmatically read (e.g. resource usage values), and those that are likely to be programmatically written but only actually read by humans for debugging purposes (like this one) and make it clear up-front when a control file is added which type they're considered to be. We could then ignore the API consistency requirements for the debugging-readable files. On a separate note, can you document the recommended way to completely overwrite the set of device permissions for a cgroup? Does this involves writing a "--" permission for every device that you don't want in the cgroup? Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-02-21T20:47:07-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Paul Menage wrote: > On Thu, Feb 7, 2008 at 5:01 AM, Pavel Emelyanov <xemul@openvz.org> wrote: >> + >> +[cb] <major>:(<minor>|*) [r-][w-] >> + ^ ^ ^ >> + | | | >> + | | +--- access rights (1) >> + | | >> + | +-- device major and minor numbers (2) >> + | >> + +-- device type (character / block) >> ... >> +When reading from it, one may see something like >> + >> + c 1:5 rw >> + b 8:* r- >> + > > In the interest of avoiding proliferating cgroup control file formats, > I'm wondering if we can abstract out the general form of the data > being presented here and maybe simplify it in such a way that we can > hopefully reuse the format for other control files in the future? > > For example, one way to represent this would be a map from device > strings such c:1:5 to permission strings such as rw. Or maybe > numerical device ids to numerical permission values. You mean smth like <some_device_id><space><some_permissions_string>? Well, I don't mind, but AFAIK the <major>:<minor> form is very common for specifying the device. So I agree with the 'c:1:5 rw' form. > The alternative might be to accept that there are two kinds of control > files - those which are likely to be programmatically read (e.g. > resource usage values), and those that are likely to be > programmatically written but only actually read by humans for > debugging purposes (like this one) and make it clear up-front when a > control file is added which type they're considered to be. We could > then ignore the API consistency requirements for the > debugging-readable files. Hmm, you mean make them a binary files? I thought that filesystem-based API should be human readable and writable as much as possible... > On a separate note, can you document the recommended way to completely > overwrite the set of device permissions for a cgroup? Does this There's not way to flush all the permissions in this implementation, but I though about one. Maybe 'echo 0 > devices.permissions' would be good? > involves writing a "--" …
creator: Pavel Emelianov
date: 2008-02-22T08:12:45-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: On Fri, Feb 22, 2008 at 12:12 AM, Pavel Emelyanov <xemul@openvz.org> wrote: > > Hmm, you mean make them a binary files? No, not by default. But I'm working on a plan to have an optional binary API to cgroups, that would allow multiple control files to be read in a binary format with a single system call. The existing API would still be available as well, of course. The idea would be that monitoring programs that frequently read lots of values from a single cgroup (or even multiple cgroups) would be able to do so more cheaply than by making multiple different reads on different files. In order for this to work, CGroups needs to know the data type of a given control file - so this would only be available for the control files that use typed cgroup output methods rather than the raw file output interface. > I thought that filesystem-based > API should be human readable and writable as much as possible... > Yes, but even without a binary API it makes sense for values that are likely to be parsed by programs be in a consistent format. But after thinking more about this, I think that the devices permission control file output doesn't really fall under this category - from a programmatic point of view, I suspect it's write-only, and only humans will be reading the output, for debugging. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-02-23T23:12:59-00:00

Re: [PATCH 4/4] The control group itself
type: item
description: Paul Menage wrote: > On Fri, Feb 22, 2008 at 12:12 AM, Pavel Emelyanov <xemul@openvz.org> wrote: >> Hmm, you mean make them a binary files? > > No, not by default. But I'm working on a plan to have an optional > binary API to cgroups, that would allow multiple control files to be > read in a binary format with a single system call. The existing API > would still be available as well, of course. The idea would be that > monitoring programs that frequently read lots of values from a single > cgroup (or even multiple cgroups) would be able to do so more cheaply > than by making multiple different reads on different files. > > In order for this to work, CGroups needs to know the data type of a > given control file - so this would only be available for the control > files that use typed cgroup output methods rather than the raw file > output interface. Sounds reasonable. >> I thought that filesystem-based >> API should be human readable and writable as much as possible... >> > > Yes, but even without a binary API it makes sense for values that are > likely to be parsed by programs be in a consistent format. > > But after thinking more about this, I think that the devices > permission control file output doesn't really fall under this category > - from a programmatic point of view, I suspect it's write-only, and > only humans will be reading the output, for debugging. Yup. So, if you're fine with the proposed API, I think I will prepare this set and send it to Andrew this week. > Paul > Thanks, Pavel _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2008-02-26T07:54:09-00:00