[PATCH 4.18 150/168] sysfs: Do not return POSIX ACL xattrs via listxattr
4.18-stable review patch. If anyone has any objections, please let me know.
--
From: Andreas Gruenbacher
commit ffc4c92227db5699493e43eb140b4cb5904c30ff upstream.
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields
to ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurèle La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
Cc: sta...@vger.kernel.org # v4.5+
Signed-off-by: Al Viro
Signed-off-by: Greg Kroah-Hartman
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,17 +949,19 @@ ssize_t simple_xattr_list(struct inode *
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
[PATCH 4.18 150/168] sysfs: Do not return POSIX ACL xattrs via listxattr
4.18-stable review patch. If anyone has any objections, please let me know.
--
From: Andreas Gruenbacher
commit ffc4c92227db5699493e43eb140b4cb5904c30ff upstream.
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields
to ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurèle La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
Cc: sta...@vger.kernel.org # v4.5+
Signed-off-by: Al Viro
Signed-off-by: Greg Kroah-Hartman
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,17 +949,19 @@ ssize_t simple_xattr_list(struct inode *
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
[PATCH 4.14 82/94] sysfs: Do not return POSIX ACL xattrs via listxattr
4.14-stable review patch. If anyone has any objections, please let me know.
--
From: Andreas Gruenbacher
commit ffc4c92227db5699493e43eb140b4cb5904c30ff upstream.
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields
to ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurèle La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
Cc: sta...@vger.kernel.org # v4.5+
Signed-off-by: Al Viro
Signed-off-by: Greg Kroah-Hartman
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -951,17 +951,19 @@ ssize_t simple_xattr_list(struct inode *
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
[PATCH 4.14 82/94] sysfs: Do not return POSIX ACL xattrs via listxattr
4.14-stable review patch. If anyone has any objections, please let me know.
--
From: Andreas Gruenbacher
commit ffc4c92227db5699493e43eb140b4cb5904c30ff upstream.
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields
to ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurèle La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
Cc: sta...@vger.kernel.org # v4.5+
Signed-off-by: Al Viro
Signed-off-by: Greg Kroah-Hartman
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -951,17 +951,19 @@ ssize_t simple_xattr_list(struct inode *
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
[PATCH 4.9 52/59] sysfs: Do not return POSIX ACL xattrs via listxattr
4.9-stable review patch. If anyone has any objections, please let me know.
--
From: Andreas Gruenbacher
commit ffc4c92227db5699493e43eb140b4cb5904c30ff upstream.
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields
to ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurèle La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
Cc: sta...@vger.kernel.org # v4.5+
Signed-off-by: Al Viro
Signed-off-by: Greg Kroah-Hartman
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -953,17 +953,19 @@ ssize_t simple_xattr_list(struct inode *
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
[PATCH 4.9 52/59] sysfs: Do not return POSIX ACL xattrs via listxattr
4.9-stable review patch. If anyone has any objections, please let me know.
--
From: Andreas Gruenbacher
commit ffc4c92227db5699493e43eb140b4cb5904c30ff upstream.
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields
to ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurèle La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
Cc: sta...@vger.kernel.org # v4.5+
Signed-off-by: Al Viro
Signed-off-by: Greg Kroah-Hartman
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -953,17 +953,19 @@ ssize_t simple_xattr_list(struct inode *
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
[PATCH] sysfs: Do not return POSIX ACL xattrs via listxattr
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields to
ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports
POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurele La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
CC: Stable # v4.5+
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index daa732550088..0d6a6a4af861 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -948,17 +948,19 @@ ssize_t simple_xattr_list(struct inode *inode, struct
simple_xattrs *xattrs,
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
--
2.17.1
[PATCH] sysfs: Do not return POSIX ACL xattrs via listxattr
Commit 786534b92f3c introduced a regression that caused listxattr to
return the POSIX ACL attribute names even though sysfs doesn't support
POSIX ACLs. This happens because simple_xattr_list checks for NULL
i_acl / i_default_acl, but inode_init_always initializes those fields to
ACL_NOT_CACHED ((void *)-1). For example:
$ getfattr -m- -d /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
Fix this in simple_xattr_list by checking if the filesystem supports
POSIX ACLs.
Fixes: 786534b92f3c ("tmpfs: listxattr should include POSIX ACL xattrs")
Reported-by: Marc Aurele La France
Tested-by: Marc Aurèle La France
Signed-off-by: Andreas Gruenbacher
CC: Stable # v4.5+
---
fs/xattr.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index daa732550088..0d6a6a4af861 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -948,17 +948,19 @@ ssize_t simple_xattr_list(struct inode *inode, struct
simple_xattrs *xattrs,
int err = 0;
#ifdef CONFIG_FS_POSIX_ACL
- if (inode->i_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(, _size,
-XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
+ if (IS_POSIXACL(inode)) {
+ if (inode->i_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+ if (inode->i_default_acl) {
+ err = xattr_list_one(, _size,
+XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
}
#endif
--
2.17.1
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
--- 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(, _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(, _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(>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
Anything more on this?
Thanks and have a great day.
Marc.
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
--- 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(, _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(, _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(>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
Anything more on this?
Thanks and have a great day.
Marc.
Re: sysfs: Do not return POSIX ACL xattrs via listxattr()
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
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,13 +949,13 @@ ssize_t simple_xattr_list(struct inode *inode, struct
simple_xattrs *xattrs,
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(, _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(, _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(>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
Thanks.
Marc.
Re: sysfs: Do not return POSIX ACL xattrs via listxattr()
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
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,13 +949,13 @@ ssize_t simple_xattr_list(struct inode *inode, struct
simple_xattrs *xattrs,
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(, _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(, _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(>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
Thanks.
Marc.
Re: sysfs: Do not return POSIX ACL xattrs via listxattr()
Marc,
Am Mo., 10. Sep. 2018 um 05:03 Uhr schrieb Marc Aurele La France
:
> 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=- /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.
>
> Please Reply-To-All.
>
> Thanks and have a great day.
>
> Marc.
>
> Signed-off-by: Marc Aurèle La France
>
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -949,13 +949,13 @@ ssize_t simple_xattr_list(struct inode *inode, struct
> simple_xattrs *xattrs,
> 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(, _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(, _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 = mapping;
INIT_HLIST_HEAD(>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
Thanks,
Andreas
Re: sysfs: Do not return POSIX ACL xattrs via listxattr()
Marc,
Am Mo., 10. Sep. 2018 um 05:03 Uhr schrieb Marc Aurele La France
:
> 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=- /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.
>
> Please Reply-To-All.
>
> Thanks and have a great day.
>
> Marc.
>
> Signed-off-by: Marc Aurèle La France
>
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -949,13 +949,13 @@ ssize_t simple_xattr_list(struct inode *inode, struct
> simple_xattrs *xattrs,
> 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(, _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(, _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 = mapping;
INIT_HLIST_HEAD(>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
Thanks,
Andreas
sysfs: Do not return POSIX ACL xattrs via listxattr()
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=- /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
#
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.
Please Reply-To-All.
Thanks and have a great day.
Marc.
Signed-off-by: Marc Aurèle La France
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,13 +949,13 @@ ssize_t simple_xattr_list(struct inode *inode, struct
simple_xattrs *xattrs,
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(, _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(, _size,
XATTR_NAME_POSIX_ACL_DEFAULT);
if (err)
sysfs: Do not return POSIX ACL xattrs via listxattr()
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=- /sys
/sys: system.posix_acl_access: Operation not supported
/sys: system.posix_acl_default: Operation not supported
#
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.
Please Reply-To-All.
Thanks and have a great day.
Marc.
Signed-off-by: Marc Aurèle La France
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,13 +949,13 @@ ssize_t simple_xattr_list(struct inode *inode, struct
simple_xattrs *xattrs,
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(, _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(, _size,
XATTR_NAME_POSIX_ACL_DEFAULT);
if (err)
