From: "Marc Aurèle La France" <tsi@tuyoix.net>
To: "Andreas Grünbacher" <andreas.gruenbacher@gmail.com>
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>
Subject: Re: sysfs: Do not return POSIX ACL xattrs via listxattr()
Date: Mon, 17 Sep 2018 08:01:30 -0600 (Mountain Daylight Time) [thread overview]
Message-ID: <alpine.WNT.2.20.1809170801030.4228@CLUIJ> (raw)
In-Reply-To: <alpine.WNT.2.20.1809101709260.3100@CLUIJ>
[-- Attachment #1: Type: text/plain, Size: 3042 bytes --]
On Mon, 10 Sep 2018, Marc Aurèle La France wrote:
> On Mon, 10 Sep 2018, Andreas Grünbacher wrote:
>> Am Mo., 10. Sep. 2018 schrieb Marc Aurèle La France:
>>> 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=- /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 >= 4.7. A fix for 4.5 <=
>>> 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.
>>> Signed-off-by: Marc Aurèle 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,
>>> int err = 0;
>>>
>>> #ifdef CONFIG_FS_POSIX_ACL
>>> - if (inode->i_acl) {
>>> + if (inode->i_acl && !is_uncached_acl(inode->i_acl)) {
>>> err = 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 = 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->i_mapping = mapping;
>> INIT_HLIST_HEAD(&inode->i_dentry); /* buggered by rcu freeing */
>> #ifdef CONFIG_FS_POSIX_ACL
>> - inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
>> + inode->i_acl = inode->i_default_acl =
>> + (sb->s_flags & SB_POSIXACL) ? ACL_NOT_CACHED : NULL;
>> #endif
>>
>> #ifdef CONFIG_FSNOTIFY
> Yes, that works too, and doesn't seem to cause other issues.
> Tested-by: Marc Aurèle La France <tsi@tuyoix.net>
Anything more on this?
Thanks and have a great day.
Marc.
prev parent reply other threads:[~2018-09-17 19:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-09 21:54 sysfs: Do not return POSIX ACL xattrs via listxattr() Marc Aurele La France
2018-09-10 20:53 ` Andreas Grünbacher
2018-09-10 23:16 ` Marc Aurèle La France
2018-09-17 14:01 ` Marc Aurèle La France [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.WNT.2.20.1809170801030.4228@CLUIJ \
--to=tsi@tuyoix.net \
--cc=agruenba@redhat.com \
--cc=andreas.gruenbacher@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tj@kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).