On Sat, Dec 19, 2020 at 12:16 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > When inode has no listxattr op of its own (e.g. squashfs) vfs_listxattr > calls the LSM inode_listsecurity hooks to list the xattrs that LSMs will > intercept in inode_getxattr hooks. > > When selinux LSM is installed but not initialized, it will list the > security.selinux xattr in inode_listsecurity, but will not intercept it > in inode_getxattr. This results in -ENODATA for a getxattr call for an > xattr returned by listxattr. > > This situation was manifested as overlayfs failure to copy up lower > files from squashfs when selinux is built-in but not initialized, > because ovl_copy_xattr() iterates the lower inode xattrs by > vfs_listxattr() and vfs_getxattr(). > > ovl_copy_xattr() skips copy up of security labels that are indentified by > inode_copy_up_xattr LSM hooks, but it does that after vfs_getxattr(). > Since we are not going to copy them, skip vfs_getxattr() of the security > labels. > > Reported-by: Michael Labriola <michael.d.labriola@xxxxxxxxx> > Tested-by: Michael Labriola <michael.d.labriola@xxxxxxxxx> > Link: https://lore.kernel.org/linux-unionfs/2nv9d47zt7.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/ > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > --- > > Miklos, > > This is a workaround for a v5.9 selinux related regression reported by > Michael that caused copy up failure is a very specific configuration > involving lower squashfs and built-in but disabled selinux. > > I've sent the bug fix to selinux list, so this patch is complementary. > I removed the stable/Fixes tags, because this patch does not cleanly > apply to v5.9 and is not the real bug fix. > Ping. FWIW, the selinux bug fix should already be in next. Thanks, Amir. > > fs/overlayfs/copy_up.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c > index e5b616c93e11..0fed532efa68 100644 > --- a/fs/overlayfs/copy_up.c > +++ b/fs/overlayfs/copy_up.c > @@ -84,6 +84,14 @@ int ovl_copy_xattr(struct super_block *sb, struct dentry *old, > > if (ovl_is_private_xattr(sb, name)) > continue; > + > + error = security_inode_copy_up_xattr(name); > + if (error < 0 && error != -EOPNOTSUPP) > + break; > + if (error == 1) { > + error = 0; > + continue; /* Discard */ > + } > retry: > size = vfs_getxattr(old, name, value, value_size); > if (size == -ERANGE) > @@ -107,13 +115,6 @@ int ovl_copy_xattr(struct super_block *sb, struct dentry *old, > goto retry; > } > > - error = security_inode_copy_up_xattr(name); > - if (error < 0 && error != -EOPNOTSUPP) > - break; > - if (error == 1) { > - error = 0; > - continue; /* Discard */ > - } > error = vfs_setxattr(new, name, value, size, 0); > if (error) { > if (error != -EOPNOTSUPP || ovl_must_copy_xattr(name)) > -- > 2.25.1 >