Messages in this thread | | | Date | Mon, 17 Sep 2018 08:01:30 -0600 (Mountain Daylight Time) | From | Marc Aurèle La France <> | Subject | Re: sysfs: Do not return POSIX ACL xattrs via listxattr() |
| |
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. | |