On Fri, Dec 21, 2018 at 5:59 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > On Fri, Dec 21, 2018 at 4:49 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > On Fri, Dec 21, 2018 at 9:45 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > > > On 12/21/18 3:18 PM, Ondrej Mosnacek wrote: > > > > Ignore all selinux_inode_notifysecctx() calls on mounts with SBLABEL_MNT > > > > flag unset. This is achived by returning -EOPNOTSUPP for this case in > > > > selinux_inode_setsecurtity() (because that function should not be called > > > > in such case anyway) and translating this error to 0 in > > > > selinux_inode_notifysecctx(). > > > > > > > > This fixes behavior of kernfs-based filesystems when mounted with the > > > > 'context=' option. Before this patch, if a node's context had been > > > > explicitly set to a non-default value and later the filesystem has been > > > > remounted with the 'context=' option, then this node would show up as > > > > having the manually-set context and not the mount-specified one. > > > > > > > > Steps to reproduce: > > > > # mount -t cgroup2 cgroup2 /sys/fs/cgroup/unified > > > > # chcon unconfined_u:object_r:user_home_t:s0 /sys/fs/cgroup/unified/cgroup.stat > > > > # ls -lZ /sys/fs/cgroup/unified > > > > total 0 > > > > -r--r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Dec 13 10:41 cgroup.controllers > > > > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Dec 13 10:41 cgroup.max.depth > > > > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Dec 13 10:41 cgroup.max.descendants > > > > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Dec 13 10:41 cgroup.procs > > > > -r--r--r--. 1 root root unconfined_u:object_r:user_home_t:s0 0 Dec 13 10:41 cgroup.stat > > > > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Dec 13 10:41 cgroup.subtree_control > > > > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Dec 13 10:41 cgroup.threads > > > > # umount /sys/fs/cgroup/unified > > > > # mount -o context=system_u:object_r:tmpfs_t:s0 -t cgroup2 cgroup2 /sys/fs/cgroup/unified > > > > > > > > Result before: > > > > # ls -lZ /sys/fs/cgroup/unified > > > > total 0 > > > > -r--r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.controllers > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.max.depth > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.max.descendants > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.procs > > > > -r--r--r--. 1 root root unconfined_u:object_r:user_home_t:s0 0 Dec 13 10:41 cgroup.stat > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.subtree_control > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.threads > > > > > > > > Result after: > > > > # ls -lZ /sys/fs/cgroup/unified > > > > total 0 > > > > -r--r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.controllers > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.max.depth > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.max.descendants > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.procs > > > > -r--r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.stat > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.subtree_control > > > > -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.threads > > > > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > > > > > > The patch looks good to me but I am a little puzzled by the cgroup2 > > > behavior here. I would have expected that unmounting would have killed > > > the superblock and thus discarded the previously set label. I guess > > > something else is keeping it alive? > > > > It is because the context set via setxattr is stored inside the kernfs > > node and the kernfs tree is preserved across mounts/unmounts. It > > actually behaves sort of like a normal filesystem backed by a block > > device, just the "device" is represented by an in-memory kernfs tree > > which is discarded at poweroff. > > That makes sense. > > I'll take a closer look at these once we're past the upcoming merge > window, but they look okay after a quick glance. Still looked fine to me, merged both 1/2 and 2/2 into selinux/next. Thanks Ondrej. -- paul moore www.paul-moore.com
