On 05/04/2024 10:48, Bruno Haible wrote:
Hi,
The 'install' program from coreutils-9.5 fails to copy a regular file
from an ext4 mount to an autofs/cifs mount.
The same operation, with 'cp -a', works fine.
Also, it works fine when coreutils was built with the configure options
"--disable-acl --disable-xattr".
How to reproduce
================
1) On the machine sparcdev.matoro.tk (Linux 6.8.2), I built coreutils-9.5
from source,
- once with default options, in build-sparc64/,
- once with "--disable-acl --disable-xattr", in build-sparc64-no-acl/.
2) Create a regular file on an ext4 mount:
$ echo hi > /var/tmp/foo3941
$ ls -lZ /var/tmp/foo3941
-rw-r----- 1 g-haible g-haible ? 3 Apr 4 13:29 /var/tmp/foo3941
$ getfacl /var/tmp/foo3941
getfacl: Removing leading '/' from absolute path names
# file: var/tmp/foo3941
# owner: g-haible
# group: g-haible
user::rw-
group::r--
other::---
$ df -m /var/tmp/
Filesystem 1M-blocks Used Available Use% Mounted on
/dev/root 560245 123140 408574 24% /
$ mount | grep ' / '
/dev/sda2 on / type ext4 (rw,noatime)
3) Details about the destination directory:
$ echo $HOME
/media/guest-homedirs/haible
$ mount | grep /media/guest-homedirs/haible
/etc/autofs/auto.guest-homedirs on /media/guest-homedirs/haible type autofs
(rw,relatime,fd=7,pgrp=2325,timeout=60,minproto=5,maxproto=5,direct,pipe_ino=46092)
//syslog.matoro.tk/guest-haible on /media/guest-homedirs/haible type cifs
(rw,nosuid,relatime,vers=1.0,cache=strict,username=nobody,uid=30014,forceuid,gid=30014,forcegid,addr=fd05:0000:0000:0000:0000:0000:0000:0001,soft,unix,posixpaths,serverino,mapposix,acl,rsize=1048576,wsize=65536,bsize=1048576,retrans=1,echo_interval=60,actimeo=1,closetimeo=1)
4) The operation that fails:
$ build-sparc64/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941; echo $?
build-sparc64/src/ginstall: setting permissions for
'/media/guest-homedirs/haible/foo3941': Permission denied
1
$ build-sparc64-no-acl/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941; echo $?
0
5) The same thing with 'cp -a' succeeds:
$ build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0
$ build-sparc64-no-acl/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0
6) 'strace' shows a failing call to fsetxattr:
$ strace build-sparc64/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941
fsetxattr(4, "system.posix_acl_access",
"\2\0\0\0\1\0\6\0\377\377\377\377\4\0\0\0\377\377\377\377 \0\0\0\377\377\377\377", 28, 0)
= -1 EACCES (Permission denied)
fchmod(4, 0600) = 0
Notes
=====
The 'cp' program does *not* use fsetxattr() calls on the destination file
descriptor and therefore does not fail:
$ strace build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941
flistxattr(3, NULL, 0) = 0
flistxattr(3, 0x7feff9860a0, 0) = 0
fchmod(4, 0100640) = 0
flistxattr(3, NULL, 0) = 0
flistxattr(3, 0x7feff9860c0, 0) = 0
As you can see, it uses 4 flistxattr() calls on the source file descriptor,
apparently detecting that it's a regular file without ACLs, and proceeds to
do a simple fchmod() call on the destination file descriptor.
Probably the same logic is needed in the 'install' program.
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.
cheers,
Pádraig.From d828d9656c3bd1ddf0fcddb578ddb2ed9a4d3701 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 13 Apr 2024 17:13:02 +0100
Subject: [PATCH] acl-permissions: avoid erroneous failure on CIFS
* lib/set-permissions.c (set_acls): On Linux also discount
EACESS as a valid errno with FD operations, as CIFS was seen to
return that erroneously in some cases.
---
ChangeLog | 7 +++++++
lib/set-permissions.c | 8 +++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index c72165e268..fd094d1091 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-04-13 Pádraig Brady <p...@draigbrady.com>
+
+ acl-permissions: avoid erroneous failure on CIFS
+ * lib/set-permissions.c (set_acls): On Linux (and others), also discount
+ EACESS as a valid errno with FD operations, as CIFS was seen to
+ return that erroneously in some cases.
+
2024-04-13 Bruno Haible <br...@clisp.org>
gnulib-tool.py: Code tweak.
diff --git a/lib/set-permissions.c b/lib/set-permissions.c
index 83a355faa5..7f8e55f5cd 100644
--- a/lib/set-permissions.c
+++ b/lib/set-permissions.c
@@ -520,7 +520,13 @@ set_acls (struct permission_context *ctx, const char *name, int desc,
ret = acl_set_file (name, ACL_TYPE_ACCESS, ctx->acl);
if (ret != 0)
{
- if (! acl_errno_valid (errno))
+ if (! acl_errno_valid (errno)
+# if defined __linux__
+ /* Special case EACCES for fd operations as CIFS
+ was seen to erroneously return that in some cases. */
+ || (HAVE_ACL_SET_FD && desc != -1 && errno == EACCES)
+# endif
+ )
{
ctx->acls_not_supported = true;
if (from_mode || acl_access_nontrivial (ctx->acl) == 0)
--
2.44.0
