Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <alpine.LNX.2.20.1809091544070.15534@fanir.tuyoix.net>
In-Reply-To: <alpine.LNX.2.20.1809091544070.15534@fanir.tuyoix.net>
From:   =?UTF-8?Q?Andreas_Gr=C3=BCnbacher?= <andreas.gruenbacher@gmail.com>
Date:   Mon, 10 Sep 2018 22:53:09 +0200
Message-ID: <CAHpGcMJ9jB+Ekfvf6HUvWUr8ipoSJkS+U5arShpMwyjuc4qftg@mail.gmail.com>
Subject: Re: sysfs: Do not return POSIX ACL xattrs via listxattr()
To:     tsi@tuyoix.net
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Tejun Heo <tj@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>,
        Andreas Gruenbacher <agruenba@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Marc,

Am Mo., 10. Sep. 2018 um 05:03 Uhr schrieb Marc Aurele La France
<tsi@tuyoix.net>:
> Greetings.
>
> Commit 786534b92f3ce68f4afc8a761c80b76887797b0a "tmpfs: listxattr
> should include POSIX ACL xattrs", which first appeared in 4.5 kernels,
> introduced a regression whereby listxattr() syscalls on anything in
> sysfs, or its mountpoint, return the name of the two POSIX ACL xattrs,
> but attempts to retrieve these values are denied with EOPNOTSUP.  For
> example ...
>
>         # getfattr -d --match=3D- /sys
>         /sys: system.posix_acl_access: Operation not supported
>         /sys: system.posix_acl_default: Operation not supported
>         #

I can confirm this regression.

> This inconsistent behaviour confuses rsync(1) (among others) when it
> runs into a sysfs mountpoint, even when told to not descend into it.
> This issue occurs because simple_xattr_list() does not correctly deal
> with cached ACLs.
>
> The suggested fix below was developed with a 4.18.7 kernel, but should
> apply, modulo patch fuzz, to any kernel >=3D 4.7.  A fix for 4.5 <=3D
> kernels < 4.7 is trivially different, but I won't bother given such
> kernels are no longer maintained.
>
> Note that the only other simple_xattr_list() caller, shmem, avoids
> this glitch by previously calling cache_no_acl() on all inodes it
> creates, so perhaps sysfs/kernfs should do the same.
>
> Please Reply-To-All.
>
> Thanks and have a great day.
>
> Marc.
>
> Signed-off-by: Marc Aur=C3=A8le La France <tsi@tuyoix.net>
>
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -949,13 +949,13 @@ ssize_t simple_xattr_list(struct inode *inode, stru=
ct simple_xattrs *xattrs,
>         int err =3D 0;
>
>  #ifdef CONFIG_FS_POSIX_ACL
> -       if (inode->i_acl) {
> +       if (inode->i_acl && !is_uncached_acl(inode->i_acl)) {
>                 err =3D xattr_list_one(&buffer, &remaining_size,
>                                      XATTR_NAME_POSIX_ACL_ACCESS);
>                 if (err)
>                         return err;
>         }
> -       if (inode->i_default_acl) {
> +       if (inode->i_default_acl && !is_uncached_acl(inode->i_default_acl=
)) {
>                 err =3D xattr_list_one(&buffer, &remaining_size,
>                                      XATTR_NAME_POSIX_ACL_DEFAULT);
>                 if (err)

This seems to be a better fix, but I haven't fully verified it, yet:

--- a/fs/inode.c
+++ b/fs/inode.c
@@ -187,7 +187,8 @@ int inode_init_always(struct super_block *sb,
struct inode *inode)
     inode->i_mapping =3D mapping;
     INIT_HLIST_HEAD(&inode->i_dentry);    /* buggered by rcu freeing */
 #ifdef CONFIG_FS_POSIX_ACL
-    inode->i_acl =3D inode->i_default_acl =3D ACL_NOT_CACHED;
+    inode->i_acl =3D inode->i_default_acl =3D
+           (sb->s_flags & SB_POSIXACL) ? ACL_NOT_CACHED : NULL;
 #endif

 #ifdef CONFIG_FSNOTIFY

Thanks,
Andreas