CC-tea/coreutils
1
0
Fork 0
mirror of git://git.sv.gnu.org/coreutils.git synced 2026-02-12 02:12:06 +02:00
Code Issues Projects Releases Wiki Activity

ln: avoid directory hard-link races

Browse Source 
Previously, 'ln A B' did 'stat("B"), lstat("A"), link("A","B")'
where the stat and lstat were necessary to avoid hard-linking
directories on systems that can hard-link directories.
Now, in situations that prohibit hard links to directories,
'ln A B' merely does 'link("A","B")'.  The new behavior
avoids some races and should be more efficient.
This patch was inspired by Bug#10020, which was about 'ln'.
* bootstrap.conf (gnulib_modules): Add unlinkdir.
* src/force-link.c (force_linkat, force_symlinkat): New arg for
error number of previous try.  Return error number, 0, or -1 if
error, success, or success after removal.  All callers changed.
* src/ln.c: Include priv-set.h, unlinkdir.h.
(beware_hard_dir_link): New static var.
(errnoize, atomic_link): New functions.
(target_directory_operand): Use errnoize for simplicity.
(do_link): New arg for error number of previous try.  All callers
changed.  Do each link atomically if possible.
(main): Do -r check earlier.  Remove linkdir privileges so we can
use a single linkat/symlinkat instead of a racy substitute for the
common case of 'ln A B' and 'ln -s A B'.  Set beware_hard_dir_link
to disable this optimization.
This commit is contained in:
Paul Eggert
2018-10-19 12:19:43 -07:00
parent cbc427485e
commit 571f63f501
6 changed files with 244 additions and 194 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
NEWS
View File

@@ -10,6 +10,12 @@ GNU coreutils NEWS -*- outline -*-
df no longer corrupts displayed multibyte characters on macOS.
When possible 'ln A B' now merely links A to B and reports an error
if this fails, instead of statting A and B before linking. This
uses fewer system calls and avoids some races. The old statting
approach is still used in situations where hard links to directories
are allowed (e.g., NetBSD when superuser).
** New features
id now supports specifying multiple users.

1
bootstrap.conf
View File

@@ -257,6 +257,7 @@ gnulib_modules="
unistd-safer
unlink-busy
unlinkat
unlinkdir
unlocked-io
unsetenv
update-copyright

30
src/copy.c
View File

@@ -1783,16 +1783,16 @@ static bool
create_hard_link (char const *src_name, char const *dst_name,
bool replace, bool verbose, bool dereference)
{
int status = force_linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name,
dereference ? AT_SYMLINK_FOLLOW : 0,
replace);
if (status < 0)
int err = force_linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name,
dereference ? AT_SYMLINK_FOLLOW : 0,
replace, -1);
if (0 < err)
{
error (0, errno, _("cannot create hard link %s to %s"),
error (0, err, _("cannot create hard link %s to %s"),
quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
return false;
}
if (0 < status && verbose)
if (err < 0 && verbose)
printf (_("removed %s\n"), quoteaf (dst_name));
return true;
}
@@ -2630,11 +2630,12 @@ copy_internal (char const *src_name, char const *dst_name,
goto un_backup;
}
}
if (force_symlinkat (src_name, AT_FDCWD, dst_name,
x->unlink_dest_after_failed_open)
< 0)
int err = force_symlinkat (src_name, AT_FDCWD, dst_name,
x->unlink_dest_after_failed_open, -1);
if (0 < err)
{
error (0, errno, _("cannot create symbolic link %s to %s"),
error (0, err, _("cannot create symbolic link %s to %s"),
quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
goto un_backup;
}
@@ -2713,10 +2714,9 @@ copy_internal (char const *src_name, char const *dst_name,
goto un_backup;
}
int symlink_r = force_symlinkat (src_link_val, AT_FDCWD, dst_name,
x->unlink_dest_after_failed_open);
int symlink_err = symlink_r < 0 ? errno : 0;
if (symlink_err && x->update && !new_dst && S_ISLNK (dst_sb.st_mode)
int symlink_err = force_symlinkat (src_link_val, AT_FDCWD, dst_name,
x->unlink_dest_after_failed_open, -1);
if (0 < symlink_err && x->update && !new_dst && S_ISLNK (dst_sb.st_mode)
&& dst_sb.st_size == strlen (src_link_val))
{
/* See if the destination is already the desired symlink.
@@ -2733,7 +2733,7 @@ copy_internal (char const *src_name, char const *dst_name,
}
}
free (src_link_val);
if (symlink_err)
if (0 < symlink_err)
{
error (0, symlink_err, _("cannot create symbolic link %s"),
quoteaf (dst_name));

54
src/force-link.c
View File

@@ -85,22 +85,27 @@ try_link (char *dest, void *arg)
/* Hard-link directory SRCDIR's file SRCNAME to directory DSTDIR's
file DSTNAME, using linkat-style FLAGS to control the linking.
If FORCE and DSTNAME already exists, replace it atomically. Return
1 if successful and DSTNAME already existed,
If FORCE and DSTNAME already exists, replace it atomically.
If LINKAT_ERRNO is 0, the hard link is already done; if positive,
the hard link was tried and failed with errno == LINKAT_ERRNO. Return
-1 if successful and DSTNAME already existed,
0 if successful and DSTNAME did not already exist, and
-1 (setting errno) on failure. */
a positive errno value on failure. */
extern int
force_linkat (int srcdir, char const *srcname,
int dstdir, char const *dstname, int flags, bool force)
int dstdir, char const *dstname, int flags, bool force,
int linkat_errno)
{
int r = linkat (srcdir, srcname, dstdir, dstname, flags);
if (!force || r == 0 || errno != EEXIST)
return r;
if (linkat_errno < 0)
linkat_errno = (linkat (srcdir, srcname, dstdir, dstname, flags) == 0
? 0 : errno);
if (!force || linkat_errno != EEXIST)
return linkat_errno;
char buf[smallsize];
char *dsttmp = samedir_template (dstname, buf);
if (! dsttmp)
return -1;
return errno;
struct link_arg arg = { srcdir, srcname, dstdir, flags };
int err;
@@ -108,7 +113,7 @@ force_linkat (int srcdir, char const *srcname,
err = errno;
else
{
err = renameat (dstdir, dsttmp, dstdir, dstname) == 0 ? 0 : errno;
err = renameat (dstdir, dsttmp, dstdir, dstname) == 0 ? -1 : errno;
/* Unlink DSTTMP even if renameat succeeded, in case DSTTMP
and DSTNAME were already the same hard link and renameat
was a no-op. */
@@ -117,10 +122,7 @@ force_linkat (int srcdir, char const *srcname,
if (dsttmp != buf)
free (dsttmp);
if (!err)
return 1;
errno = err;
return -1;
return err;
}
@@ -140,22 +142,25 @@ try_symlink (char *dest, void *arg)
}
/* Create a symlink containing SRCNAME in directory DSTDIR's file DSTNAME.
If FORCE and DSTNAME already exists, replace it atomically. Return
1 if successful and DSTNAME already existed,
If FORCE and DSTNAME already exists, replace it atomically.
If SYMLINKAT_ERRNO is 0, the symlink is already done; if positive,
the symlink was tried and failed with errno == SYMLINKAT_ERRNO. Return
-1 if successful and DSTNAME already existed,
0 if successful and DSTNAME did not already exist, and
-1 (setting errno) on failure. */
a positive errno value on failure. */
extern int
force_symlinkat (char const *srcname, int dstdir, char const *dstname,
bool force)
bool force, int symlinkat_errno)
{
int r = symlinkat (srcname, dstdir, dstname);
if (!force || r == 0 || errno != EEXIST)
return r;
if (symlinkat_errno < 0)
symlinkat_errno = symlinkat (srcname, dstdir, dstname) == 0 ? 0 : errno;
if (!force || symlinkat_errno != EEXIST)
return symlinkat_errno;
char buf[smallsize];
char *dsttmp = samedir_template (dstname, buf);
if (!dsttmp)
return -1;
return errno;
struct symlink_arg arg = { srcname, dstdir };
int err;
@@ -170,13 +175,10 @@ force_symlinkat (char const *srcname, int dstdir, char const *dstname,
{
/* Don't worry about renameat being a no-op, since DSTTMP is
newly created. */
err = 0;
err = -1;
}
if (dsttmp != buf)
free (dsttmp);
if (!err)
return 1;
errno = err;
return -1;
return err;
}

4
src/force-link.h
View File

@@ -1,2 +1,2 @@
extern int force_linkat (int, char const *, int, char const *, int, bool);
extern int force_symlinkat (char const *, int, char const *, bool);
extern int force_linkat (int, char const *, int, char const *, int, bool, int);
extern int force_symlinkat (char const *, int, char const *, bool, int);

343
src/ln.c
View File

@@ -30,8 +30,10 @@
#include "force-link.h"
#include "hash.h"
#include "hash-triple.h"
#include "priv-set.h"
#include "relpath.h"
#include "same.h"
#include "unlinkdir.h"
#include "yesno.h"
#include "canonicalize.h"
@@ -69,6 +71,9 @@ static bool verbose;
directories on most existing systems (Solaris being an exception). */
static bool hard_dir_link;
/* If true, watch out for creating or removing hard links to directories. */
static bool beware_hard_dir_link;
/* If nonzero, and the specified destination is a symbolic link to a
directory, treat it just as if it were a directory. Otherwise, the
command 'ln --force --no-dereference file symlink-to-dir' deletes
@@ -113,6 +118,14 @@ errno_nonexisting (int err)
return err == ENOENT || err == ENAMETOOLONG || err == ENOTDIR || err == ELOOP;
}
/* Return an errno value for a system call that returned STATUS.
This is zero if STATUS is zero, and is errno otherwise. */
static int
errnoize (int status)
{
return status < 0 ? errno : 0;
}
/* FILE is the last operand of this command. Return true if FILE is a
directory. But report an error if there is a problem accessing FILE,
@@ -126,9 +139,8 @@ target_directory_operand (char const *file)
size_t blen = strlen (b);
bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
struct stat st;
int stat_result =
(dereference_dest_dir_symlinks ? stat (file, &st) : lstat (file, &st));
int err = (stat_result == 0 ? 0 : errno);
int flags = dereference_dest_dir_symlinks ? 0 : AT_SYMLINK_NOFOLLOW;
int err = errnoize (fstatat (AT_FDCWD, file, &st, flags));
bool is_a_dir = !err && S_ISDIR (st.st_mode);
if (err && ! errno_nonexisting (errno))
die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
@@ -171,162 +183,181 @@ convert_abs_rel (const char *from, const char *target)
return relative_from ? relative_from : xstrdup (from);
}
/* Link SOURCE to DEST atomically. Return 0 if successful, a positive
errno value on failure, and -1 if an atomic link cannot be done.
This handles the common case where DEST does not already exist and
-r is not specified. */
static int
atomic_link (char const *source, char const *dest)
{
return (symbolic_link
? (relative ? -1 : errnoize (symlink (source, dest)))
: beware_hard_dir_link
? -1
: errnoize (linkat (AT_FDCWD, source, AT_FDCWD, dest,
logical ? AT_SYMLINK_FOLLOW : 0)));
}
/* Make a link DEST to the (usually) existing file SOURCE.
Symbolic links to nonexistent files are allowed.
LINK_ERRNO is zero if the link has already been made,
positive if attempting the link failed with errno == LINK_ERRNO,
-1 if no attempt has been made to create the link.
Return true if successful. */
static bool
do_link (const char *source, const char *dest)
do_link (const char *source, const char *dest, int link_errno)
{
struct stat source_stats;
struct stat dest_stats;
int source_status = 1;
char *dest_backup = NULL;
char *rel_source = NULL;
bool dest_lstat_ok = false;
bool source_is_dir = false;
int nofollow_flag = logical ? 0 : AT_SYMLINK_NOFOLLOW;
if (link_errno < 0)
link_errno = atomic_link (source, dest);
if (!symbolic_link)
/* Get SOURCE_STATS if later code will need it, if only for sharper
diagnostics. */
if ((link_errno || dest_set) && !symbolic_link)
{
/* Which stat to use depends on whether linkat will follow the
symlink. We can't use the shorter
(logical?stat:lstat) (source, &source_stats)
since stat might be a function-like macro. */
if ((logical ? stat (source, &source_stats)
: lstat (source, &source_stats))
!= 0)
source_status = fstatat (AT_FDCWD, source, &source_stats, nofollow_flag);
if (source_status != 0)
{
error (0, errno, _("failed to access %s"), quoteaf (source));
return false;
}
}
if (S_ISDIR (source_stats.st_mode))
if (link_errno)
{
if (!symbolic_link && !hard_dir_link && S_ISDIR (source_stats.st_mode))
{
source_is_dir = true;
if (! hard_dir_link)
error (0, 0, _("%s: hard link not allowed for directory"),
quotef (source));
return false;
}
if (relative)
source = rel_source = convert_abs_rel (source, dest);
bool force = (remove_existing_files || interactive
|| backup_type != no_backups);
if (force)
{
struct stat dest_stats;
if (lstat (dest, &dest_stats) != 0)
{
error (0, 0, _("%s: hard link not allowed for directory"),
quotef (source));
if (errno != ENOENT)
{
error (0, errno, _("failed to access %s"), quoteaf (dest));
return false;
}
force = false;
}
else if (S_ISDIR (dest_stats.st_mode))
{
error (0, 0, _("%s: cannot overwrite directory"), quotef (dest));
return false;
}
}
}
if (remove_existing_files || interactive || backup_type != no_backups)
{
dest_lstat_ok = (lstat (dest, &dest_stats) == 0);
if (!dest_lstat_ok && errno != ENOENT)
{
error (0, errno, _("failed to access %s"), quoteaf (dest));
return false;
}
}
/* If the current target was created as a hard link to another
source file, then refuse to unlink it. */
if (dest_lstat_ok
&& dest_set != NULL
&& seen_file (dest_set, dest, &dest_stats))
{
error (0, 0,
_("will not overwrite just-created %s with %s"),
quoteaf_n (0, dest), quoteaf_n (1, source));
return false;
}
/* If --force (-f) has been specified without --backup, then before
making a link ln must remove the destination file if it exists.
(with --backup, it just renames any existing destination file)
But if the source and destination are the same, don't remove
anything and fail right here. */
if ((remove_existing_files
/* Ensure that "ln --backup f f" fails here, with the
"... same file" diagnostic, below. Otherwise, subsequent
code would give a misleading "file not found" diagnostic.
This case is different than the others handled here, since
the command in question doesn't use --force. */
|| (!symbolic_link && backup_type != no_backups))
&& dest_lstat_ok
/* Allow 'ln -sf --backup k k' to succeed in creating the
self-referential symlink, but don't allow the hard-linking
equivalent: 'ln -f k k' (with or without --backup) to get
beyond this point, because the error message you'd get is
misleading. */
&& (backup_type == no_backups || !symbolic_link)
&& (!symbolic_link || stat (source, &source_stats) == 0)
&& SAME_INODE (source_stats, dest_stats)
/* The following detects whether removing DEST will also remove
SOURCE. If the file has only one link then both are surely
the same link. Otherwise check whether they point to the same
name in the same directory. */
&& (source_stats.st_nlink == 1 || same_name (source, dest)))
{
error (0, 0, _("%s and %s are the same file"),
quoteaf_n (0, source), quoteaf_n (1, dest));
return false;
}
if (dest_lstat_ok)
{
if (S_ISDIR (dest_stats.st_mode))
{
error (0, 0, _("%s: cannot overwrite directory"), quotef (dest));
return false;
}
if (interactive)
{
fprintf (stderr, _("%s: replace %s? "), program_name, quoteaf (dest));
if (!yesno ())
return true;
remove_existing_files = true;
}
if (backup_type != no_backups)
{
dest_backup = find_backup_file_name (dest, backup_type);
if (rename (dest, dest_backup) != 0)
else if (seen_file (dest_set, dest, &dest_stats))
{
int rename_errno = errno;
free (dest_backup);
dest_backup = NULL;
if (rename_errno != ENOENT)
/* The current target was created as a hard link to another
source file. */
error (0, 0,
_("will not overwrite just-created %s with %s"),
quoteaf_n (0, dest), quoteaf_n (1, source));
return false;
}
else
{
/* Beware removing DEST if it is the same directory entry as
SOURCE, because in that case removing DEST can cause the
subsequent link creation either to fail (for hard links), or
to replace a non-symlink DEST with a self-loop (for symbolic
links) which loses the contents of DEST. So, when backing
up, worry about creating hard links (since the backups cover
the symlink case); otherwise, worry about about -f. */
if (backup_type != no_backups
? !symbolic_link
: remove_existing_files)
{
error (0, rename_errno, _("cannot backup %s"),
quoteaf (dest));
return false;
/* Detect whether removing DEST would also remove SOURCE.
If the file has only one link then both are surely the
same directory entry. Otherwise check whether they point
to the same name in the same directory. */
if (source_status != 0)
source_status = stat (source, &source_stats);
if (source_status == 0
&& SAME_INODE (source_stats, dest_stats)
&& (source_stats.st_nlink == 1
|| same_name (source, dest)))
{
error (0, 0, _("%s and %s are the same file"),
quoteaf_n (0, source), quoteaf_n (1, dest));
return false;
}
}
if (link_errno < 0 || link_errno == EEXIST)
{
if (interactive)
{
fprintf (stderr, _("%s: replace %s? "),
program_name, quoteaf (dest));
if (!yesno ())
return true;
}
if (backup_type != no_backups)
{
dest_backup = find_backup_file_name (dest, backup_type);
if (rename (dest, dest_backup) != 0)
{
int rename_errno = errno;
free (dest_backup);
dest_backup = NULL;
if (rename_errno != ENOENT)
{
error (0, rename_errno, _("cannot backup %s"),
quoteaf (dest));
return false;
}
force = false;
}
}
}
}
}
/* If the attempt to create a link fails and we are removing or
backing up destinations, unlink the destination and try again.
On the surface, POSIX states that 'ln -f A B' unlinks B before trying
to link A to B. But strictly following this has the counterintuitive
effect of losing the contents of B if A does not exist. Fortunately,
POSIX 2008 clarified that an application is free to fail early if it
can prove that continuing onwards cannot succeed, so we can try to
link A to B before blindly unlinking B, thus sometimes attempting to
link a second time during a successful 'ln -f A B'.
Try to unlink DEST even if we may have backed it up successfully.
In some unusual cases (when DEST and DEST_BACKUP are hard-links
that refer to the same file), rename succeeds and DEST remains.
If we didn't remove DEST in that case, the subsequent symlink or
link call would fail. */
link_errno
= (symbolic_link
? force_symlinkat (source, AT_FDCWD, dest, force, link_errno)
: force_linkat (AT_FDCWD, source, AT_FDCWD, dest,
logical ? AT_SYMLINK_FOLLOW : 0,
force, link_errno));
/* Until now, link_errno < 0 meant the link has not been tried.
From here on, link_errno < 0 means the link worked but
required removing the destination first. */
}
if (relative)
source = rel_source = convert_abs_rel (source, dest);
/* If the attempt to create a link fails and we are removing or
backing up destinations, unlink the destination and try again.
On the surface, POSIX describes an algorithm that states that
'ln -f A B' will call unlink() on B before ever attempting
link() on A. But strictly following this has the counterintuitive
effect of losing the contents of B, if A does not exist.
Fortunately, POSIX 2008 clarified that an application is free
to fail early if it can prove that continuing onwards cannot
succeed, so we are justified in trying link() before blindly
removing B, thus sometimes calling link() a second time during
a successful 'ln -f A B'.
Try to unlink DEST even if we may have backed it up successfully.
In some unusual cases (when DEST and DEST_BACKUP are hard-links
that refer to the same file), rename succeeds and DEST remains.
If we didn't remove DEST in that case, the subsequent symlink or link
call would fail. */
bool ok_to_remove = remove_existing_files || dest_backup;
bool ok = 0 <= (symbolic_link
? force_symlinkat (source, AT_FDCWD, dest, ok_to_remove)
: force_linkat (AT_FDCWD, source, AT_FDCWD, dest,
logical ? AT_SYMLINK_FOLLOW : 0,
ok_to_remove));
if (ok)
if (link_errno <= 0)
{
/* Right after creating a hard link, do this: (note dest name and
source_stats, which are also the just-linked-destinations stats) */
@@ -343,15 +374,15 @@ do_link (const char *source, const char *dest)
}
else
{
error (0, errno,
error (0, link_errno,
(symbolic_link
? (errno != ENAMETOOLONG && *source
? (link_errno != ENAMETOOLONG && *source
? _("failed to create symbolic link %s")
: _("failed to create symbolic link %s -> %s"))
: (errno == EMLINK && !source_is_dir
: (link_errno == EMLINK
? _("failed to create hard link to %.0s%s")
: (errno == EDQUOT || errno == EEXIST || errno == ENOSPC
|| errno == EROFS)
: (link_errno == EDQUOT || link_errno == EEXIST
|| link_errno == ENOSPC || link_errno == EROFS)
? _("failed to create hard link %s")
: _("failed to create hard link %s => %s"))),
quoteaf_n (0, dest), quoteaf_n (1, source));
@@ -365,7 +396,7 @@ do_link (const char *source, const char *dest)
free (dest_backup);
free (rel_source);
return ok;
return link_errno <= 0;
}
void
@@ -444,6 +475,7 @@ main (int argc, char **argv)
bool no_target_directory = false;
int n_files;
char **file;
int link_errno = -1;
initialize_main (&argc, &argv);
set_program_name (argv[0]);
@@ -535,6 +567,15 @@ main (int argc, char **argv)
usage (EXIT_FAILURE);
}
if (relative && !symbolic_link)
die (EXIT_FAILURE, 0, _("cannot do --relative without --symbolic"));
if (!hard_dir_link)
{
priv_set_remove_linkdir ();
beware_hard_dir_link = !cannot_unlink_dir ();
}
if (no_target_directory)
{
if (target_directory)
@@ -556,11 +597,17 @@ main (int argc, char **argv)
{
if (n_files < 2)
target_directory = ".";
else if (2 <= n_files && target_directory_operand (file[n_files - 1]))
target_directory = file[--n_files];
else if (2 < n_files)
die (EXIT_FAILURE, 0, _("target %s is not a directory"),
quoteaf (file[n_files - 1]));
else
{
if (n_files == 2)
link_errno = atomic_link (file[0], file[1]);
if ((link_errno < 0 || link_errno == EEXIST || link_errno == ENOTDIR)
&& target_directory_operand (file[n_files - 1]))
target_directory = file[--n_files];
else if (2 < n_files)
die (EXIT_FAILURE, 0, _("target %s is not a directory"),
quoteaf (file[n_files - 1]));
}
}
backup_type = (make_backups
@@ -568,12 +615,6 @@ main (int argc, char **argv)
: no_backups);
set_simple_backup_suffix (backup_suffix);
if (relative && !symbolic_link)
{
die (EXIT_FAILURE, 0,
_("cannot do --relative without --symbolic"));
}
if (target_directory)
{
@@ -607,12 +648,12 @@ main (int argc, char **argv)
last_component (file[i]),
&dest_base);
strip_trailing_slashes (dest_base);
ok &= do_link (file[i], dest);
ok &= do_link (file[i], dest, -1);
free (dest);
}
}
else
ok = do_link (file[0], file[1]);
ok = do_link (file[0], file[1], link_errno);
return ok ? EXIT_SUCCESS : EXIT_FAILURE;
}