Miklos Szeredi <mszer...@redhat.com> writes: > It turns out overlayfs is actually okay wrt. mutliple conversions, because > it uses the right context for lower operations. I.e. before calling > vfs_{set,get}xattr() on underlying fs, it overrides creds with that of the > mounter, so the current user ns will now match that of > overlay_sb->s_user_ns, meaning that the caps will be converted to just the > right format for the next layer > > OTOH ecryptfs, which is the only other one affected by commit 7c03e2cda4a5 > ("vfs: move cap_convert_nscap() call into vfs_setxattr()") needs to be > fixed up, since it doesn't do the cap override thing that overlayfs does. > > I don't have an ecryptfs setup, so untested, but it's a fairly trivial > change. > > My other observation was that cap_inode_getsecurity() messes up conversion > of caps in more than one case. This is independent of the overlayfs user > ns enablement but affects it as well. > > Maybe we can revisit the infrastructure improvements we discussed, but I > think these fixes are more appropriate for the current cycle.
I mostly agree. Fixing the bugs in a back-portable way is important. However we need to sort out the infrastructure, and implementation. As far as I can tell it is only the fact that overlayfs does not support the new mount api aka fs_context that allows this fix to work and be correct. I believe the new mount api would allow specifying a different userns thatn curent_user_ns for the overlay filesystem and that would break this. So while I agree with the making a minimal fix for now. We need a good fix because this code is much too subtle, and it can break very easily with no one noticing. Eric > Thanks, > Miklos > > Miklos Szeredi (2): > ecryptfs: fix uid translation for setxattr on security.capability > security.capability: fix conversions on getxattr > > fs/ecryptfs/inode.c | 10 +++++-- > security/commoncap.c | 67 ++++++++++++++++++++++++++++---------------- > 2 files changed, 50 insertions(+), 27 deletions(-)