linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Aurele La France <tsi@tuyoix.net>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	Andreas Gruenbacher <agruenba@redhat.com>
Subject: sysfs:  Do not return POSIX ACL xattrs via listxattr()
Date: Sun, 9 Sep 2018 15:54:19 -0600 (Mountain Daylight Time)	[thread overview]
Message-ID: <alpine.LNX.2.20.1809091544070.15534@fanir.tuyoix.net> (raw)

[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]

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 <tsi@tuyoix.net>

--- 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(&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)

             reply	other threads:[~2018-09-10  7:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-09 21:54 Marc Aurele La France [this message]
2018-09-10 20:53 ` sysfs: Do not return POSIX ACL xattrs via listxattr() Andreas Grünbacher
2018-09-10 23:16   ` Marc Aurèle La France
2018-09-17 14:01     ` Marc Aurèle La France

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.LNX.2.20.1809091544070.15534@fanir.tuyoix.net \
    --to=tsi@tuyoix.net \
    --cc=agruenba@redhat.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).