On 15/07/2024 13:59, Bruno Haible wrote:
The CI of GNU sed shows a test failure of the Gnulib 'acl' tests on CentOS 7
[1]. Namely:

   FAIL: test-copy-acl.sh
   FAIL: test-copy-acl-1.sh
   FAIL: test-copy-acl-2.sh

It can be reproduced like this:

   $ echo 'Simple contents' > tmpfile0
   $ echo 'Simple contents' > tmpfile2
   $ chmod 600 tmpfile0
   $ setfacl -m user:1:1 tmpfile0
   $ getfacl tmpfile0
   user::rw-
   user:bin:--x
   group::---
   mask::--x
   other::---
   $ ./test-copy-acl tmpfile0 tmpfile2
   $ getfacl tmpfile2
   user::rw-
   group::--x
   other::---

The tests passed in older gnulib:

   2018-01-01 PASS
   2020-01-01 PASS
   2023-01-01 PASS
   2023-04-12 FAIL
   2024-01-01 FAIL

So, it must have been a regression from the 2023-01-12 commit.

Debugging it, I see two invocations of is_attr_permissions:

   is_attr_permissions("security.selinux",NULL) => 0
   is_attr_permissions("system.posix_acl_access",NULL) => 0

It is the latter which causes libattr to copy no attributes.

The patch below fixes it.

[1] https://github.com/gnu-sed/ci-check/actions/runs/9935643107


2024-07-15  Bruno Haible  <br...@clisp.org>

        qcopy-acl: Fix copying of ACLs on CentOS 7 (regression 2023-01-12).
        * lib/qcopy-acl.c: Include <string.h>, <linux/xattr.h>.
        (XATTR_NAME_NFSV4_ACL, XATTR_NAME_POSIX_ACL_ACCESS,
        XATTR_NAME_POSIX_ACL_DEFAULT): New macros, from file-has-acl.c.
        (is_attr_permissions): Test for these names explicitly.
        * m4/acl.m4 (gl_QCOPY_ACL): New macro.
        * modules/qcopy-acl (Files): Add m4/acl.m4.
        (configure.ac): Invoke gl_QCOPY_ACL.

diff --git a/lib/qcopy-acl.c b/lib/qcopy-acl.c
index dfc39cead0..877f42588b 100644
--- a/lib/qcopy-acl.c
+++ b/lib/qcopy-acl.c
@@ -26,6 +26,20 @@
  #if USE_XATTR
# include <attr/libattr.h>
+# include <string.h>
+
+# if HAVE_LINUX_XATTR_H
+#  include <linux/xattr.h>
+# endif
+# ifndef XATTR_NAME_NFSV4_ACL
+#  define XATTR_NAME_NFSV4_ACL "system.nfs4_acl"
+# endif
+# ifndef XATTR_NAME_POSIX_ACL_ACCESS
+#  define XATTR_NAME_POSIX_ACL_ACCESS "system.posix_acl_access"
+# endif
+# ifndef XATTR_NAME_POSIX_ACL_DEFAULT
+#  define XATTR_NAME_POSIX_ACL_DEFAULT "system.posix_acl_default"
+# endif
/* Returns 1 if NAME is the name of an extended attribute that is related
     to permissions, i.e. ACLs.  Returns 0 otherwise.  */
@@ -33,7 +47,12 @@
  static int
  is_attr_permissions (const char *name, struct error_context *ctx)
  {
-  return attr_copy_action (name, ctx) == ATTR_ACTION_PERMISSIONS;
+  /* We need to explicitly test for the known extended attribute names,
+     because at least on CentOS 7, attr_copy_action does not do it.  */
+  return strcmp (name, XATTR_NAME_POSIX_ACL_ACCESS) == 0
+         || strcmp (name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0
+         || strcmp (name, XATTR_NAME_NFSV4_ACL) == 0
+         || attr_copy_action (name, ctx) == ATTR_ACTION_PERMISSIONS;
  }

I was wondering a little about the generality of this patch,
and how it somwehat overrides /etc/xattr.conf which gives configurability
over how these xattrs are treated.

I checked one centos 7 system, and it didn't have a /etc/xattr.conf file
which might explain the behavior noticed above.

Then on centos 8 we have:
system.nfs4_acl                 permissions
system.nfs4acl                  permissions
system.posix_acl_access         permissions
system.posix_acl_default        permissions

While on Fedora 42 we have:
system.posix_acl_access         permissions
system.posix_acl_default        permissions

I'm not sure why nfs4 acls were only considered for a few Red Hat releases,
but it might explain the (already resolved) issue with nfs4 acls at
https://bugzilla.redhat.com/2363149

cheers,
Pádraig

Reply via email to