CC-tea/coreutils
1
0
Fork 0
mirror of git://git.sv.gnu.org/coreutils.git synced 2026-04-20 10:51:48 +02:00
Code Issues Projects Releases Wiki Activity

Correct cp's handling of destination symlinks in some cases.

Browse Source 
* NEWS: "cp" no longer considers a destination symlink to be the
same as the referenced file when copying links or making backups.
* src/copy.c (copy_reg): When following a symlink, use the
followed name in later chown etc. requests, so that the created
file is affected, rather than the symlink.  Use O_NOFOLLOW on
source when not dereferencing symlinks; this avoids a race.
Preserve errno correctly when doing multiple open attempts on the
destination.
(copy_internal): Follow destination symlinks only when copying a
regular file and only when we don't intend to remove or rename the
destination first, regardless of whether following source
symlinks; this is because since POSIX and tradition (e.g.,
FreeBSD) say we should ordinarily follow destination symlinks if
the system calls would ordinarily do so.
* src/copy.h (struct cp_options): Add comment that 'dereference'
is only for source files.
* src/cp.c (usage): Note that --derereference etc. are only for
source files.
(make_dir_parents_private): Follow symlinks, regardless of whether
--dereference is specified, because these are destination symlinks.
* tests/cp/same-file: Adjust tests to match revised behavior.
Filter out perror output since it might vary from host to host.
Use sed alone instead of also using echo.

* doc/coreutils.texi (cp invocation): Document the behavior better when
the destination is a symlink.  Clarify source versus destination
symlinks.  Describe the new behavior for destination symlinks.

2007-06-15  Jim Meyering  <jim@meyering.net>

* src/copy.c: Include "canonicalize.h".
(copy_reg): Use canonicalize_filename_mode to follow the symlink,
so that we can always open with O_EXCL and avoid a race.
This commit is contained in:
Paul Eggert
2007-06-15 22:47:16 +02:00
committed by Jim Meyering
parent cad884a245
commit cdec7e6e93
7 changed files with 135 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
ChangeLog
View File

@@ -1,3 +1,40 @@
2007-06-15 Paul Eggert <eggert@cs.ucla.edu>
Correct cp's handling of destination symlinks in some cases.
* NEWS: "cp" no longer considers a destination symlink to be the
same as the referenced file when copying links or making backups.
* src/copy.c (copy_reg): When following a symlink, use the
followed name in later chown etc. requests, so that the created
file is affected, rather than the symlink. Use O_NOFOLLOW on
source when not dereferencing symlinks; this avoids a race.
Preserve errno correctly when doing multiple open attempts on the
destination.
(copy_internal): Follow destination symlinks only when copying a
regular file and only when we don't intend to remove or rename the
destination first, regardless of whether following source
symlinks; this is because since POSIX and tradition (e.g.,
FreeBSD) say we should ordinarily follow destination symlinks if
the system calls would ordinarily do so.
* src/copy.h (struct cp_options): Add comment that 'dereference'
is only for source files.
* src/cp.c (usage): Note that --derereference etc. are only for
source files.
(make_dir_parents_private): Follow symlinks, regardless of whether
--dereference is specified, because these are destination symlinks.
* tests/cp/same-file: Adjust tests to match revised behavior.
Filter out perror output since it might vary from host to host.
Use sed alone instead of also using echo.
* doc/coreutils.texi (cp invocation): Document the behavior better when
the destination is a symlink. Clarify source versus destination
symlinks. Describe the new behavior for destination symlinks.
2007-06-15 Jim Meyering <jim@meyering.net>
* src/copy.c: Include "canonicalize.h".
(copy_reg): Use canonicalize_filename_mode to follow the symlink,
so that we can always open with O_EXCL and avoid a race.
2007-06-15 Jim Meyering <jim@meyering.net>
Don't include "quote.h" when it is not used.

7
NEWS
View File

@@ -18,7 +18,12 @@ GNU coreutils NEWS -*- outline -*-
** Bug fixes
cp no longer fails to write through a dangling symlink
[bug introduced in coreutils-6.7]
[bug introduced in coreutils-6.7]. Also, 'cp' no longer considers a
destination symlink to be the same as the referenced file when
copying links or making backups. For example, if SYM is a symlink
to FILE, "cp -l FILE SYM" now reports an error instead of silently
doing nothing. The behavior of 'cp' is now better documented when
the destination is a symlink.
cut now diagnoses a range starting with zero (e.g., -f 0-2) as invalid;
before, it would treat it as if it started with 1 (-f 1-2).

24
doc/coreutils.texi
View File

@@ -6910,13 +6910,22 @@ By default, @command{cp} does not copy directories. However, the
copy recursively by descending into source directories and copying files
to corresponding destination directories.
By default, @command{cp} follows symbolic links only when not copying
When copying from a symbolic link, @command{cp} normally follows the
link only when not copying
recursively. This default can be overridden with the
@option{--archive} (@option{-a}), @option{-d}, @option{--dereference}
(@option{-L}), @option{--no-dereference} (@option{-P}), and
@option{-H} options. If more than one of these options is specified,
the last one silently overrides the others.
When copying to a symbolic link, @command{cp} normally follows the
link when creating or copying to a regular file, even if the link is
dangling. However, @command{cp} does not follow these links when
creating directories or special files. Also, when an option like
@option{--backup} or @option{--link} acts to rename or remove the
destination before copying, @command{cp} renames or removes the
symbolic link rather than the file it points to.
By default, @command{cp} copies the contents of special files only
when not copying recursively. This default can be overridden with the
@option{--copy-contents} option.
@@ -6996,10 +7005,10 @@ Equivalent to @option{--no-dereference --preserve=links}.
@opindex --force
When copying without this option and an existing destination file cannot
be opened for writing, the copy fails. However, with @option{--force}),
when a destination file cannot be opened, @command{cp} then unlinks it and
when a destination file cannot be opened, @command{cp} then removes it and
tries to open it again. Contrast this behavior with that enabled by
@option{--link} and @option{--symbolic-link}, whereby the destination file
is never opened but rather is unlinked unconditionally. Also see the
is never opened but rather is removed unconditionally. Also see the
description of @option{--remove-destination}.
This option is independent of the @option{--interactive} or
@@ -7029,7 +7038,7 @@ Make hard links instead of copies of non-directories.
@itemx --dereference
@opindex -L
@opindex --dereference
Always follow symbolic links.
Follow symbolic links when copying from them.
@item -P
@itemx --no-dereference
@@ -7037,7 +7046,8 @@ Always follow symbolic links.
@opindex --no-dereference
@cindex symbolic links, copying
Copy symbolic links as symbolic links rather than copying the files that
they point to.
they point to. This option affects only symbolic links in the source;
symbolic links in the destination are always followed if possible.
@item -p
@itemx @w{@kbd{--preserve}[=@var{attribute_list}]}
@@ -7127,8 +7137,8 @@ about each existing destination file.
@cindex copying directories recursively
@cindex recursively copying directories
@cindex non-directories, copying as special files
Copy directories recursively. Symbolic links are not followed by
default; see the @option{--archive} (@option{-a}), @option{-d},
Copy directories recursively. By default, do not follow symbolic
links in the source; see the @option{--archive} (@option{-a}), @option{-d},
@option{--dereference} (@option{-L}), @option{--no-dereference}
(@option{-P}), and @option{-H} options. Special files are copied by
creating a destination file of the same type as the source; see the

69
src/copy.c
View File

@@ -34,6 +34,7 @@
#include "acl.h"
#include "backupfile.h"
#include "buffer-lcm.h"
#include "canonicalize.h"
#include "copy.h"
#include "cp-hash.h"
#include "euidaccess.h"
@@ -261,14 +262,19 @@ copy_reg (char const *src_name, char const *dst_name,
{
char *buf;
char *buf_alloc = NULL;
char *name_alloc = NULL;
char const *followed_dest_name = dst_name;
int dest_desc;
int dest_errno;
int source_desc;
mode_t src_mode = src_sb->st_mode;
struct stat sb;
struct stat src_open_sb;
bool return_val = true;
source_desc = open (src_name, O_RDONLY | O_BINARY);
source_desc = open (src_name,
(O_RDONLY | O_BINARY
| (x->dereference == DEREF_NEVER ? O_NOFOLLOW : 0)));
if (source_desc < 0)
{
error (0, errno, _("cannot open %s for reading"), quote (src_name));
@@ -298,6 +304,7 @@ copy_reg (char const *src_name, char const *dst_name,
if (! *new_dst)
{
dest_desc = open (dst_name, O_WRONLY | O_TRUNC | O_BINARY);
dest_errno = errno;
/* When using cp --preserve=context to copy to an existing destination,
use the default context rather than that of the source. Why?
@@ -353,21 +360,35 @@ copy_reg (char const *src_name, char const *dst_name,
if (*new_dst)
{
int open_flags = O_WRONLY | O_CREAT | O_BINARY;
dest_desc = open (dst_name, open_flags | O_EXCL,
int open_flags = O_WRONLY | O_CREAT | O_EXCL | O_BINARY;
dest_desc = open (dst_name, open_flags,
dst_mode & ~omitted_permissions);
dest_errno = errno;
/* When trying to copy through a dangling destination symlink,
the above open fails with EEXIST. If that happens, and
lstat'ing the DST_NAME shows that it is a symlink, repeat
the open call, but this time without O_EXCL. */
if (dest_desc < 0 && errno == EEXIST)
the open call, but this time with the name of the final,
missing directory entry. */
if (dest_desc < 0 && dest_errno == EEXIST)
{
struct stat dangling_link_sb;
if (lstat (dst_name, &dangling_link_sb) == 0
&& S_ISLNK (dangling_link_sb.st_mode))
dest_desc = open (dst_name, open_flags,
dst_mode & ~omitted_permissions);
{
/* FIXME: This is way overkill, since all that's needed
is to follow the symlink that is the last file name
component. */
name_alloc =
canonicalize_filename_mode (dst_name, CAN_MISSING);
if (name_alloc)
{
followed_dest_name = name_alloc;
dest_desc = open (followed_dest_name, open_flags,
dst_mode & ~omitted_permissions);
dest_errno = errno;
}
}
}
}
else
@@ -375,7 +396,8 @@ copy_reg (char const *src_name, char const *dst_name,
if (dest_desc < 0)
{
error (0, errno, _("cannot create regular file %s"), quote (dst_name));
error (0, dest_errno, _("cannot create regular file %s"),
quote (dst_name));
return_val = false;
goto close_src_desc;
}
@@ -567,7 +589,7 @@ copy_reg (char const *src_name, char const *dst_name,
timespec[0] = get_stat_atime (src_sb);
timespec[1] = get_stat_mtime (src_sb);
if (gl_futimens (dest_desc, dst_name, timespec) != 0)
if (gl_futimens (dest_desc, followed_dest_name, timespec) != 0)
{
error (0, errno, _("preserving times for %s"), quote (dst_name));
if (x->require_preserve)
@@ -580,7 +602,7 @@ copy_reg (char const *src_name, char const *dst_name,
if (x->preserve_ownership && ! SAME_OWNER_AND_GROUP (*src_sb, sb))
{
switch (set_owner (x, dst_name, dest_desc,
switch (set_owner (x, followed_dest_name, dest_desc,
src_sb->st_uid, src_sb->st_gid))
{
case -1:
@@ -593,24 +615,24 @@ copy_reg (char const *src_name, char const *dst_name,
}
}
set_author (dst_name, dest_desc, src_sb);
set_author (followed_dest_name, dest_desc, src_sb);
if (x->preserve_mode || x->move_mode)
{
if (copy_acl (src_name, source_desc, dst_name, dest_desc, src_mode) != 0
if (copy_acl (src_name, source_desc, followed_dest_name, dest_desc, src_mode) != 0
&& x->require_preserve)
return_val = false;
}
else if (x->set_mode)
{
if (set_acl (dst_name, dest_desc, x->mode) != 0)
if (set_acl (followed_dest_name, dest_desc, x->mode) != 0)
return_val = false;
}
else if (omitted_permissions)
{
omitted_permissions &= ~ cached_umask ();
if (omitted_permissions
&& fchmod_or_lchmod (dest_desc, dst_name, dst_mode) != 0)
&& fchmod_or_lchmod (dest_desc, followed_dest_name, dst_mode) != 0)
{
error (0, errno, _("preserving permissions for %s"),
quote (dst_name));
@@ -633,6 +655,7 @@ close_src_desc:
}
free (buf_alloc);
free (name_alloc);
return return_val;
}
@@ -1137,7 +1160,21 @@ copy_internal (char const *src_name, char const *dst_name,
if (!new_dst)
{
if (XSTAT (x, dst_name, &dst_sb) != 0)
/* Regular files can be created by writing through symbolic
links, but other files cannot. So use stat on the
destination when copying a regular file, and lstat otherwise.
However, if we intend to unlink or remove the destination
first, use lstat, since a copy won't actually be made to the
destination in that case. */
if ((((S_ISREG (src_mode)
|| (x->copy_as_regular
&& ! (S_ISDIR (src_mode) || S_ISLNK (src_mode))))
&& ! (x->move_mode || x->symbolic_link || x->hard_link
|| x->backup_type != no_backups
|| x->unlink_dest_before_opening))
? stat (dst_name, &dst_sb)
: lstat (dst_name, &dst_sb))
!= 0)
{
if (errno != ENOENT)
{
@@ -1151,7 +1188,7 @@ copy_internal (char const *src_name, char const *dst_name,
}
else
{ /* Here, we know that dst_name exists, at least to the point
that it is XSTAT'able. */
that it is stat'able or lstat'table. */
bool return_now;
bool unlink_src;

2
src/copy.h
View File

@@ -87,7 +87,7 @@ struct cp_options
them, symbolic links,) as if they were regular files. */
bool copy_as_regular;
/* How to handle symlinks. */
/* How to handle symlinks in the source. */
enum Dereference_symlink dereference;
/* If true, remove each existing destination nondirectory before

12
src/cp.c
View File

@@ -181,14 +181,14 @@ Mandatory arguments to long options are mandatory for short options too.\n\
-f, --force if an existing destination file cannot be\n\
opened, remove it and try again\n\
-i, --interactive prompt before overwrite\n\
-H follow command-line symbolic links\n\
-H follow command-line symbolic links in SOURCE\n\
"), stdout);
fputs (_("\
-l, --link link files instead of copying\n\
-L, --dereference always follow symbolic links\n\
-L, --dereference always follow symbolic links in SOURCE\n\
"), stdout);
fputs (_("\
-P, --no-dereference never follow symbolic links\n\
-P, --no-dereference never follow symbolic links in SOURCE\n\
"), stdout);
fputs (_("\
-p same as --preserve=mode,ownership,timestamps\n\
@@ -393,7 +393,7 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
*attr_list = NULL;
if (XSTAT (x, dst_dir, &stats))
if (stat (dst_dir, &stats) != 0)
{
/* A parent of CONST_DIR does not exist.
Make all missing intermediate directories. */
@@ -413,7 +413,7 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
*attr_list = new;
*slash = '\0';
if (XSTAT (x, dir, &stats))
if (stat (dir, &stats) != 0)
{
mode_t src_mode;
mode_t omitted_permissions;
@@ -426,7 +426,7 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
make_dir_parents_private creates only e_dir/../a if
./b already exists. */
*new_dst = true;
src_errno = (XSTAT (x, src, &stats) != 0
src_errno = (stat (src, &stats) != 0
? errno
: S_ISDIR (stats.st_mode)
? 0

24
tests/cp/same-file
View File

@@ -89,9 +89,15 @@ for args in 'foo symlink' 'symlink foo' 'foo foo' 'sl1 sl2' 'foo hardlink'; do
cp $options $args 2>_err
echo $? $options
# Normalize the program name in the error output,
# Normalize the program name and diagnostics in the error output,
# and put brackets around the output.
test -s _err && echo "[`sed 's/^[^:][^:]*:/cp:/' _err`]"
if test -s _err; then
sed '
s/^[^:]*:\([^:]*\).*/cp:\1/
1s/^/[/
$s/$/]/
' _err
fi
# Strip off all but the file names.
ls="`ls -gG --ignore=_err . \
| sed \
@@ -128,13 +134,13 @@ cat <<\EOF > $expected
0 -bd (foo symlink symlink.~1~ -> foo)
0 -bf (foo symlink symlink.~1~ -> foo)
0 -bdf (foo symlink symlink.~1~ -> foo)
0 -l (foo symlink -> foo)
1 -l [cp: cannot create link `symlink'] (foo symlink -> foo)
0 -dl (foo symlink -> foo)
0 -fl (foo symlink -> foo)
0 -fl (foo symlink)
0 -dfl (foo symlink)
0 -bl (foo symlink -> foo)
0 -bl (foo symlink symlink.~1~ -> foo)
0 -bdl (foo symlink symlink.~1~ -> foo)
0 -bfl (foo symlink -> foo)
0 -bfl (foo symlink symlink.~1~ -> foo)
0 -bdfl (foo symlink symlink.~1~ -> foo)
1 [cp: `symlink' and `foo' are the same file] (foo symlink -> foo)
@@ -179,10 +185,10 @@ cat <<\EOF > $expected
0 -bd (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo)
0 -bf (foo sl1 -> foo sl2 sl2.~1~ -> foo)
0 -bdf (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo)
0 -l (foo sl1 -> foo sl2 -> foo)
1 -l [cp: cannot create link `sl2'] (foo sl1 -> foo sl2 -> foo)
0 -fl (foo sl1 -> foo sl2 -> foo)
0 -bl (foo sl1 -> foo sl2 -> foo)
0 -bfl (foo sl1 -> foo sl2 -> foo)
0 -bl (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo)
0 -bfl (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo)
1 [cp: `foo' and `hardlink' are the same file] (foo hardlink)
1 -d [cp: `foo' and `hardlink' are the same file] (foo hardlink)