GNU bug report logs - #70214
'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

Previous Next

Full log

Message #11 received at 70214 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Bruno Haible <bruno <at> clisp.org>, 70214 <at> debbugs.gnu.org
Cc: Andreas Gruenbacher <andreas.gruenbacher <at> gmail.com>
Subject: Re: bug#70214: 'install' fails to copy regular file to autofs/cifs,
 due to ACL or xattr handling
Date: Sat, 13 Apr 2024 18:42:25 +0100

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.