From d02e4e77753f580ab91afc5915333222edc82104 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 23 Aug 2007 11:51:01 +0200 Subject: 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. Signed-off-by: Jim Meyering --- ChangeLog | 10 ++++++++++ NEWS | 17 ++++++++++++----- src/ln.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- tests/mv/childproof | 19 ++++++++++++++++--- 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index e20e96fae..4947123a4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2007-08-23 Jim Meyering + 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 c548c0b1b..6a0f18d9a 100644 --- 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] diff --git a/src/ln.c b/src/ln.c index 3ddcfdfff..aa0e47382 100644 --- a/src/ln.c +++ b/src/ln.c @@ -22,11 +22,14 @@ #include #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) { diff --git a/tests/mv/childproof b/tests/mv/childproof index 092afc81f..e35afb6b4 100755 --- a/tests/mv/childproof +++ b/tests/mv/childproof @@ -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 -- cgit v1.2.3-70-g09d2