mirrors/coreutils
0
0
Fork 0
mirror of https://github.com/coreutils/coreutils.git synced 2024-11-24 10:23:31 +08:00
Code Issues Packages Projects Releases Wiki Activity

copy: make backup files more reliably

Browse Source 
* NEWS, doc/coreutils.texi (Backup options): Document the change.
* bootstrap.conf (gnulib_modules): Add backup-rename.
* src/copy.c (copy_internal): Silently switch to numbered backups
if a simple backup might lose data.  Use backup_file_rename
to avoid races with numbered backups.
* tests/cp/backup-is-src.sh, tests/mv/backup-is-src.sh:
Adjust to match new behavior.
This commit is contained in:
Paul Eggert 2017-07-30 17:11:24 -07:00
parent 2ae1460dad
commit 0d74ac470f
6 changed files with 62 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
NEWS
View File

@ -9,6 +9,18 @@ GNU coreutils NEWS -*- outline -*-
mv would leave such symlinks behind in the source file system.
[the bug dates back to the initial implementation]
When creating numbered backups, cp, install, ln, and mv now avoid
races that could lose backup data in unlikely circumstances. Since
the fix relies on the renameat2 system call of Linux kernel 3.15 and
later, the races are still present on other platforms.
[the bug dates back to the initial implementation]
cp, install, ln, and mv now use a numbered instead of a simple
backup if copying a backup file to what might be its original.
E.g., 'rm -f a a~; : > a; echo data > a~; cp --backup=simple a~ ./a'
now makes a numbered backup file instead of losing the data.
[the bug dates back to the initial implementation]
date and touch no longer overwrite the heap with large
user specified TZ values (CVE-2017-7476).
[bug introduced in coreutils-8.27]

1
bootstrap.conf
View File

@ -35,6 +35,7 @@ gnulib_modules="
assert
autobuild
backupfile
backup-rename
base32
base64
buffer-lcm

18
doc/coreutils.texi
View File

@ -865,17 +865,19 @@ Never make backups.
@opindex numbered @r{backup method}
Always make numbered backups.
@item existing
@itemx nil
@opindex existing @r{backup method}
Make numbered backups of files that already have them, simple backups
of the others.
@item simple
@itemx never
@opindex simple @r{backup method}
Always make simple backups. Please note @samp{never} is not to be
confused with @samp{none}.
Make simple backups, except make a numbered backup if the source might
be a simple backup of the destination; this avoids losing source data.
Please note @samp{never} is not to be confused with @samp{none}.
@item existing
@itemx nil
@opindex existing @r{backup method}
Make numbered backups of files that already have them, or if the source
might be a simple backup of the destination.
Otherwise, make simple backups.
@end table

67
src/copy.c
View File

@ -1837,7 +1837,6 @@ copy_internal (char const *src_name, char const *dst_name,
bool restore_dst_mode = false;
char *earlier_file = NULL;
char *dst_backup = NULL;
bool backup_succeeded = false;
bool delayed_ok;
bool copied_as_regular = false;
bool dest_is_symlink = false;
@ -2070,10 +2069,11 @@ copy_internal (char const *src_name, char const *dst_name,
}
}
char const *srcbase;
if (x->backup_type != no_backups
/* Don't try to back up a destination if the last
component of src_name is "." or "..". */
&& ! dot_or_dotdot (last_component (src_name))
&& ! dot_or_dotdot (srcbase = last_component (src_name))
/* Create a backup of each destination directory in move mode,
but not in copy mode. FIXME: it might make sense to add an
option to suppress backup creation also for move mode.
@ -2081,55 +2081,38 @@ copy_internal (char const *src_name, char const *dst_name,
existing hierarchy. */
&& (x->move_mode || ! S_ISDIR (dst_sb.st_mode)))
{
char *tmp_backup = find_backup_file_name (dst_name,
x->backup_type);
/* Detect (and fail) when creating the backup file would
destroy the source file. Before, running the commands
/* Silently use numbered backups if creating the backup
file might destroy the source file. Otherwise, the commands:
cd /tmp; rm -f a a~; : > a; echo A > a~; cp --b=simple a~ a
would leave two zero-length files: a and a~. */
/* FIXME: but simply change e.g., the final a~ to './a~'
and the source will still be destroyed. */
if (STREQ (tmp_backup, src_name))
enum backup_type backup_type = x->backup_type;
if (backup_type != numbered_backups)
{
const char *fmt;
fmt = (x->move_mode
? _("backing up %s would destroy source; %s not moved")
: _("backing up %s would destroy source; %s not copied"));
error (0, 0, fmt,
quoteaf_n (0, dst_name),
quoteaf_n (1, src_name));
free (tmp_backup);
return false;
size_t srcbaselen = strlen (srcbase);
char const *dstbase = last_component (dst_name);
size_t dstbaselen = strlen (dstbase);
if (srcbaselen == dstbaselen + strlen (simple_backup_suffix)
&& memcmp (srcbase, dstbase, dstbaselen) == 0
&& STREQ (srcbase + dstbaselen, simple_backup_suffix))
backup_type = numbered_backups;
}
char *tmp_backup = backup_file_rename (dst_name, backup_type);
/* FIXME: use fts:
Using alloca for a file name that may be arbitrarily
long is not recommended. In fact, even forming such a name
should be discouraged. Eventually, this code will be rewritten
to use fts, so using alloca here will be less of a problem. */
ASSIGN_STRDUPA (dst_backup, tmp_backup);
free (tmp_backup);
/* In move mode, when src_name and dst_name are on the
same partition (FIXME, and when they are non-directories),
make the operation atomic: link dest
to backup, then rename src to dest. */
if (rename (dst_name, dst_backup) != 0)
if (tmp_backup)
{
if (errno != ENOENT)
{
error (0, errno, _("cannot backup %s"),
quoteaf (dst_name));
return false;
}
else
{
dst_backup = NULL;
}
ASSIGN_STRDUPA (dst_backup, tmp_backup);
free (tmp_backup);
}
else
else if (errno != ENOENT)
{
backup_succeeded = true;
error (0, errno, _("cannot backup %s"), quoteaf (dst_name));
return false;
}
new_dst = true;
}
@ -2194,7 +2177,7 @@ copy_internal (char const *src_name, char const *dst_name,
sure we'll create a directory. Also don't announce yet when moving
so we can distinguish renames versus copies. */
if (x->verbose && !x->move_mode && !S_ISDIR (src_mode))
emit_verbose (src_name, dst_name, backup_succeeded ? dst_backup : NULL);
emit_verbose (src_name, dst_name, dst_backup);
/* Associate the destination file name with the source device and inode
so that if we encounter a matching dev/ino pair in the source tree
@ -2317,8 +2300,7 @@ copy_internal (char const *src_name, char const *dst_name,
if (x->verbose)
{
printf (_("renamed "));
emit_verbose (src_name, dst_name,
backup_succeeded ? dst_backup : NULL);
emit_verbose (src_name, dst_name, dst_backup);
}
if (x->set_security_context)
@ -2423,8 +2405,7 @@ copy_internal (char const *src_name, char const *dst_name,
if (x->verbose && !S_ISDIR (src_mode))
{
printf (_("copied "));
emit_verbose (src_name, dst_name,
backup_succeeded ? dst_backup : NULL);
emit_verbose (src_name, dst_name, dst_backup);
}
new_dst = true;
}

16
tests/cp/backup-is-src.sh
View File

@ -20,17 +20,15 @@
print_ver_ cp
echo a > a || framework_failure_
cp a a0 || framework_failure_
echo a-tilde > a~ || framework_failure_
cp a~ a~0 || framework_failure_
# This cp command should exit nonzero.
cp --b=simple a~ a > out 2>&1 && fail=1
# This cp command should not trash the source.
cp --b=simple a~ ./a > out 2>&1 || fail=1
sed "s,cp:,XXX:," out > out2
cat > exp <<\EOF
XXX: backing up 'a' would destroy source; 'a~' not copied
EOF
compare exp out2 || fail=1
compare a~0 a || fail=1
compare a~0 a~ || fail=1
compare a0 a.~1~ || fail=1
Exit $fail

23
tests/mv/backup-is-src.sh
View File

@ -22,25 +22,18 @@ cleanup_() { rm -rf "$other_partition_tmpdir"; }
. "$abs_srcdir/tests/other-fs-tmpdir"
a="$other_partition_tmpdir/a"
a2="$other_partition_tmpdir/a~"
a2="$other_partition_tmpdir/./a~"
rm -f "$a" "$a2" || framework_failure_
rm -f "$a" "$a2" a20 || framework_failure_
echo a > "$a" || framework_failure_
cp "$a" a0 || framework_failure_
echo a2 > "$a2" || framework_failure_
cp "$a2" a20 || framework_failure_
# This mv command should exit nonzero.
mv --b=simple "$a2" "$a" > out 2>&1 && fail=1
# This mv command should not trash the source.
mv --b=simple "$a2" "$a" > out 2>&1 || fail=1
sed \
-e "s,mv:,XXX:," \
-e "s,$a,YYY," \
-e "s,$a2,ZZZ," \
out > out2
cat > exp <<\EOF
XXX: backing up 'YYY' would destroy source; 'ZZZ' not moved
EOF
compare exp out2 || fail=1
compare a20 "$a" || fail=1
compare a0 "$a.~1~" || fail=1
Exit $fail