> On May 21, 2018, 7:24 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/linux/devices.cpp > > Lines 170-175 (patched) > > <https://reviews.apache.org/r/67097/diff/4/?file=2024122#file2024122line171> > > > > Any reason we only do that for the directory, not the actual device > > file? > > James Peach wrote: > The actual device file is supposed to be accessed by the task user, so it > has to be owned by that user. This chown is ensuring that we don't also grant > access to other host users as a side-effect of the container. > > Jie Yu wrote: > What do you mean by "side-effect"? > > James Peach wrote: > We only want to grant access to the containerized task, not to other > processes that might be running on the host. For example, when we grant read > access, we set `S_IRGRP | S_IROTH` which makes the device readable by > everyone on the host. Restricting the permissions on the container devices > directory prevents this, and setting the owner ensures we will still have > access even if we are later running as the task user. > > Jie Yu wrote: > What I don't follow is that why we need to set the ownership and > permission on the devicesDir. What if we just change the actual device files > to have proper ownership (task's user) and permission (700), will that be > sufficient? > > James Peach wrote: > > What if we just change the actual device files to have proper ownership > (task's user) and permission (700), will that be sufficient? > > That will work, but I think this approach is better semantically. It > presents devices as owned by root, which is what you would get on a VM or a > host, and it gives the expected access throughout the container, which is > consistent with the cgroups devices policy and avoids any potential issues > with multiple credentials inside the container. > > Jie Yu wrote: > I feel that it's a bit un-intuitive to me that the access control of the > device depends on the parent directory's permission (not the device itself). > For instance, a host process (user `foo`) having `CAP_DAC_READ_SEARCH` will > have write access to the device if the device is allowed to be written by the > container user `bar`.
As per our conversation on slack, I added a comment documenting that the access control for devices has container granularity. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67097/#review203515 ----------------------------------------------------------- On May 15, 2018, 5:53 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67097/ > ----------------------------------------------------------- > > (Updated May 15, 2018, 5:53 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu. > > > Bugs: MESOS-8792 > https://issues.apache.org/jira/browse/MESOS-8792 > > > Repository: mesos > > > Description > ------- > > Added `linux/devices` isolator support for populating the container > devices. This introduces a general mechanism for populating devices > into a specific container but currently only implements devices for all > containers based on the devices specified by the `--allowed_devices` > agent flag. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/67097/diff/4/ > > > Testing > ------- > > make check (Fedora 27) > > > Thanks, > > James Peach > >
