On 11/01/2025 13:44, Pádraig Brady wrote:
On 11/01/2025 08:37, Paul Eggert wrote:
On 2025-01-10 06:48, Pádraig Brady wrote:
With the previously discussed ls patch included, we also suppress the error
(while indicating the obtainable security context):
$ src/ls -l /mnt/nfs
total 0
--w-------. 1 padraig padraig 0 Jan 8 20:42 file
I'll push that ls patch now.
Thanks, but I'm not sure that resolves all the issues that can occur
when [l]listxattr returns -1 with errno==EACCES or errno==E2BIG. In this
case we've fixed the bug where the file has a security context but no
NFSv4 or POSIX ACLs. But what if the file has those ACLs?
True.
...
So I guess we need to treat EACCES from listxattr()
as meaning any significant xattr is present, but we should GET to confirm.
This shouldn't be too onerous perf wise at least given it's only for
files without read access.
The attached gnulib patch does that.
With a file without read access we get:
$ strace -e trace=/.*xattr.* ~/git/coreutils/src/ls -l /mnt/nfs/file
llistxattr("/mnt/nfs/file", 0x7ffc615bddcc, 152) = -1 EACCES (Permission denied)
lgetxattr("/mnt/nfs/file", "security.selinux", "system_u:object_r:nfs_t:s0",
255) = 27
lgetxattr("/mnt/nfs/file", "system.nfs4_acl", NULL, 0) = 128
lgetxattr("/mnt/nfs/file", "system.nfs4_acl",
"\0\0\0\5\0\0\0\1\0\0\0\0\0\0\0!\0\0\0\6OWNER@\0\0\0\0\0", 152) = 128
--w-rwx---+ 1 padraig padraig 0 Jan 8 20:42 /mnt/nfs/file
$ chmod u+r file # Restore normal fast case
$ strace -e trace=/.*xattr.* ~/git/coreutils/src/ls -l /mnt/nfs/file
llistxattr("/mnt/nfs/file", "system.nfs4_acl\0user.attr_foo\0", 152) = 30
lgetxattr("/mnt/nfs/file", "system.nfs4_acl",
"\0\0\0\5\0\0\0\1\0\0\0\0\0\0\0!\0\0\0\6OWNER@\0\0\0\0\0", 152) = 128
--w-rwx---+ 1 padraig padraig 0 Jan 8 20:42 /mnt/nfs/file
cheers,
Pádraig
From 05c63bc908a67a316fea29ddf4c702d89cf5bdec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 11 Jan 2025 16:40:33 +0000
Subject: [PATCH] file-has-acl: handle NFSv4 ACLs with listxattr returning
EACCES
* lib/file-has-acl.c (has_xattr): A new helper function to
lookup aclinfo for the xattr or fallback to getxattr() if appropriate.
(get_aclinfo): Use has_xattr() rather than aclinfo_has_xattr().
Discussed at <https://bugs.gnu.org/74692>
---
ChangeLog | 8 ++++++++
lib/file-has-acl.c | 46 ++++++++++++++++++++++++++++++++++++----------
2 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 9399bcbb9b..6ccd999e80 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2025-01-11 Pádraig Brady <p...@draigbrady.com>
+
+ file-has-acl: handle NFSv4 ACLs with listxattr returning EACCES
+ * lib/file-has-acl.c (has_xattr): A new helper function to
+ lookup aclinfo for the xattr or fallback to getxattr() if appropriate.
+ (get_aclinfo): Use has_xattr() rather than aclinfo_has_xattr().
+ Discussed at <https://bugs.gnu.org/74692>
+
2025-01-11 Bruno Haible <br...@clisp.org>
sys_un-h: Document the glibc bug.
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 5ec6b256ef..e8413f8f85 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -99,6 +99,36 @@ enum {
ACE4_IDENTIFIER_GROUP = 0x00000040
};
+/* AI indicates XATTR may be present but wasn't accessible.
+ This is the case when [l]listxattr failed with E2BIG,
+ or failed with EACCES which in Linux kernel 6.12 NFS can mean merely
+ that we lack read access.
+*/
+
+static bool
+aclinfo_may_indicate_xattr (struct aclinfo const *ai)
+{
+ return ai->size < 0 && (ai->u.err == EACCES || ai->u.err == E2BIG);
+}
+
+/* Does NAME have XATTR? */
+
+static bool
+has_xattr (char const *xattr, struct aclinfo const *ai,
+ MAYBE_UNUSED char const *restrict name, MAYBE_UNUSED int flags)
+{
+ if (ai && aclinfo_has_xattr (ai, xattr))
+ return true;
+ else if (!ai || aclinfo_may_indicate_xattr (ai))
+ {
+ int ret = ((flags & ACL_SYMLINK_FOLLOW ? getxattr : lgetxattr)
+ (name, xattr, NULL, 0));
+ if (0 <= ret || (errno == ERANGE || errno == E2BIG))
+ return true;
+ }
+ return false;
+}
+
/* Does AI's xattr set contain XATTR? */
bool
@@ -176,13 +206,9 @@ get_aclinfo (char const *name, struct aclinfo *ai, int flags)
}
}
- /* A security context can exist only if extended attributes do: i.e.,
- [l]listxattr either returned a positive number, or failed with E2BIG,
- or failed with EACCES which in Linux kernel 6.12 NFS can mean merely
- that we lack read access. */
+ /* A security context can exist only if extended attributes do. */
if (flags & ACL_GET_SCONTEXT
- && (0 < ai->size
- || (ai->size < 0 && (ai->u.err == E2BIG || ai->u.err == EACCES))))
+ && (0 < ai->size || aclinfo_may_indicate_xattr (ai)))
{
if (is_smack_enabled ())
{
@@ -358,7 +384,7 @@ file_has_aclinfo (MAYBE_UNUSED char const *restrict name,
int initial_errno = errno;
get_aclinfo (name, ai, flags);
- if (ai->size <= 0)
+ if (!aclinfo_may_indicate_xattr (ai) && ai->size <= 0)
{
errno = ai->size < 0 ? ai->u.err : initial_errno;
return ai->size;
@@ -369,11 +395,11 @@ file_has_aclinfo (MAYBE_UNUSED char const *restrict name,
In earlier Fedora the two types of ACLs were mutually exclusive.
Attempt to work correctly on both kinds of systems. */
- if (!aclinfo_has_xattr (ai, XATTR_NAME_NFSV4_ACL))
+ if (!has_xattr (XATTR_NAME_NFSV4_ACL, ai, name, flags))
return
- (aclinfo_has_xattr (ai, XATTR_NAME_POSIX_ACL_ACCESS)
+ (has_xattr (XATTR_NAME_POSIX_ACL_ACCESS, ai, name, flags)
|| ((d_type == DT_DIR || d_type == DT_UNKNOWN)
- && aclinfo_has_xattr (ai, XATTR_NAME_POSIX_ACL_DEFAULT)));
+ && has_xattr (XATTR_NAME_POSIX_ACL_DEFAULT, ai, name, flags)));
/* A buffer large enough to hold any trivial NFSv4 ACL.
The max length of a trivial NFSv4 ACL is 6 words for owner,
--
2.47.1
