It would also be good to add a test for other races, but I still need
to think of the best way to do it. Perhaps for the rename one, a long
file name could maybe reproduce it consistently, since it calls
renameatu N times where N is the length of the file name.
The chmod one can probably only be done with a breakpoint or something
like that.
-- 8< --
* src/shred.c (wipefile): When --force or --remove are in use, and the
file is in a subdirectory, open a file descriptor to its parent and
perform operations on the basename relative to it.
(wipename): Add a file descriptor argument to the parent directory. Open
a readable file descriptor to the directory if we need to sync it. Use
the file descriptor with renameatu and unlinkat.
* tests/shred/unlink-race.sh: New file.
* tests/local.mk (all_tests): Add the new test case.
* NEWS: Mention the bug fixes.
Discovered by Anthropic's OSS Scanner, as bug ANT-2026-O4Q4UVP2.
---
NEWS | 11 +++++
src/shred.c | 82 +++++++++++++++++++++++++++-----------
tests/local.mk | 1 +
tests/shred/unlink-race.sh | 59 +++++++++++++++++++++++++++
4 files changed, 130 insertions(+), 23 deletions(-)
create mode 100755 tests/shred/unlink-race.sh
diff --git a/NEWS b/NEWS
index 6b4ed1d7b..5ad3dba01 100644
--- a/NEWS
+++ b/NEWS
@@ -141,6 +141,17 @@ GNU coreutils NEWS -*-
outline -*-
Also negative operands like "-,5" are no longer treated as options.
[bugs introduced in coreutils-8.24 and coreutils-8.31]
+ 'shred --force' no longer has races when opening non-writable files.
+ Previously, upon failing to open a writable file descriptor, it would
+ change its permissions before reopening it, following symbolic links in the
+ leading path components each time.
+ [This bug was present in "the beginning".]
+
+ 'shred --remove' no longer has races when renaming or unlinking files.
+ Previously, symbolic links in the leading path components were followed each
+ time the file was renamed and/or unlinked.
+ [This bug was present in "the beginning".]
+
'sort --compress-program=PROG' no longer treats an unset PATH environment
variable as equivalent to a PATH environment variable defined to an empty
string. On typical platforms, that would result in PROG being executed from
diff --git a/src/shred.c b/src/shred.c
index d96fefc78..4271412ce 100644
--- a/src/shred.c
+++ b/src/shred.c
@@ -1058,18 +1058,21 @@ incname (char *name, size_t len)
* file system with synchronous metadata updates, this is unnecessary.
*/
static bool
-wipename (char *oldname, char const *qoldname, struct Options const *flags)
+wipename (int const dirfd, char *oldname, char const *qoldname,
+ struct Options const *flags)
{
char *newname = xstrdup (oldname);
char *base = last_component (newname);
+ char *oldbase = oldname + (base - newname);
char *dir = dir_name (newname);
char *qdir = xstrdup (quotef (dir));
bool first = true;
bool ok = true;
- int dir_fd = -1;
+ int readable_dirfd = -1;
if (flags->remove_file == remove_wipesync)
- dir_fd = open (dir, O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
+ readable_dirfd = openat (dirfd, ".",
+ O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
if (flags->verbose)
error (0, 0, _("%s: removing"), qoldname);
@@ -1080,14 +1083,14 @@ wipename (char *oldname, char const *qoldname, struct
Options const *flags)
memset (base, nameset[0], len);
base[len] = 0;
bool rename_ok;
- while (! (rename_ok = (renameatu (AT_FDCWD, oldname, AT_FDCWD, newname,
+ while (! (rename_ok = (renameatu (dirfd, oldbase, dirfd, base,
RENAME_NOREPLACE)
== 0))
&& errno == EEXIST && incname (base, len))
continue;
if (rename_ok)
{
- if (0 <= dir_fd && dosync (dir_fd, qdir) != 0)
+ if (0 <= readable_dirfd && dosync (readable_dirfd, qdir) != 0)
ok = false;
if (flags->verbose)
{
@@ -1100,22 +1103,22 @@ wipename (char *oldname, char const *qoldname, struct
Options const *flags)
_("%s: renamed to %s"), old, newname);
first = false;
}
- memcpy (oldname + (base - newname), base, len + 1);
+ memcpy (oldbase, base, len + 1);
}
}
- if (unlink (oldname) != 0)
+ if (unlinkat (dirfd, oldbase, 0) < 0)
{
error (0, errno, _("%s: failed to remove"), qoldname);
ok = false;
}
else if (flags->verbose)
error (0, 0, _("%s: removed"), qoldname);
- if (0 <= dir_fd)
+ if (0 <= readable_dirfd)
{
- if (dosync (dir_fd, qdir) != 0)
+ if (dosync (readable_dirfd, qdir) != 0)
ok = false;
- if (close (dir_fd) != 0)
+ if (close (readable_dirfd) < 0)
{
error (0, errno, _("%s: failed to close"), qdir);
ok = false;
@@ -1143,11 +1146,38 @@ static bool
wipefile (char *name, char const *qname,
struct randint_source *s, struct Options const *flags)
{
- int fd = open (name, O_WRONLY | O_NOCTTY | O_NONBLOCK | O_BINARY);
+
+ int dirfd = AT_FDCWD;
+ char const *relname = name;
+ char const *base;
+
+ /* If we are using --force, we may need to change the permissions of NAME.
+ If we are using --remove, we will need to unlink or rename NAME.
+ In both cases, avoid following leading symbolic links, which may change
+ as 'shred' is executing. */
+ if ((flags->force || flags->remove_file)
+ && (base = last_component (name)) != name && *base)
+ {
+ char *dir = dir_name (name);
+
+ dirfd = open (dir, O_PATHSEARCH | O_DIRECTORY);
+ free (dir);
+
+ if (dirfd < 0)
+ {
+ error (0, errno, _("%s: failed to open for writing"), qname);
+ return false;
+ }
+
+ relname = base;
+ }
+
+ bool ok = true;
+ int fd = openat (dirfd, relname, O_WRONLY | O_NOCTTY | O_NONBLOCK |
O_BINARY);
if (fd < 0
&& (errno == EACCES && flags->force)
- && chmod (name, S_IWUSR) == 0)
- fd = open (name, O_WRONLY | O_NOCTTY | O_NONBLOCK | O_BINARY);
+ && fchmodat (dirfd, relname, S_IWUSR, 0) == 0)
+ fd = openat (dirfd, relname, O_WRONLY | O_NOCTTY | O_NONBLOCK | O_BINARY);
if (fd < 0)
{
struct stat st;
@@ -1155,30 +1185,36 @@ wipefile (char *name, char const *qname,
/* Try to give a more meaningful error message if we attempt to
open a FIFO with no readers. */
- if (open_errno == ENXIO && 0 <= stat (name, &st)
+ if (open_errno == ENXIO && 0 <= fstatat (dirfd, relname, &st, 0)
&& S_ISFIFO (st.st_mode))
error (0, 0, _("%s: invalid file type"), qname);
else
error (0, open_errno, _("%s: failed to open for writing"), qname);
- return false;
+ ok = false;
+ goto done;
}
/* We used O_NONBLOCK to prevent blocking when opening a FIFO.
Reset that here. */
int fd_flags = fcntl (fd, F_GETFL);
- bool ok = 0 <= fd_flags && 0 <= fcntl (fd, F_SETFL, fd_flags & ~O_NONBLOCK);
- if (ok)
- ok = do_wipefd (fd, qname, s, flags);
- else
- error (0, errno, _("couldn't reset non-blocking mode %s"), qname);
+ if (fd_flags < 0 || fcntl (fd, F_SETFL, fd_flags & ~O_NONBLOCK) < 0)
+ {
+ error (0, errno, _("couldn't reset non-blocking mode %s"), qname);
+ ok = false;
+ goto done;
+ }
+
+ ok = (do_wipefd (fd, qname, s, flags)
+ && (! flags->remove_file || wipename (dirfd, name, qname, flags)));
- if (close (fd) != 0)
+ done:
+ if (0 <= fd && close (fd) < 0)
{
error (0, errno, _("%s: failed to close"), qname);
ok = false;
}
- if (ok && flags->remove_file)
- ok = wipename (name, qname, flags);
+ if (dirfd != AT_FDCWD)
+ close (dirfd);
return ok;
}
diff --git a/tests/local.mk b/tests/local.mk
index 77cb83439..840c8118c 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -452,6 +452,7 @@ all_tests = \
tests/shred/shred-passes.sh \
tests/shred/shred-remove.sh \
tests/shred/shred-size.sh \
+ tests/shred/unlink-race.sh \
tests/shuf/shuf.sh \
tests/shuf/shuf-reservoir.sh \
tests/misc/sleep.sh \
diff --git a/tests/shred/unlink-race.sh b/tests/shred/unlink-race.sh
new file mode 100755
index 000000000..8ccb3a7e3
--- /dev/null
+++ b/tests/shred/unlink-race.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+# Test for races when 'shred' removes the file.
+
+# Copyright (C) 2026 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ shred
+
+mkfifo_or_skip_ fifo
+
+# Setup d1/f, which will be removed, and d2/f,
+# which will remain if there are no races.
+mkdir d1 d2 &&
+echo deleted >d1/f &&
+echo saved >d2/f &&
+cp d2/f exp-saved || framework_failure_
+
+# Make sure the 'shred' process gets cleaned up.
+cleanup_() { kill "$pid" 2>/dev/null; wait "$pid" 2>/dev/null; pid=; }
+
+# Use the FIFO as the random source. After opening d1/f1, it
+# will wait for bytes from the FIFO during its random pass.
+shred -v -x -n1 -u --random-source=fifo d1/f 2>err & pid=$!
+exec 3>fifo || framework_failure_
+
+# Once the FIFO is opened by a reader, 'shred' will begin its random pass.
+opened_() { sleep $1; grep 'pass 1/1 (random)\.\.\.$' err >/dev/null; }
+retry_delay_ opened_ .1 6 || framework_failure_
+
+# Move the directory containing the file being overwritten,
+# and create a symbolic link in its place.
+mv d1 d1.orig && ln -s d2 d1 || framework_failure_
+
+# Fill the random source to allow 'shred' to complete.
+timeout 10 cat /dev/zero >&3
+exec 3>&- || framework_failure_
+
+wait "$pid" || fail=1
+pid=
+
+# Verify that the 'shred' removed the file from the original
+# directory and not the one now referenced by the symbolic link.
+compare exp-saved d2/f || fail=1
+test -f d1.orig/f && fail=1
+
+Exit $fail
--
2.56.0
