[bisected] Re: docker overlay support broken post v4.8

Commit Message

Vivek Goyal Oct. 13, 2016, 9:48 p.m. UTC

On Thu, Oct 13, 2016 at 05:39:37PM -0400, Vivek Goyal wrote:
[..]
> > > > This message splits in the console.
> > > > [61250.857832] SELinux: (dev overlay, type overlay) has no xattr support
> > > Reverted the patchset of "Xattr inode operation removal" against the latest mainline
> > > fixed the problem, i.e., commits below in order.
> > > 
> > > fd50ecaddf8372a1d96e0daeaac0f93cf04e4d42
> > > 6c6ef9f26e598fb977f60935e109cd5b266c941a
> > > bf3ee71363c0b44acb62f375aea470262ac4210a
> > > 5d6c31910bc0713e37628dc0ce677dcb13c8ccf4
> > > f5c244383725a6de06bc62fa7c54c0ea0d942eec
> > > 5f6e59ae8277cef221fdbf9b12f0c4f80db59944
> > > d0a5b995a308347fdb1bb0412df32acd0312523b
> > 
> > Looking at selinux code, it seems to be coming from following code.
> > Looks like in case of overlay inode, we are not setting
> > IOP_XATTR?
> > 
> > Vivek
> > 
> > sb_finish_set_opts()
> >  
> >         if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
> >                 /* Make sure that the xattr handler exists and that no
> >                    error other than -ENODATA is returned by getxattr on
> >                    the root directory.  -ENODATA is ok, as this may be
> >                    the first boot of the SELinux kernel before we have
> >                    assigned xattr values to the filesystem. */
> >                 if (!(root_inode->i_opflags & IOP_XATTR)) {
> >                         printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
> >                                "xattr support\n", sb->s_id, sb->s_type->name);
> >                         rc = -EOPNOTSUPP;
> >                         goto out;
> 
> IOP_XATTR is set, if sb->s_xattr is not NULL.
> 
> inode_init_always() {
> 	if (sb->s_xattr)
> 		inode->i_opflags |= IOP_XATTR;
> }
> 
> In case of overlay, when preparing super block and root dentry,
> sb->s_xattr has not been set yet and we call, 
> 
> ovl_new_inode()
>   new_inode()
>     alloc_inode()
> 	inode_init_always()
> 
> Looks like we need to initalize overlay super block little more and
> set s_xattr before calling ovl_new_inode().

Following hack fix seems to fix the issue for me.


---
 fs/overlayfs/super.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Andreas Grünbacher Oct. 14, 2016, 1:08 a.m. UTC | #1

2016-10-13 23:48 GMT+02:00 Vivek Goyal <[e-mail]>:
> On Thu, Oct 13, 2016 at 05:39:37PM -0400, Vivek Goyal wrote:
> [..]
>> > > > This message splits in the console.
>> > > > [61250.857832] SELinux: (dev overlay, type overlay) has no xattr support
>> > > Reverted the patchset of "Xattr inode operation removal" against the latest mainline
>> > > fixed the problem, i.e., commits below in order.
>> > >
>> > > fd50ecaddf8372a1d96e0daeaac0f93cf04e4d42
>> > > 6c6ef9f26e598fb977f60935e109cd5b266c941a
>> > > bf3ee71363c0b44acb62f375aea470262ac4210a
>> > > 5d6c31910bc0713e37628dc0ce677dcb13c8ccf4
>> > > f5c244383725a6de06bc62fa7c54c0ea0d942eec
>> > > 5f6e59ae8277cef221fdbf9b12f0c4f80db59944
>> > > d0a5b995a308347fdb1bb0412df32acd0312523b
>> >
>> > Looking at selinux code, it seems to be coming from following code.
>> > Looks like in case of overlay inode, we are not setting
>> > IOP_XATTR?
>> >
>> > Vivek
>> >
>> > sb_finish_set_opts()
>> >
>> >         if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
>> >                 /* Make sure that the xattr handler exists and that no
>> >                    error other than -ENODATA is returned by getxattr on
>> >                    the root directory.  -ENODATA is ok, as this may be
>> >                    the first boot of the SELinux kernel before we have
>> >                    assigned xattr values to the filesystem. */
>> >                 if (!(root_inode->i_opflags & IOP_XATTR)) {
>> >                         printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
>> >                                "xattr support\n", sb->s_id, sb->s_type->name);
>> >                         rc = -EOPNOTSUPP;
>> >                         goto out;
>>
>> IOP_XATTR is set, if sb->s_xattr is not NULL.
>>
>> inode_init_always() {
>>       if (sb->s_xattr)
>>               inode->i_opflags |= IOP_XATTR;
>> }
>>
>> In case of overlay, when preparing super block and root dentry,
>> sb->s_xattr has not been set yet and we call,
>>
>> ovl_new_inode()
>>   new_inode()
>>     alloc_inode()
>>       inode_init_always()
>>
>> Looks like we need to initalize overlay super block little more and
>> set s_xattr before calling ovl_new_inode().
>
> Following hack fix seems to fix the issue for me.

This looks correct. I've sent a cleaned up version with a description
for this (Message-Id:
<[e-mail]>) which should
be ready to apply unless Al or Miklos objects.

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: rhvgoyal-linux/fs/overlayfs/super.c
===================================================================
--- rhvgoyal-linux.orig/fs/overlayfs/super.c	2016-10-12 15:59:16.197781805 -0400
+++ rhvgoyal-linux/fs/overlayfs/super.c	2016-10-13 17:42:28.207365951 -0400
@@ -1292,6 +1292,9 @@  static int ovl_fill_super(struct super_b
 	if (!oe)
 		goto out_put_cred;
 
+
+	sb->s_xattr = ovl_xattr_handlers;
+
 	root_dentry = d_make_root(ovl_new_inode(sb, S_IFDIR));
 	if (!root_dentry)
 		goto out_free_oe;
@@ -1317,7 +1320,6 @@  static int ovl_fill_super(struct super_b
 
 	sb->s_magic = OVERLAYFS_SUPER_MAGIC;
 	sb->s_op = &ovl_super_operations;
-	sb->s_xattr = ovl_xattr_handlers;
 	sb->s_root = root_dentry;
 	sb->s_fs_info = ufs;
 	sb->s_flags |= MS_POSIXACL | MS_NOREMOTELOCK;