bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
On 12/12/21 09:06, Sudhip Nashi wrote:
Thanks, I think I see the problem now. Please try the attached patch; I haven't
tested myself as I lack access to macOS. Thanks.
<0001-renameatu-port-to-macOS-tmpfs.patch>
It looks like this patch solves the problem.
Thanks, I installed the attached to implement this fix, the first patch
into Gnulib, the second into coreutils.From dd474e50930ea00910631eb1b77ff4270d7b02c0 Mon Sep 17 00:00:00 2001
From: Paul Eggert
Date: Tue, 14 Dec 2021 12:32:30 -0800
Subject: [PATCH] renameatu: port to macOS tmpfs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Problem reported by Sudhip Nashi (Bug#52193).
* lib/renameatu.c (renameat2ish) [HAVE_RENAMEAT]: New function.
(renameatu): Use the new function, to avoid a bug when
renameatx_np fails with errno == ENOTSUP. Don’t try to support
RENAME_EXCHANGE; the old code didn’t work and nobody using using
RENAME_EXCHANGE anyway.
---
ChangeLog | 10 +++
lib/renameatu.c | 69 +
2 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 0e20dcb58..370cd9839 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2021-12-14 Paul Eggert
+
+ renameatu: port to macOS tmpfs
+ Problem reported by Sudhip Nashi (Bug#52193).
+ * lib/renameatu.c (renameat2ish) [HAVE_RENAMEAT]: New function.
+ (renameatu): Use the new function, to avoid a bug when
+ renameatx_np fails with errno == ENOTSUP. Don’t try to support
+ RENAME_EXCHANGE; the old code didn’t work and nobody using using
+ RENAME_EXCHANGE anyway.
+
2021-12-12 Bruno Haible
gnulib-tool: Support non-recursive-gnulib-prefix-hack with tests.
diff --git a/lib/renameatu.c b/lib/renameatu.c
index 38438a4ef..b75f95269 100644
--- a/lib/renameatu.c
+++ b/lib/renameatu.c
@@ -61,6 +61,29 @@ rename_noreplace (char const *src, char const *dst)
#undef renameat
+#if HAVE_RENAMEAT
+
+/* Act like renameat (FD1, SRC, FD2, DST), except fail with EEXIST if
+ FLAGS is nonzero and it is easy to fail atomically if DST already exists.
+ This lets renameatu be atomic when it can be implemented in terms
+ of renameatx_np. */
+static int
+renameat2ish (int fd1, char const *src, int fd2, char const *dst,
+ unsigned int flags)
+{
+# ifdef RENAME_EXCL
+ if (flags)
+{
+ int r = renameatx_np (fd1, src, fd2, dst, RENAME_EXCL);
+ if (r == 0 || errno != ENOTSUP)
+return r;
+}
+# endif
+
+ return renameat (fd1, src, fd2, dst);
+}
+#endif
+
/* Rename FILE1, in the directory open on descriptor FD1, to FILE2, in
the directory open on descriptor FD2. If possible, do it without
changing the working directory. Otherwise, resort to using
@@ -93,9 +116,6 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
#if HAVE_RENAMEAT
{
-# if defined RENAME_EXCL/* macOS */
- unsigned int uflags;
-# endif
size_t src_len;
size_t dst_len;
char *src_temp = (char *) src;
@@ -107,23 +127,12 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
struct stat dst_st;
bool dst_found_nonexistent = false;
- /* Check the flags. */
-# if defined RENAME_EXCL
- /* We can support RENAME_EXCHANGE and RENAME_NOREPLACE. */
- if (flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
-# else
- /* RENAME_NOREPLACE is the only flag currently supported. */
- if (flags & ~RENAME_NOREPLACE)
-# endif
-return errno_fail (ENOTSUP);
-
-# if defined RENAME_EXCL
- uflags = ((flags & RENAME_EXCHANGE ? RENAME_SWAP : 0)
-| (flags & RENAME_NOREPLACE ? RENAME_EXCL : 0));
-# endif
-
- if ((flags & RENAME_NOREPLACE) != 0)
+ switch (flags)
{
+case 0:
+ break;
+
+case RENAME_NOREPLACE:
/* This has a race between the call to lstatat and the calls to
renameat below. This lstatat is needed even if RENAME_EXCL
is defined, because RENAME_EXCL is buggy on macOS 11.2:
@@ -134,26 +143,22 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
if (errno != ENOENT)
return -1;
dst_found_nonexistent = true;
+ break;
+
+default:
+ return errno_fail (ENOTSUP);
}
/* Let strace see any ENOENT failure. */
src_len = strlen (src);
dst_len = strlen (dst);
if (!src_len || !dst_len)
-# if defined RENAME_EXCL
-return renameatx_np (fd1, src, fd2, dst, uflags);
-# else
-return renameat (fd1, src, fd2, dst);
-# endif
+return renameat2ish (fd1, src, fd2, dst, flags);
src_slash = src[src_len - 1] == '/';
dst_slash = dst[dst_len - 1] == '/';
if (!src_slash && !dst_slash)
-# if defined RENAME_EXCL
-return renameatx_np (fd1, src, fd2, dst, uflags);
-# else
-return renameat (fd1, src, fd2, dst);
-# endif
+return renameat2ish (fd1, src, fd2, dst, flags);
/* Presence of a trailing slash requires directory semantics. If
the source does not exist, or if the destination c
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
> Thanks, I think I see the problem now. Please try the attached patch; I > haven't tested myself as I lack access to macOS. Thanks. > <0001-renameatu-port-to-macOS-tmpfs.patch> It looks like this patch solves the problem.
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
On 12/8/21 16:08, Sudhip Nashi wrote:
The issue is that all `mv` operations within the mountpoint fail with an
ENOTSUPP error.
Thanks, I think I see the problem now. Please try the attached patch; I
haven't tested myself as I lack access to macOS. Thanks.
From 9fe50d31aa75dd2d896225c2d1993f0b8eef2f26 Mon Sep 17 00:00:00 2001
From: Paul Eggert
Date: Wed, 8 Dec 2021 19:14:25 -0800
Subject: [PATCH] renameatu: port to macOS tmpfs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Problem reported by Sudhip Nashi (Bug#52193).
* lib/renameatu.c (renameat2ish) [HAVE_RENAMEAT]: New function.
(renameatu): Use the new function, to avoid a bug when
renameatx_np fails with errno == ENOTSUP. Don’t try to support
RENAME_EXCHANGE; the old code didn’t work and nobody using using
RENAME_EXCHANGE anyway.
---
ChangeLog | 10 +++
lib/renameatu.c | 69 +
2 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index bb3edbe44a..413a090749 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2021-12-08 Paul Eggert
+
+ renameatu: port to macOS tmpfs
+ Problem reported by Sudhip Nashi (Bug#52193).
+ * lib/renameatu.c (renameat2ish) [HAVE_RENAMEAT]: New function.
+ (renameatu): Use the new function, to avoid a bug when
+ renameatx_np fails with errno == ENOTSUP. Don’t try to support
+ RENAME_EXCHANGE; the old code didn’t work and nobody using using
+ RENAME_EXCHANGE anyway.
+
2021-12-07 Paul Eggert
regex: pacify Coverity clean_state_log_if_needed
diff --git a/lib/renameatu.c b/lib/renameatu.c
index 38438a4ef1..b75f95269f 100644
--- a/lib/renameatu.c
+++ b/lib/renameatu.c
@@ -61,6 +61,29 @@ rename_noreplace (char const *src, char const *dst)
#undef renameat
+#if HAVE_RENAMEAT
+
+/* Act like renameat (FD1, SRC, FD2, DST), except fail with EEXIST if
+ FLAGS is nonzero and it is easy to fail atomically if DST already exists.
+ This lets renameatu be atomic when it can be implemented in terms
+ of renameatx_np. */
+static int
+renameat2ish (int fd1, char const *src, int fd2, char const *dst,
+ unsigned int flags)
+{
+# ifdef RENAME_EXCL
+ if (flags)
+{
+ int r = renameatx_np (fd1, src, fd2, dst, RENAME_EXCL);
+ if (r == 0 || errno != ENOTSUP)
+return r;
+}
+# endif
+
+ return renameat (fd1, src, fd2, dst);
+}
+#endif
+
/* Rename FILE1, in the directory open on descriptor FD1, to FILE2, in
the directory open on descriptor FD2. If possible, do it without
changing the working directory. Otherwise, resort to using
@@ -93,9 +116,6 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
#if HAVE_RENAMEAT
{
-# if defined RENAME_EXCL/* macOS */
- unsigned int uflags;
-# endif
size_t src_len;
size_t dst_len;
char *src_temp = (char *) src;
@@ -107,23 +127,12 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
struct stat dst_st;
bool dst_found_nonexistent = false;
- /* Check the flags. */
-# if defined RENAME_EXCL
- /* We can support RENAME_EXCHANGE and RENAME_NOREPLACE. */
- if (flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
-# else
- /* RENAME_NOREPLACE is the only flag currently supported. */
- if (flags & ~RENAME_NOREPLACE)
-# endif
-return errno_fail (ENOTSUP);
-
-# if defined RENAME_EXCL
- uflags = ((flags & RENAME_EXCHANGE ? RENAME_SWAP : 0)
-| (flags & RENAME_NOREPLACE ? RENAME_EXCL : 0));
-# endif
-
- if ((flags & RENAME_NOREPLACE) != 0)
+ switch (flags)
{
+case 0:
+ break;
+
+case RENAME_NOREPLACE:
/* This has a race between the call to lstatat and the calls to
renameat below. This lstatat is needed even if RENAME_EXCL
is defined, because RENAME_EXCL is buggy on macOS 11.2:
@@ -134,26 +143,22 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
if (errno != ENOENT)
return -1;
dst_found_nonexistent = true;
+ break;
+
+default:
+ return errno_fail (ENOTSUP);
}
/* Let strace see any ENOENT failure. */
src_len = strlen (src);
dst_len = strlen (dst);
if (!src_len || !dst_len)
-# if defined RENAME_EXCL
-return renameatx_np (fd1, src, fd2, dst, uflags);
-# else
-return renameat (fd1, src, fd2, dst);
-# endif
+return renameat2ish (fd1, src, fd2, dst, flags);
src_slash = src[src_len - 1] == '/';
dst_slash = dst[dst_len - 1] == '/';
if (!src_slash && !dst_slash)
-# if defined RENAME_EXCL
-return renameatx_np (fd1, src, fd2, dst, uflags);
-# else
-return renameat (fd1, src, fd2, dst);
-# endif
+return renameat2ish (fd1, src, fd2, dst, flags);
/* Presence of a trailing slash requires directory semantics. If
the source does not exist, or if the destination cannot be turned
@@ -226,11 +231,7 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
on Solaris, since all ot
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
> Thanks, but as I don't have access to a macOS machine I don't know what "the
> issue" is. What are the symptoms visible to the user? Can you do the
> equivalent of an strace to show us what system calls are being executed?
>
> As near as I can make out, on macOS mv should be doing the equivalent of the
> following:
>
> sfd = open ("source", O_RDONLY | O_NOFOLLOW);
> if (fclonefileat (sfd, AT_FDCWD, "destination", 0) != 0)
> {
>dfd = open ("destination", O_WRONLY | O_CREAT | O_EXCL, mode);
>next_start = lseek (sfd, 0, SEEK_DATA);
>...
>
> and evidently something is going wrong at or after the "...". What is it?
Ah, my bad. The issue is that all `mv` operations within the mountpoint fail
with an ENOTSUPP error. Here’s a dtruss for an example:
access("/AppleInternal/XBS/.isChrooted\0", 0x0, 0x0)= -1 Err#2
bsdthread_register(0x1AEC802C8, 0x1AEC802BC, 0x4000)= 1073742303 0
shm_open(0x1AEB48F55, 0x0, 0x45E4000) = 3 0
fstat64(0x3, 0x16B81A100, 0x0) = 0 0
mmap(0x0, 0x4000, 0x1, 0x40001, 0x3, 0x0) = 0x104718000 0
close(0x3) = 0 0
ioctl(0x2, 0x4004667A, 0x16B81A1AC) = 0 0
mprotect(0x104724000, 0x4000, 0x0) = 0 0
mprotect(0x10473, 0x4000, 0x0) = 0 0
mprotect(0x104734000, 0x4000, 0x0) = 0 0
mprotect(0x10474, 0x4000, 0x0) = 0 0
mprotect(0x104744000, 0x4000, 0x0) = 0 0
mprotect(0x10475, 0x4000, 0x0) = 0 0
mprotect(0x10471C000, 0x90, 0x1)= 0 0
mprotect(0x10471C000, 0x90, 0x3)= 0 0
mprotect(0x10471C000, 0x90, 0x1)= 0 0
mprotect(0x104754000, 0x4000, 0x1) = 0 0
mprotect(0x104758000, 0x90, 0x1)= 0 0
mprotect(0x104758000, 0x90, 0x3)= 0 0
mprotect(0x104758000, 0x90, 0x1)= 0 0
mprotect(0x10471C000, 0x90, 0x3)= 0 0
mprotect(0x10471C000, 0x90, 0x1)= 0 0
mprotect(0x104754000, 0x4000, 0x3) = 0 0
mprotect(0x104754000, 0x4000, 0x1) = 0 0
objc_bp_assist_cfg_np(0x1AEB103C0, 0x80201048, 0x0) = -1
Err#5
issetugid(0x0, 0x0, 0x0)= 0 0
getentropy(0x16B819FC8, 0x20, 0x0) = 0 0
getentropy(0x16B81A018, 0x40, 0x0) = 0 0
getpid(0x0, 0x0, 0x0) = 1135 0
stat64("/AppleInternal\0", 0x16B81A710, 0x0)= -1 Err#2
csops_audittoken(0x46F, 0x7, 0x16B81A240) = 0 0
proc_info(0x2, 0x46F, 0xD) = 64 0
csops_audittoken(0x46F, 0x7, 0x16B81A300) = 0 0
sysctlbyname(kern.osvariant_status, 0x15, 0x16B81A778, 0x16B81A770, 0x0)
= 0 0
csops(0x46F, 0x0, 0x16B81A79C) = 0 0
mprotect(0x10461, 0x10, 0x1)= 0 0
open_nocancel("/usr/share/locale/en_US.UTF-8/LC_COLLATE\0", 0x0, 0x0)
= 3 0
fcntl_nocancel(0x3, 0x3, 0x0) = 0 0
getrlimit(0x1008, 0x16B81B148, 0x0) = 0 0
fstat64(0x3, 0x16B81B0C0, 0x0) = 0 0
read_nocancel(0x3, "1.1A\n\0", 0x1000) = 2086 0
close_nocancel(0x3) = 0 0
open_nocancel("/usr/share/locale/en_US.UTF-8/LC_CTYPE\0", 0x0, 0x0)
= 3 0
fcntl_nocancel(0x3, 0x3, 0x0) = 0 0
fstat64(0x3, 0x16B81B1F0, 0x0) = 0 0
fstat64(0x3, 0x16B81AFE0, 0x0) = 0 0
lseek(0x3, 0x0, 0x1)= 0 0
lseek(0x3, 0x0, 0x0)= 0 0
read_nocancel(0x3, "RuneMagAUTF-8\0", 0x1000) = 4096 0
read_nocancel(0x3, "\0", 0x1000)= 4096 0
read_nocancel(0x3, "\0", 0x1000)= 4096 0
read_nocancel(0x3, "\0", 0x1000)= 4096 0
read_nocancel(0x3, "\0", 0x1000)= 4096 0
read_nocancel(0x3, "\0", 0x1000)= 4096 0
read_nocancel(0x3, "\0", 0x1000)= 4096 0
read_nocancel(0x3, "@\004\211\0", 0xF5D0) = 62928 0
close_nocancel(0x3) = 0 0
open_nocancel("/usr/share/locale/en_US.UTF-8/LC_MONETARY\0", 0x0, 0x0)
= 3 0
fstat64(0x3, 0x16B81B210, 0x0) = 0 0
read_nocancel(0x3, "USD \n$\n.\n,\n3;3\n\n-\n2\n2\n1\n0\n1\n0\n1\n1\n(\0",
0x22)= 34 0
close_nocancel(0x3) = 0 0
open_nocancel("/usr/share/locale/en_US.UTF-8/LC_NUMERIC\0", 0x0, 0x0)
= 3 0
fstat64(0x3, 0x16B81B210, 0x0) = 0 0
read_nocancel(0x3, ".\n,\n3;3\n@$\b\0", 0x8)= 8 0
close_nocancel(0x3) = 0 0
open_nocancel("/usr/share/locale/en_US.UTF-8/LC_TIME\0", 0x0, 0x0)
= 3 0
fstat64(0x3, 0x16B81B220, 0x0) = 0 0
read_nocancel(0x3,
"Jan\nFeb\nMar\nApr\nMay\nJun\nJul\nAug\nSep\nOct\nNov\nDec\nJanuary\nFebruary\nMarch\nApril\nMay\nJune\nJuly\nAugust\nSeptember\nOctober\nNovember\nDecember\nSun\nMon\nTue\nWed\nThu\nFri\nSat\nSunday\nMonday\nTuesday\nWednesday\nThursday\nFriday\nSaturday\n%H:%M:%S\n%m/%d/%Y\n%a
%b %e %X %Y\nAM\nP", 0x179) = 377 0
close_nocancel(0x3) = 0
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
On 12/6/21 20:40, Sudhip Nashi wrote:
The issue should be reproducible when moving directories and files within
~/mntpnt, but not between that filesystem and the parent APFS one, and vice
versa.
Thanks, but as I don't have access to a macOS machine I don't know what
"the issue" is. What are the symptoms visible to the user? Can you do
the equivalent of an strace to show us what system calls are being executed?
As near as I can make out, on macOS mv should be doing the equivalent of
the following:
sfd = open ("source", O_RDONLY | O_NOFOLLOW);
if (fclonefileat (sfd, AT_FDCWD, "destination", 0) != 0)
{
dfd = open ("destination", O_WRONLY | O_CREAT | O_EXCL, mode);
next_start = lseek (sfd, 0, SEEK_DATA);
...
and evidently something is going wrong at or after the "...". What is it?
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
> Can you give a way to reproduce the probglem on a macOS system, assuming one > has a tmpfs filesystem /A and a non-tmpfs filesystem /B? The scenario wasn't > entirely clear from the bug report. Running the command “mount_tmpfs ~/mntpnt” mounts a tmpfs (and non CoW) filesystem at ~/mntpnt. The issue should be reproducible when moving directories and files within ~/mntpnt, but not between that filesystem and the parent APFS one, and vice versa.
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
On 12/5/21 12:21, Sudhip Nashi wrote: it doesn’t look like the latest commit (the tarball you sent me) fixes the issue. Can you give a way to reproduce the probglem on a macOS system, assuming one has a tmpfs filesystem /A and a non-tmpfs filesystem /B? The scenario wasn't entirely clear from the bug report.
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
> macOS is different in that /tmp isn’t actually on a tmpfs filesystem, but is > rather just a normal directory on the main data volume that is cleaned on > boot. One has to manually mount a tmpfs volume themselves with mount_tmpfs. > Anyways, I’ll try the tarball linked and see if the issue still persists. Sorry for the late response, but it doesn’t look like the latest commit (the tarball you sent me) fixes the issue.
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
> I don't observe this problem with the latest development version of > coreutils. Here's how I tried to reproduce the problem on macOS 11.3.1 20E241: > > % touch /tmp/ > % src/mv /tmp/ /tmp/ > > and it worked OK. macOS is different in that /tmp isn’t actually on a tmpfs filesystem, but is rather just a normal directory on the main data volume that is cleaned on boot. One has to manually mount a tmpfs volume themselves with mount_tmpfs. Anyways, I’ll try the tarball linked and see if the issue still persists.
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
On 11/29/21 16:50, Sudhip Nashi via GNU coreutils Bug Reports wrote: It appears that in coreutils 9.0 and greater, the mv command is broken on non-APFS filesystems on macOS. For example, on Apple tmpfs, it fails whenever moving a file with ENOTSUPP. I don't observe this problem with the latest development version of coreutils. Here's how I tried to reproduce the problem on macOS 11.3.1 20E241: % touch /tmp/ % src/mv /tmp/ /tmp/ and it worked OK. I just put a copy of the latest coreutils source here: https://www.cs.ucla.edu/~eggert/coreutils-9.0.36-5e36c.tar.gz Could you give it a try? Possibly this is related to the other bug report you mention, since mv falls back on cp's algorithm. But if so, I hope that the recent fix to cp also fixed mv.
bug#52193: mv broken on non-APFS filesystems on macOS on coreutils >= 9.0
Hello, It appears that in coreutils 9.0 and greater, the mv command is broken on non-APFS filesystems on macOS. For example, on Apple tmpfs, it fails whenever moving a file with ENOTSUPP. It works fine on APFS filesystems (which is the primary macOS filesystem), however, so I assume it slipped under the radar till now. Could this possibly be related to the previous cp problem (#51857)? Thanks, Sudhip
