bug#25342: GNU coreutils: race condition in "ln -f source dest"

[Mirsad Goran Todorovac]

When "dest" is existing, ln -f source dest does an attempt of linkat(AT_FCWDFD, 
source, AT_FCWDFD, dest), which fails with EEXIST if dest exists, and then 
attempts an unlink(dest), followed by repeated linkat().
Between unlink() and second linkat() process maybe preempted, leaving no system 
file.

An option -o (--force-overwrite) is proposed, so existing scripts would work 
w/o breaking, and linkat() can be made atomic. By rights, there oughta be an 
option for linkat() to do it, but right now it was done by spurious idea 
(Thanking the grace of Lord Merciful and Longsuffering).
Patch follows.
All the best,Mirsad X. Todorovac

__________________________________________________________________________________________________
address@hidden:~$ diff -r -U 3 coreutils-8.26 coreutils-8.26p1
diff -r -U 3 coreutils-8.26/src/ln.c coreutils-8.26p1/src/ln.c
--- coreutils-8.26/src/ln.c    2016-11-14 13:05:20.000000000 +0100
+++ coreutils-8.26p1/src/ln.c    2017-01-02 22:23:53.608194338 +0100
@@ -59,6 +59,9 @@
 /* If true, remove existing files unconditionally.  */
 static bool remove_existing_files;
 
+/* If true, remove existing files unconditionally, even if losing data.  */
+static bool force_remove_existing_files;
+
 /* If true, list each file as it is moved. */
 static bool verbose;
 
@@ -90,6 +93,7 @@
   {"no-dereference", no_argument, NULL, 'n'},
   {"no-target-directory", no_argument, NULL, 'T'},
   {"force", no_argument, NULL, 'f'},
+  {"force-overwrite", no_argument, NULL, 'o'},
   {"interactive", no_argument, NULL, 'i'},
   {"suffix", required_argument, NULL, 'S'},
   {"target-directory", required_argument, NULL, 't'},
@@ -283,7 +287,8 @@
       if (backup_type != no_backups)
         {
           dest_backup = find_backup_file_name (dest, backup_type);
-          if (rename (dest, dest_backup) != 0)
+      if (logical && rename (dest, dest_backup) != 0 ||
+              !logical && link (dest, dest_backup) != 0)
             {
               int rename_errno = errno;
               free (dest_backup);
@@ -301,10 +306,31 @@
   if (relative)
     source = rel_source = convert_abs_rel (source, dest);
 
-  ok = ((symbolic_link ? symlink (source, dest)
+  if (logical || !force_remove_existing_files)
+    ok = ((symbolic_link ? symlink (source, dest)
          : linkat (AT_FDCWD, source, AT_FDCWD, dest,
                    logical ? AT_SYMLINK_FOLLOW : 0))
         == 0);
+  else {
+    char *tmpname = NULL;
+
+    if (force_remove_existing_files &&
+        asprintf (&tmpname, "%s.%06d.bak", source, (int)getpid()) < 
strlen(source) + 1 + 6 + 4) {
+      int asprintf_errno = errno;
+      if (tmpname)
+    free (tmpname);
+      free (dest_backup);
+      free (rel_source);
+      error (0, asprintf_errno, _("cannot backup %s: cannot allocate temp 
string"),
+             quoteaf (dest));
+      return false;
+    }
+    
+    ok = ((symbolic_link ? symlink (source, dest)
+         : linkat (AT_FDCWD, source, AT_FDCWD, tmpname, 0) == 0
+           && renameat (AT_FDCWD, tmpname, AT_FDCWD, dest) == 0));
+  }
+
 
   /* If the attempt to create a link failed and we are removing or
      backing up destinations, unlink the destination and try again.
@@ -417,6 +443,7 @@
                                 directories (note: will probably fail due to\n\
                                 system restrictions, even for the superuser)\n\
   -f, --force                 remove existing destination files\n\
+  -o, --overwrite             force remove existing destination files\n\
 "), stdout);
       fputs (_("\
   -i, --interactive           prompt whether to remove destinations\n\
@@ -467,10 +494,10 @@
 
   atexit (close_stdin);
 
-  symbolic_link = remove_existing_files = interactive = verbose
+  symbolic_link = force_remove_existing_files = remove_existing_files = 
interactive = verbose
     = hard_dir_link = false;
 
-  while ((c = getopt_long (argc, argv, "bdfinrst:vFLPS:T", long_options, NULL))
+  while ((c = getopt_long (argc, argv, "bdfinorst:vFLPS:T", long_options, 
NULL))
          != -1)
     {
       switch (c)
@@ -484,6 +511,8 @@
         case 'F':
           hard_dir_link = true;
           break;
+        case 'o':
+          force_remove_existing_files = true;
         case 'f':
           remove_existing_files = true;
           interactive = false;
address@hidden:~$