Don't let ln be a party to destroying user data.
authorJim Meyering <jim@meyering.net>
Thu, 23 Aug 2007 09:51:01 +0000 (11:51 +0200)
committerJim Meyering <jim@meyering.net>
Thu, 23 Aug 2007 12:00:35 +0000 (14:00 +0200)
* src/ln.c: Include "file-set.h", "hash.h" and "hash-triple.h".
(dest_set, DEST_INFO_INITIAL_CAPACITY): New globals.
(do_link): Refuse to remove a just-created link.
Record a name,dev,ino triple for each link we create.
(main): Initialize dest_set, if needed.
* tests/mv/childproof: Test for the above fix.
* NEWS: Document this.
Reported by Eric Blake.

Signed-off-by: Jim Meyering <jim@meyering.net>
ChangeLog
NEWS
src/ln.c
tests/mv/childproof

index e20e96f..4947123 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2007-08-23  Jim Meyering  <jim@meyering.net>
 
+       Don't let ln be a party to destroying user data.
+       * src/ln.c: Include "file-set.h", "hash.h" and "hash-triple.h".
+       (dest_set, DEST_INFO_INITIAL_CAPACITY): New globals.
+       (do_link): Refuse to remove a just-created link.
+       Record a name,dev,ino triple for each link we create.
+       (main): Initialize dest_set, if needed.
+       * tests/mv/childproof: Test for the above fix.
+       * NEWS: Document this.
+       Reported by Eric Blake.
+
        Move functions from copy.c into new modules, since ln needs them, too.
        * bootstrap.conf (gnulib_modules): Add file-set.
        * gl/lib/file-set.c (record_file, seen_file): Functions from copy.c.
diff --git a/NEWS b/NEWS
index c548c0b..6a0f18d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -56,6 +56,18 @@ GNU coreutils NEWS                                    -*- outline -*-
   ptx longer accepts the --copyright option.
   who no longer accepts -i or --idle.
 
+** Improved robustness
+
+  ln -f can no longer silently clobber a just-created hard link.
+  In some cases, ln could be seen as being responsible for data loss.
+  For example, given directories a, b, c, and files a/f and b/f, we
+  should be able to do this safely: ln -f a/f b/f c && rm -f a/f b/f
+  However, before this change, ln would succeed, and thus cause the
+  loss of the contents of a/f.
+
+  stty no longer silently accepts certain invalid hex values
+  in its 35-colon commmand-line argument
+
 ** Bug fixes
 
   cp attempts to read a regular file, even if stat says it is empty.
@@ -130,11 +142,6 @@ GNU coreutils NEWS                                    -*- outline -*-
   tr -c no longer aborts when translating with Set2 larger than the
   complement of Set1.  [introduced with the original version, in 1992]
 
-** Improved robustness
-
-  stty no longer silently accepts certain invalid hex values
-  in its 35-colon commmand-line argument
-
 
 * Noteworthy changes in release 6.9 (2007-03-22) [stable]
 
index 3ddcfdf..aa0e473 100644 (file)
--- a/src/ln.c
+++ b/src/ln.c
 #include <getopt.h>
 
 #include "system.h"
-#include "same.h"
 #include "backupfile.h"
 #include "error.h"
 #include "filenamecat.h"
+#include "file-set.h"
+#include "hash.h"
+#include "hash-triple.h"
 #include "quote.h"
+#include "same.h"
 #include "yesno.h"
 
 /* The official name of this program (e.g., no `g' prefix).  */
@@ -82,6 +85,15 @@ static bool hard_dir_link;
    symlink-to-dir before creating the new link.  */
 static bool dereference_dest_dir_symlinks = true;
 
+/* This is a set of destination name/inode/dev triples for hard links
+   created by ln.  Use this data structure to avoid data loss via a
+   sequence of commands like this:
+   rm -rf a b c; mkdir a b c; touch a/f b/f; ln -f a/f b/f c && rm -r a b */
+static Hash_table *dest_set;
+
+/* Initial size of the dest_set hash table.  */
+enum { DEST_INFO_INITIAL_CAPACITY = 61 };
+
 static struct option const long_options[] =
 {
   {"backup", optional_argument, NULL, 'b'},
@@ -178,6 +190,18 @@ do_link (const char *source, const char *dest)
        }
     }
 
+  /* 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"),
+            quote_n (0, dest), quote_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)
@@ -278,6 +302,10 @@ do_link (const char *source, const char *dest)
 
   if (ok)
     {
+      /* Right after creating a hard link, do this: (note dest name and
+        source_stats, which are also the just-linked-destinations stats) */
+      record_file (dest_set, dest, &source_stats);
+
       if (verbose)
        {
          if (dest_backup)
@@ -514,6 +542,29 @@ main (int argc, char **argv)
   if (target_directory)
     {
       int i;
+
+      /* Create the data structure we'll use to record which hard links we
+        create.  Used to ensure that ln detects an obscure corner case that
+        might result in user data loss.  Create it only if needed.  */
+      if (2 <= n_files
+         && remove_existing_files
+         /* Don't bother trying to protect symlinks, since ln clobbering
+            a just-created symlink won't ever lead to real data loss.  */
+         && ! symbolic_link
+         /* No destination hard link can be clobbered when making
+            numbered backups.  */
+         && backup_type != numbered_backups)
+
+       {
+         dest_set = hash_initialize (DEST_INFO_INITIAL_CAPACITY,
+                                     NULL,
+                                     triple_hash,
+                                     triple_compare,
+                                     triple_free);
+         if (dest_set == NULL)
+           xalloc_die ();
+       }
+
       ok = true;
       for (i = 0; i < n_files; ++i)
        {
index 092afc8..e35afb6 100755 (executable)
@@ -1,8 +1,9 @@
 #!/bin/sh
-# Ensure that cp/mv don't clobber a just-copied file.
-# With fileutils-4.1 and earlier, this test would fail.
+# Ensure that cp/mv/ln don't clobber a just-copied/moved/linked file.
+# With fileutils-4.1 and earlier, this test would fail for cp and mv.
+# With coreutils-6.9 and earlier, this test would fail for ln.
 
-# Copyright (C) 2001, 2004, 2006 Free Software Foundation, Inc.
+# Copyright (C) 2001, 2004, 2006-2007 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
@@ -21,6 +22,7 @@ if test "$VERBOSE" = yes; then
   set -x
   cp --version
   mv --version
+  ln --version
 fi
 
 . $srcdir/../envvar-check
@@ -87,4 +89,15 @@ test -f b/g && fail=1  # b/g should have been moved
 test -f c/f || fail=1
 test -f c/g || fail=1
 
+# Test ln -f.
+
+rm -f a/f b/f c/f
+echo a > a/f || fail=1
+echo b > b/f || fail=1
+ln -f a/f b/f c 2> /dev/null && fail=1
+# a/f and c/f must be linked
+test `stat --format %i a/f` = `stat --format %i c/f` || fail=1
+# b/f and c/f must not be linked
+test `stat --format %i b/f` = `stat --format %i c/f` && fail=1
+
 (exit $fail); exit $fail