bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-02 Thread Paul Eggert 

On 2023-09-02 04:18, Bruno Haible wrote:

chmod fails with EACCES, hence per POSIX

it's buggy.


Thanks, I reported the bug with chmod, chown, etc. to the linux-cifs 
mailing list, here:


https://lore.kernel.org/linux-cifs/fe8ab586-c697-583b-650d-3adac64df...@cs.ucla.edu/T/#u

I did some testing with fsetxattr and found that it has a similar issue 
even with ext4; for example, setxattr("/", "a", "b", 1, 0) fails with 
EACCES. This is not a POSIX violation, since POSIX lacks setxattr. 
However it's bad form. And it means coreutils needs the attached 
workaround not just for CIFS, but also for ext4 in some (rare) cases.


I installed the attached workaround on coreutils and am closing the bug 
report. Thanks again for mentioning it.From 9cd52bd9993163c2ef8b3d62b757c573fb5320df Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 2 Sep 2023 13:27:45 -0700
Subject: [PATCH] cp,mv,install: fix chmod on Linux CIFS

This bug occurs only when temporarily setting the mode to the
intersection of old and new modes when changing ownership.
* src/copy.c (owner_failure_ok): Treat EACCES like EPERM.
---
 NEWS   | 6 +++---
 src/copy.c | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 6801832f7..2adea1e11 100644
--- a/NEWS
+++ b/NEWS
@@ -4,9 +4,9 @@ GNU coreutils NEWS-*- outline -*-
 
 ** Bug fixes
 
-  cp, mv, install no longer issue spurious "failed to preserve ownership"
-  diagnostics when copying to GNU/Linux CIFS filesystems.  They do
-  this by working around a Linux CIFS bug.
+  cp, mv, and install no longer issue spurious diagnostics like "failed
+  to preserve ownership" when copying to GNU/Linux CIFS file systems.
+  They do this by working around some Linux CIFS bugs.
 
   numfmt options like --suffix no longer have an arbitrary 127-byte limit.
   [bug introduced with numfmt in coreutils-8.21]
diff --git a/src/copy.c b/src/copy.c
index 0b3de04f3..9f1f3e85a 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -3468,7 +3468,8 @@ chown_failure_ok (struct cp_options const *x)
 static bool
 owner_failure_ok (struct cp_options const *x)
 {
-  return ((errno == EPERM || errno == EINVAL) && !x->owner_privileges);
+  return ((errno == EPERM || errno == EINVAL || errno == EACCES)
+  && !x->owner_privileges);
 }
 
 /* Return the user's umask, caching the result.
-- 
2.39.2

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-02 Thread Bruno Haible 
Paul Eggert wrote:
> Yes please. If chmod also does not work coreutils might need more 
> workarounds.
> 
> Just to be clear; please try it on a file where chmod should fail 
> because you don't own the file and you're not root. chmod should fail 
> with EPERM in this case; it it fails with EACCES it's buggy. Thanks.

chmod fails with EACCES, hence per POSIX

it's buggy.

See: On the server I have 2 files and 2 directories:

# ls -ld dir1 dir1/file1 dir2 dir2/file2
drwxrwxr-x2 brunoeveryone  4096 Sep  2 12:58 dir1/
-rw-rw-r--1 brunoeveryone 4 Sep  2 00:52 dir1/file1
drwxrwxrwx2 adminadminist  4096 Sep  2 00:54 dir2/
-rw-rw-r--1 adminadminist 4 Sep  2 00:54 dir2/file2

The user that exports the mount point is 'bruno'. But since dir2
and dir2/file2 are not owned by 'bruno', an error will (and should)
occur when these two are manipulated through the CIFS client.

chown fails; that's the problem you already committed a workaround for:

$ ./fchownat-test /media/nas/bruno/dir1/file1
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied
$ ./fchownat-test /media/nas/bruno/dir1
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied
$ ./fchownat-test /media/nas/bruno/dir2/file2
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied
$ ./fchownat-test /media/nas/bruno/dir2
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied

chmod also fails with EACCES instead of EPERM:

$ ./fchmodat-test /media/nas/bruno/dir1/file1
$ ./fchmodat-test /media/nas/bruno/dir1
$ ./fchmodat-test /media/nas/bruno/dir2/file2
lchmod: Permission denied
fchmodat: Permission denied
chmod: Permission denied
fchmod: Permission denied
$ ./fchmodat-test /media/nas/bruno/dir2
lchmod: Permission denied
fchmodat: Permission denied
chmod: Permission denied
fchmod: Permission denied

The same thing with strace:

$ strace ./fchmodat-test /media/nas/bruno/dir2/file2
...
newfstatat(AT_FDCWD, "/media/nas/bruno/dir2/file2", {st_mode=S_IFREG|0664, 
st_size=4, ...}, AT_SYMLINK_NOFOLLOW) = 0
openat(AT_FDCWD, "/media/nas/bruno/dir2/file2", 
O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH) = 3
newfstatat(3, "", {st_mode=S_IFREG|0664, st_size=4, ...}, AT_EMPTY_PATH) = 0
chmod("/proc/self/fd/3", 0664)  = -1 EACCES (Permission denied)
close(3)= 0
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x402 (flags O_RDWR|O_APPEND)
getrandom("\x68\x8f\xbd\x08\xf8\x4b\xa9\xa5", 8, GRND_NONBLOCK) = 8
brk(NULL)   = 0x5563c5d01000
brk(0x5563c5d22000) = 0x5563c5d22000
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x5), ...}, 
AT_EMPTY_PATH) = 0
write(3, "lchmod: Permission denied\n", 26lchmod: Permission denied
) = 26
close(3)= 0
openat(AT_FDCWD, "/media/nas/bruno/dir2/file2", 
O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH) = 3
newfstatat(3, "", {st_mode=S_IFREG|0664, st_size=4, ...}, AT_EMPTY_PATH) = 0
chmod("/proc/self/fd/3", 0664)  = -1 EACCES (Permission denied)
close(3)= 0
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x402 (flags O_RDWR|O_APPEND)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x5), ...}, 
AT_EMPTY_PATH) = 0
write(3, "fchmodat: Permission denied\n", 28fchmodat: Permission denied
) = 28
close(3)= 0
chmod("/media/nas/bruno/dir2/file2", 0664) = -1 EACCES (Permission denied)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x402 (flags O_RDWR|O_APPEND)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x5), ...}, 
AT_EMPTY_PATH) = 0
write(3, "chmod: Permission denied\n", 25chmod: Permission denied
) = 25
close(3)= 0
openat(AT_FDCWD, "/media/nas/bruno/dir2/file2", O_RDONLY|O_NOFOLLOW) = 3
fchmod(3, 0664) = -1 EACCES (Permission denied)
dup(2)  = 4
fcntl(4, F_GETFL)   = 0x402 (flags O_RDWR|O_APPEND)
newfstatat(4, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x5), ...}, 
AT_EMPTY_PATH) = 0
write(4, "fchmod: Permission denied\n", 26fchmod: Permission denied
) = 26
close(4)= 0
exit_group(1)   = ?
+++ exited with 1 +++

$ strace ./fchmodat-test /media/nas/bruno/dir2
...
newfstatat(AT_FDCWD, "/media/nas/bruno/dir2", {st_mode=S_IFDIR|0777, st_size=0, 
...}, AT_SYMLINK_NOFOLLOW) = 0
openat(AT_FDCWD, "/media/nas/bruno/dir2", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH) 
= 3
newfstatat(3, "", {st_mode=S_IFDIR|0777, st_size=0, ...}, AT_EMPTY_PATH) = 0

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Paul Eggert 

On 2023-09-01 19:52, Paul Eggert wrote:
Yes please. If chmod also does not work coreutils might need more 
workarounds.


Just to be clear; please try it on a file where chmod should fail 
because you don't own the file and you're not root. chmod should fail 
with EPERM in this case; it it fails with EACCES it's buggy. Thanks.

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Paul Eggert 

On 2023-09-01 16:12, Bruno Haible wrote:

Well, you asked me to try it on / but / is an ext4 FS, not a CIFS FS
on my system. Should I try it also on a directory inside the CIFS mount?


Yes please. If chmod also does not work coreutils might need more 
workarounds.

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Bruno Haible 
Paul Eggert wrote:
> Good, thanks for confirming that the chmod family does not have a 
> similar bug.

Well, you asked me to try it on / but / is an ext4 FS, not a CIFS FS
on my system. Should I try it also on a directory inside the CIFS mount?

> I installed the attached workaround into coreutils master on Savannah. 
> Please give it a try.

It does remove the diagnostic as intended:

Preparations:
$ mkdir dir3 dir4
$ echo foo > dir3/file3
$ echo foo > dir4/file4

With coreutils-9.4:
$ mv dir3 /media/nas/bruno/
/gnu-inst-coreutils/9.4/bin/mv: failed to preserve ownership for 
'/media/nas/bruno/dir3': Permission denied

With coreutils from git now:
$ src/mv dir4 /media/nas/bruno/


I had worries regarding losing diagnostics that would be worth seeing.
But in fact, coreutils-9.4 did not actually produce such a diagnostic.
So, there is no regression for this case.

Preparations:
$ mkdir dir1 dir2
$ echo foo > dir1/file1
$ echo foo > dir2/file2
$ chmod a+rwx dir1 dir2
$ sudo chown root:root dir1 dir2

With coreutils-9.4:
$ mv dir1 /media/nas/bruno/dir1

With coreutils from git now:
$ src/mv dir2 /media/nas/bruno/dir2

Bruno

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Bruno Haible 
Paul Eggert wrote:
> > If you compile and run the attached 
> > program on a file that you don't own (e.g., './a.out /'), does it 
> > incorrectly issue "Permission denied" instead of "Operation not 
> > permitted" diagnostics?

and I replied:

> In this case, the diagnostic is "Operation not permitted" (from EPERM):
> 
> $ ./a.out /
> lchown: Operation not permitted
> fchownat: Operation not permitted
> chown: Operation not permitted
> fchown: Operation not permitted

Oops, I mistakenly compiled and ran the wrong test program. Here are the
results for the test program you meant:

$ ./a.out /
lchmod: Operation not permitted
fchmodat: Operation not permitted
chmod: Operation not permitted
fchmod: Operation not permitted

$ strace ./a.out /
...
chmod("/proc/self/fd/3", 0755)  = -1 EPERM (Operation not permitted)
close(3)= 0
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
getrandom("\xd5\xae\x46\x10\x0d\x5a\x98\x9a", 8, GRND_NONBLOCK) = 8
brk(NULL)   = 0x5575761c2000
brk(0x5575761e3000) = 0x5575761e3000
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0xd), ...}, 
AT_EMPTY_PATH) = 0
write(3, "lchmod: Operation not permitted\n", 32lchmod: Operation not permitted
) = 32
close(3)= 0
openat(AT_FDCWD, "/", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH) = 3
newfstatat(3, "", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_EMPTY_PATH) = 0
chmod("/proc/self/fd/3", 0755)  = -1 EPERM (Operation not permitted)
close(3)= 0
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0xd), ...}, 
AT_EMPTY_PATH) = 0
write(3, "fchmodat: Operation not permitte"..., 34fchmodat: Operation not 
permitted
) = 34
close(3)= 0
chmod("/", 0755)= -1 EPERM (Operation not permitted)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0xd), ...}, 
AT_EMPTY_PATH) = 0
write(3, "chmod: Operation not permitted\n", 31chmod: Operation not permitted
) = 31
close(3)= 0
openat(AT_FDCWD, "/", O_RDONLY|O_NOFOLLOW) = 3
fchmod(3, 0755) = -1 EPERM (Operation not permitted)
dup(2)  = 4
fcntl(4, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(4, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0xd), ...}, 
AT_EMPTY_PATH) = 0
write(4, "fchmod: Operation not permitted\n", 32fchmod: Operation not permitted
) = 32
close(4)= 0
exit_group(1)   = ?
+++ exited with 1 +++

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Paul Eggert 
Good, thanks for confirming that the chmod family does not have a 
similar bug.


I installed the attached workaround into coreutils master on Savannah. 
Please give it a try.From 5f971361608749e1245c5eb12096de2acd32bf83 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 1 Sep 2023 15:05:21 -0700
Subject: [PATCH] cp,mv,install: fix chown on Linux CIFS

* src/copy.c (chown_failure_ok): Also treat EACCES as OK.
---
 NEWS   |  4 
 src/copy.c | 11 +--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index c00ff0cf2..6801832f7 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,10 @@ GNU coreutils NEWS-*- outline -*-
 
 ** Bug fixes
 
+  cp, mv, install no longer issue spurious "failed to preserve ownership"
+  diagnostics when copying to GNU/Linux CIFS filesystems.  They do
+  this by working around a Linux CIFS bug.
+
   numfmt options like --suffix no longer have an arbitrary 127-byte limit.
   [bug introduced with numfmt in coreutils-8.21]
 
diff --git a/src/copy.c b/src/copy.c
index 485879525..0b3de04f3 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -3449,9 +3449,16 @@ chown_failure_ok (struct cp_options const *x)
 {
   /* If non-root uses -p, it's ok if we can't preserve ownership.
  But root probably wants to know, e.g. if NFS disallows it,
- or if the target system doesn't support file ownership.  */
+ or if the target system doesn't support file ownership.
 
-  return ((errno == EPERM || errno == EINVAL) && !x->chown_privileges);
+ Treat EACCES like EPERM and EINVAL to work around a bug in Linux
+ CIFS .  Although this means coreutils
+ will ignore EACCES errors that it should report, problems should
+ occur only when some other process is racing with coreutils and
+ coreutils is not immune to races anyway.  */
+
+  return ((errno == EPERM || errno == EINVAL || errno == EACCES)
+  && !x->chown_privileges);
 }
 
 /* Similarly, return true if it's OK for chmod and similar operations
-- 
2.39.2

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Bruno Haible 
Paul Eggert wrote:
> If you compile and run the attached 
> program on a file that you don't own (e.g., './a.out /'), does it 
> incorrectly issue "Permission denied" instead of "Operation not 
> permitted" diagnostics?

In this case, the diagnostic is "Operation not permitted" (from EPERM):

$ ./a.out /
lchown: Operation not permitted
fchownat: Operation not permitted
chown: Operation not permitted
fchown: Operation not permitted

$ strace ./a.out /
...
lchown("/", 0, 0)   = -1 EPERM (Operation not permitted)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
getrandom("\x30\x26\x33\x40\x27\x47\x25\x78", 8, GRND_NONBLOCK) = 8
brk(NULL)   = 0x55f0af43
brk(0x55f0af451000) = 0x55f0af451000
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "lchown: Operation not permitted\n", 32lchown: Operation not permitted
) = 32
close(3)= 0
fchownat(AT_FDCWD, "/", 0, 0, AT_SYMLINK_NOFOLLOW) = -1 EPERM (Operation not 
permitted)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "fchownat: Operation not permitte"..., 34fchownat: Operation not 
permitted
) = 34
close(3)= 0
chown("/", 0, 0)= -1 EPERM (Operation not permitted)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "chown: Operation not permitted\n", 31chown: Operation not permitted
) = 31
close(3)= 0
openat(AT_FDCWD, "/", O_RDONLY|O_NOFOLLOW) = 3
fchown(3, 0, 0) = -1 EPERM (Operation not permitted)
dup(2)  = 4
fcntl(4, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(4, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(4, "fchown: Operation not permitted\n", 32fchown: Operation not permitted
) = 32
close(4)= 0
exit_group(1)   = ?
+++ exited with 1 +++

Bruno

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Paul Eggert 

On 2023-09-01 08:09, Bruno Haible wrote:

It applies to all 4 variants, as can be seen from applying it to a directory:

$ ./a.out /media/nas/bruno/dir1
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied


Thanks, for coreutils it'd be helpful to now whether there's a similar 
problem with chmod-related syscalls. If you compile and run the attached 
program on a file that you don't own (e.g., './a.out /'), does it 
incorrectly issue "Permission denied" instead of "Operation not 
permitted" diagnostics?#define _GNU_SOURCE
#include 
#include 
#include 
#include 

int
main (int argc, char **argv)
{
  char const *file = argv[1] ? argv[1] : ".";
  struct stat st;
  if (lstat (file, ) < 0)
return perror ("lstat"), 1;
  mode_t mode = st.st_mode & 0;
  int status = 0;
  if (lchmod (file, mode) < 0)
perror ("lchmod"), status = 1;
  if (fchmodat (AT_FDCWD, file, mode, AT_SYMLINK_NOFOLLOW) < 0)
perror ("fchmodat"), status = 1;
  if (!S_ISLNK (st.st_mode))
{
  if (chmod (file, mode) < 0)
	perror ("chmod"), status = 1;
  int fd = openat (AT_FDCWD, file, O_RDONLY | O_NOFOLLOW);
  if (fd < 0)
	perror ("openat"), status = 1;
  else if (fchmod (fd, mode) < 0)
	perror ("fchmod"), status = 1;
}
  return status;
}

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Bruno Haible 
Paul Eggert wrote:
> it'd be helpful to know whether the bug is limited to fchownat or 
> (as I suspect) applies also to the other chown variants. Could you try 
> running the attached program on your platform, to see whether the bug 
> affects these other variants?

It applies to all 4 variants, as can be seen from applying it to a directory:

$ ./a.out /media/nas/bruno/dir1
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied

$ strace ./a.out /media/nas/bruno/dir1
...
lchown("/media/nas/bruno/dir1", 1000, 1000) = -1 EACCES (Permission denied)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
getrandom("\x31\xd2\xe1\x4b\xa3\xa9\x0a\x95", 8, GRND_NONBLOCK) = 8
brk(NULL)   = 0x56031b7c9000
brk(0x56031b7ea000) = 0x56031b7ea000
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "lchown: Permission denied\n", 26lchown: Permission denied
) = 26
close(3)= 0
fchownat(AT_FDCWD, "/media/nas/bruno/dir1", 1000, 1000, AT_SYMLINK_NOFOLLOW) = 
-1 EACCES (Permission denied)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "fchownat: Permission denied\n", 28fchownat: Permission denied
) = 28
close(3)= 0
chown("/media/nas/bruno/dir1", 1000, 1000) = -1 EACCES (Permission denied)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "chown: Permission denied\n", 25chown: Permission denied
) = 25
close(3)= 0
openat(AT_FDCWD, "/media/nas/bruno/dir1", O_RDONLY|O_NOFOLLOW) = 3
fchown(3, 1000, 1000)   = -1 EACCES (Permission denied)
dup(2)  = 4
fcntl(4, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(4, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(4, "fchown: Permission denied\n", 26fchown: Permission denied
) = 26
close(4)= 0
exit_group(1)   = ?
+++ exited with 1 +++

or to a regular file:

$ ./a.out /media/nas/bruno/dir1/file1 
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied

$ strace ./a.out /media/nas/bruno/dir1/file1 
...
lchown("/media/nas/bruno/dir1/file1", 1000, 1000) = -1 EACCES (Permission 
denied)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
getrandom("\x59\x38\xf4\xfa\x24\xc4\x89\x64", 8, GRND_NONBLOCK) = 8
brk(NULL)   = 0x56337cdfa000
brk(0x56337ce1b000) = 0x56337ce1b000
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "lchown: Permission denied\n", 26lchown: Permission denied
) = 26
close(3)= 0
fchownat(AT_FDCWD, "/media/nas/bruno/dir1/file1", 1000, 1000, 
AT_SYMLINK_NOFOLLOW) = -1 EACCES (Permission denied)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "fchownat: Permission denied\n", 28fchownat: Permission denied
) = 28
close(3)= 0
chown("/media/nas/bruno/dir1/file1", 1000, 1000) = -1 EACCES (Permission denied)
dup(2)  = 3
fcntl(3, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(3, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(3, "chown: Permission denied\n", 25chown: Permission denied
) = 25
close(3)= 0
openat(AT_FDCWD, "/media/nas/bruno/dir1/file1", O_RDONLY|O_NOFOLLOW) = 3
fchown(3, 1000, 1000)   = -1 EACCES (Permission denied)
dup(2)  = 4
fcntl(4, F_GETFL)   = 0x2 (flags O_RDWR)
newfstatat(4, "", {st_mode=S_IFCHR|0600, st_rdev=makedev(0x88, 0x9), ...}, 
AT_EMPTY_PATH) = 0
write(4, "fchown: Permission denied\n", 26fchown: Permission denied
) = 26
close(4)= 0
exit_group(1)   = ?
+++ exited with 1 +++

> Also, could you let us know the kernel and glibc version? The bug should 
> be documented in gnulib/doc, I'd think. (Plus, reported to the Linux 
> and/or glibc maintainers)

$ uname -srv
Linux 5.15.0-79-generic #86-Ubuntu SMP Mon Jul 10 16:07:21 UTC 2023
$ /lib/x86_64-linux-gnu/libc.so.6
GNU C Library (Ubuntu GLIBC 2.35-0ubuntu3.1) stable release version 2.35.
...

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-09-01 Thread Paul Eggert 
Thanks for the followup. I looked into fixing it in Gnulib and it 
appears that this would require an extra syscall (or maybe even two) per 
file when copying to a CIFS filesystem, which I'd rather avoid. So I'm 
leaning more towards working around the bug in coreutils. Before doing 
that, it'd be helpful to know whether the bug is limited to fchownat or 
(as I suspect) applies also to the other chown variants. Could you try 
running the attached program on your platform, to see whether the bug 
affects these other variants?


Also, could you let us know the kernel and glibc version? The bug should 
be documented in gnulib/doc, I'd think. (Plus, reported to the Linux 
and/or glibc maintainers)#define _GNU_SOURCE
#include 
#include 
#include 
#include 

int
main (int argc, char **argv)
{
  char const *file = argv[1] ? argv[1] : ".";
  struct stat st;
  if (lstat (file, ) < 0)
return perror ("lstat"), 1;
  int status = 0;
  if (lchown (file, st.st_uid, st.st_gid) < 0)
perror ("lchown"), status = 1;
  if (fchownat (AT_FDCWD, file, st.st_uid, st.st_gid, AT_SYMLINK_NOFOLLOW) < 0)
perror ("fchownat"), status = 1;
  if (!S_ISLNK (st.st_mode))
{
  if (chown (file, st.st_uid, st.st_gid) < 0)
	perror ("chown"), status = 1;
  int fd = openat (AT_FDCWD, file, O_RDONLY | O_NOFOLLOW);
  if (fd < 0)
	perror ("openat"), status = 1;
  else if (fchown (fd, st.st_uid, st.st_gid) < 0)
	perror ("fchown"), status = 1;
}
  return status;
}

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-08-31 Thread Bruno Haible 
Thanks for looking into this, Paul.

But there's a big misunderstanding. This is not on Android. It's on
Linux with glibc (Ubuntu 22.04, to be precise), as can be seen from the
log file:
  openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
and from the fact that I could write to /tmp.

The fchownat() failure comes from the combination of a 'cifs' mount
of the Linux kernel (mount options: 
rw,relatime,vers=1.0,cache=strict,username=bruno,uid=1000,forceuid,gid=1000,forcegid,addr=192.168.**.**,soft,unix,posixpaths,serverino,mapposix,acl,rsize=1048576,wsize=1048576,bsize=1048576,echo_interval=60,actimeo=1,closetimeo=1
 )
and the CIFS/SMB server on my NAS (an old Linux/mips machine).

> Although the attached hacky coreutils patch (which I haven't installed) 
> might solve the immediate problem

Unfortunately, this patch would also silence warnings that are justified
(e.g. when a normal user moves a directory owned by 'root', whose parent
directory has mode drwxrwxrwx). I think the diagnostic should only be
supporessed when, despite the failed fchownat() call, the destination
has the same uid and gid as the 3rd and 4th argument of the fchownat()
invocation.

> it sounds like this is something that Gnulib's fchownat

Whether this is better done in Gnulib or in coreutils/src/copy.c, I can't
judge.

Bruno

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-08-30 Thread Paul Eggert 

On 2023-08-29 11:20, Bruno Haible wrote:

People say that "chown only works for root anyway" [1]


I don't think that is the issue here. fchownat is failing with EACCES, 
which is supposed to mean that search permission is denied on a 
component of the path prefix, but in Android I guess EACCES also means 
the calling process doesn't have the required permissions to change 
ownership (i.e., errno should be EPERM according to POSIX).


Clearly search permission is allowed, since there's a successful 
utimensat call with the same file name. So EACCES does not mean search 
permission is denied; it means something else.


Do fchown, chown, and lchown have similar bugs on Android?

Although the attached hacky coreutils patch (which I haven't installed) 
might solve the immediate problem, it sounds like this is something that 
Gnulib's fchownat (and maybe related modules) should fix on Android, so 
that the fix is everywhere and not just in mv+cp.


diff --git a/src/copy.c b/src/copy.c
index b9fff03a3..7b901ffc3 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -3460,6 +3460,12 @@ chown_failure_ok (struct cp_options const *x)
  But root probably wants to know, e.g. if NFS disallows it,
  or if the target system doesn't support file ownership.  */
 
+#ifdef __ANDROID__
+  /* Work around Android bug .  */
+  if (errno == EACCES && !x->chown_privileges)
+return true;
+#endif
+
   return ((errno == EPERM || errno == EINVAL) && !x->chown_privileges);
 }

bug#65599: mv and cp give a pointless warning when moving/copying a directory

2023-08-29 Thread Bruno Haible 
When I move a directory from an ext4 file system to a CIFS v1 file system,
I get a diagnostic
  mv: failed to preserve ownership for '...': Permission denied
although the owner and group of the directory after and before the move
are the same. So, there is actually nothing to warn about.

$ mv --version
mv (GNU coreutils) 9.3.147-d553ab
Copyright (C) 2023 Free Software Foundation, Inc.
...

$ mkdir dir1
$ echo foo > dir1/file1
$ ls -ld dir1
drwxrwxr-x 2 bruno bruno 4096 Aug 26 19:38 dir1
$ strace -o /tmp/mv.log mv dir1 /media/nas/bruno/dir1
mv: failed to preserve ownership for '/media/nas/bruno/dir1': Permission denied
$ echo $?
0
$ ls -ld /media/nas/bruno/dir1
drwxrwxr-x 2 bruno bruno 0 Aug 26 19:38 /media/nas/bruno/dir1

Find attached the strace log file. The essential line is
fchownat(AT_FDCWD, "/media/nas/bruno/dir1", 1000, 1000, AT_SYMLINK_NOFOLLOW) = 
-1 EACCES (Permission denied)

People say that "chown only works for root anyway" [1], but it's certainly
safer to try the chown nevertheless, because
  - the current user might not be root but might have non-standard capabilities.
  - the target file system may be reporting failure but may do the requested
action nevertheless,
  - the target file system might map uids (think of NFS or CIFS [2]).

However, the diagnostic should be omitted if the 'mv' program can verify
that the uid and gid of the directory on the target file system is the same
as on the source file system.

Likewise for the 'cp' program:

$ cp --version
cp (GNU coreutils) 9.3.147-d553ab
...

Written by Torbjorn Granlund, David MacKenzie, and Jim Meyering.
$ mkdir dir2
$ echo foo > dir2/file2
$ ls -ld dir2
drwxrwxr-x 2 bruno bruno 4096 Aug 29 20:15 dir2
$ strace -o /tmp/cp.log cp dir2 /media/nas/bruno/dir2
$ strace -o /tmp/cp.log cp -a dir2 /media/nas/bruno/dir2
cp: failed to preserve ownership for '/media/nas/bruno/dir2': Permission denied
$ ls -ld /media/nas/bruno/dir2
drwx-- 2 bruno bruno 0 Aug 29 20:15 /media/nas/bruno/dir2

Here, in cp.log, the essential line is:
fchownat(AT_FDCWD, "/media/nas/bruno/dir2", 1000, 1000, AT_SYMLINK_NOFOLLOW) = 
-1 EACCES (Permission denied)

Bruno

[1] https://lists.gnu.org/archive/html/emacs-devel/2009-10/msg00094.html
[2] https://linux.die.net/man/8/mount.cifs
execve("/media/develdata/devel/build/coreutils-9.3.147-d553ab/build-64/src/mv", ["mv", "dir1", "/media/nas/bruno/dir1"], 0x7ffd93a100d0 /* 76 vars */) = 0
brk(NULL)   = 0xc7d000
arch_prctl(0x3001 /* ARCH_??? */, 0x7ffc663bc920) = -1 EINVAL (Invalid argument)
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f3771429000
access("/etc/ld.so.preload", R_OK)  = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=130427, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 130427, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f3771409000
close(3)= 0
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libselinux.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0\0\0\0\0\0\0\0"..., 832) = 832
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=166280, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 177672, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f37713dd000
mprotect(0x7f37713e3000, 139264, PROT_NONE) = 0
mmap(0x7f37713e3000, 106496, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x6000) = 0x7f37713e3000
mmap(0x7f37713fd000, 28672, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2) = 0x7f37713fd000
mmap(0x7f3771405000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x27000) = 0x7f3771405000
mmap(0x7f3771407000, 5640, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f3771407000
close(3)= 0
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libacl.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0\0\0\0\0\0\0\0"..., 832) = 832
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=34888, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 36896, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f37713d3000
mprotect(0x7f37713d5000, 24576, PROT_NONE) = 0
mmap(0x7f37713d5000, 16384, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2000) = 0x7f37713d5000
mmap(0x7f37713d9000, 4096, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x6000) = 0x7f37713d9000
mmap(0x7f37713db000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x7000) = 0x7f37713db000
close(3)= 0
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libattr.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0\0\0\0\0\0\0\0"..., 832) = 832
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=26696, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 28696, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f37713cb000
mmap(0x7f37713cd000, 12288, PROT_READ|PROT_EXEC,