bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Pádraig Brady]

On 13/04/2024 17:39, Pádraig Brady wrote:
> install(1) defaults to mode 600 for new files, and uses set_acl() with that
> (since 2007 https://github.com/coreutils/coreutils/commit/f634e8844 )
> The psuedo code that install(1) uses is:
>
> copy_reg()
>     if (x->set_mode) /* install */
>       set_acl(dest, x->mode /* 600 */)
>         ctx->acl = acl_from_mode ( /* 600 */)
>         acl_set_fd (ctx->acl) /* fails EACCES */
>         if (! acls_set)
>            must_chmod = true;
>         if (must_chmod)
>           saved_errno = EACCES;
>           chmod (ctx->mode /* 600 */)
>           if (save_errno)
>             return -1;
>
> This issue only only seems to be on CIFS.
> I'm seeing lot of weird behavior with ACLs there:
>
>     acl_set_fd (acl_from_mode (600)) -> EACCES
>     acl_set_fd (acl_from_mode (755)) -> EINVAL
>     getxattr ("system.posix_acl_access") -> EOPNOTSUPP
>
> Note we ignore EINVAL and EOPNOTSUPP errors in set_acl(),
> and it's just the EACCES that's problematic.
> Note this is quite similar to https://debbugs.gnu.org/65599
> where Paul also noticed EACCES with fsetxattr() (and others) on CIFS.
>
> The attached is a potential solution which I tested as working
> on the same matoro system that Bruno used.
>
> I think I'll apply that after thinking a bit more about it.

Actually that probably isn't appropriate,
as fsetxattr() can validly return EACESS
even though not documented in the man page at least.
The following demonstrates that:
.
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/xattr.h>
#include <fcntl.h>
#include <attr/libattr.h>
#include <stdio.h>
#include <unistd.h>

int main(void)
{
    int wfd;
    /* Note S_IWUSR is not set.  */
    if ((wfd=open("writable", O_CREAT|O_WRONLY|O_EXCL, S_IRUSR)) == -1)
        fprintf(stderr, "open('writable') error [%m]\n");
    if (write(wfd, "data", 1) == -1)
        fprintf(stderr, "write() error [%m]\n");
    if (fsetxattr(wfd, "user.test", "test", 4, 0) == -1)
        fprintf(stderr, "fsetxattr() error [%m]\n");
}

Another solution might be to file_has_acl() in copy.c
and skip if "not supported" or nothing is returned.
The following would do that, but I'm not sure about this
(as I'm not sure about ACLs in general TBH).
Note listxattr() returns 0 on CIFS here,
while getxattr ("system.posix_acl_access") returns EOPNOTSUPP,
and file_has_acl() uses listxattr() first.

diff --git a/src/copy.c b/src/copy.c
index d584a27eb..2145d89d5 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1673,8 +1673,13 @@ set_dest_mode:
     }
   else if (x->set_mode)
     {
-      if (set_acl (dst_name, dest_desc, x->mode) != 0)
-        return_val = false;
+      errno = 0;
+      int n = file_has_acl (dst_name, &sb);
+      if (0 < n || (errno && ! (is_ENOTSUP (errno) || errno == ENOSYS)))
+        {
+          if (set_acl (dst_name, dest_desc, x->mode) != 0)
+            return_val = false;
+        }
     }


BTW I'm surprised this wasn't reported previously for CIFS,
so I due to this bug and https://debbugs.gnu.org/65599
I suspect a recentish change in CIFS wrt EACCES.

cheers,
Pádraig