summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorPaul Eggert <eggert@cs.ucla.edu>2007-06-18 11:58:47 +0200
committerJim Meyering <jim@meyering.net>2007-06-18 15:20:44 +0200
commit811901cb41421e315c4b2e04989fb380ec20cb33 (patch)
treee82183001e237bd1316f5dce4577542dbbc5977d /src
parent0adc02a086c8a40bad3191d14b8612aae5ee9995 (diff)
downloadcoreutils-811901cb41421e315c4b2e04989fb380ec20cb33.tar.xz
A few more symlink-related fixes. Fix a bug triggered by cp
--parents and symlinks. Close some race conditions possible when the destination replaces a newly-created file with a symlink. * NEWS: Document that 'cp --parents' no longer mishandles symlinks in file name components of source. * src/copy.c (HAVE_LCHOWN): Default to false. (lchown) [!defined HAVE_LCHOWN]: Define to chown, for convenience. * src/cp.c (lchown) [!HAVE_LCHOWN]: Likewise. * src/install.c (lchown [!HAVE_LCHOWN]: Likewise. * src/copy.c (set_owner): Use lchown instead of chown, for safety in case the file got replaced by a symlink in the meantime. * src/cp.c (re_protect): Likewise. * src/install.c (change_attributes): Likewise. * src/copy.c (copy_internal): Use ordinary C rather than an #if. * src/cp.c (lchown) [!HAVE_LCHOWN]: Define to chown, for convenience. (struct dir_attr): Cache the entire struct stat of the directory, rather than just its mode, so that we needn't stat the directory twice (which can lead to races). (re_protect): Don't use XSTAT as that's not appropriate in this context (symlinks should be followed here). Instead, use the cached stat value. (make_dir_parents_private): Save dir's entire struct stat, not just its mode. * tests/cp/cp-parents: Add test to check against bug with cp --parents and symlinks.
Diffstat (limited to 'src')
-rw-r--r--src/copy.c31
-rw-r--r--src/cp.c35
-rw-r--r--src/install.c6
3 files changed, 41 insertions, 31 deletions
diff --git a/src/copy.c b/src/copy.c
index fd0519096..fec8c231b 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -62,6 +62,11 @@
# define fchown(fd, uid, gid) (-1)
#endif
+#ifndef HAVE_LCHOWN
+# define HAVE_LCHOWN false
+# define lchown(name, uid, gid) chown (name, uid, gid)
+#endif
+
#define SAME_OWNER(A, B) ((A).st_uid == (B).st_uid)
#define SAME_GROUP(A, B) ((A).st_gid == (B).st_gid)
#define SAME_OWNER_AND_GROUP(A, B) (SAME_OWNER (A, B) && SAME_GROUP (A, B))
@@ -172,7 +177,9 @@ copy_dir (char const *src_name_in, char const *dst_name_in, bool new_dst,
/* Set the owner and owning group of DEST_DESC to the st_uid and
st_gid fields of SRC_SB. If DEST_DESC is undefined (-1), set
- the owner and owning group of DST_NAME instead. DEST_DESC must
+ the owner and owning group of DST_NAME instead; for
+ safety prefer lchown if the system supports it since no
+ symbolic links should be involved. DEST_DESC must
refer to the same file as DEST_NAME if defined.
Return 1 if the syscall succeeds, 0 if it fails but it's OK
not to preserve ownership, -1 otherwise. */
@@ -188,7 +195,7 @@ set_owner (const struct cp_options *x, char const *dst_name, int dest_desc,
}
else
{
- if (chown (dst_name, uid, gid) == 0)
+ if (lchown (dst_name, uid, gid) == 0)
return 1;
}
@@ -212,6 +219,9 @@ static void
set_author (const char *dst_name, int dest_desc, const struct stat *src_sb)
{
#if HAVE_STRUCT_STAT_ST_AUTHOR
+ /* FIXME: Modify the following code so that it does not
+ follow symbolic links. */
+
/* Preserve the st_author field. */
file_t file = (dest_desc < 0
? file_name_lookup (dst_name, 0, 0)
@@ -1909,20 +1919,21 @@ copy_internal (char const *src_name, char const *dst_name,
{
/* Preserve the owner and group of the just-`copied'
symbolic link, if possible. */
-#if HAVE_LCHOWN
- if (lchown (dst_name, src_sb.st_uid, src_sb.st_gid) != 0
+ if (HAVE_LCHOWN
+ && lchown (dst_name, src_sb.st_uid, src_sb.st_gid) != 0
&& ! chown_failure_ok (x))
{
error (0, errno, _("failed to preserve ownership for %s"),
dst_name);
goto un_backup;
}
-#else
- /* Can't preserve ownership of symlinks.
- FIXME: maybe give a warning or even error for symlinks
- in directories with the sticky bit set -- there, not
- preserving owner/group is a potential security problem. */
-#endif
+ else
+ {
+ /* Can't preserve ownership of symlinks.
+ FIXME: maybe give a warning or even error for symlinks
+ in directories with the sticky bit set -- there, not
+ preserving owner/group is a potential security problem. */
+ }
}
}
else
diff --git a/src/cp.c b/src/cp.c
index 1d66bc382..9436be6fb 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -36,6 +36,10 @@
#include "utimens.h"
#include "acl.h"
+#if ! HAVE_LCHOWN
+# define lchown(name, uid, gid) chown (name, uid, gid)
+#endif
+
#define ASSIGN_BASENAME_STRDUPA(Dest, File_name) \
do \
{ \
@@ -56,7 +60,7 @@
need to be fixed after copying. */
struct dir_attr
{
- mode_t mode;
+ struct stat st;
bool restore_mode;
size_t slash_offset;
struct dir_attr *next;
@@ -290,17 +294,8 @@ re_protect (char const *const_dst_name, size_t src_offset,
for (p = attr_list; p; p = p->next)
{
- struct stat src_sb;
-
dst_name[p->slash_offset] = '\0';
- if (XSTAT (x, src_name, &src_sb))
- {
- error (0, errno, _("failed to get attributes of %s"),
- quote (src_name));
- return false;
- }
-
/* Adjust the times (and if possible, ownership) for the copy.
chown turns off set[ug]id bits for non-root,
so do the chmod last. */
@@ -309,8 +304,8 @@ re_protect (char const *const_dst_name, size_t src_offset,
{
struct timespec timespec[2];
- timespec[0] = get_stat_atime (&src_sb);
- timespec[1] = get_stat_mtime (&src_sb);
+ timespec[0] = get_stat_atime (&p->st);
+ timespec[1] = get_stat_mtime (&p->st);
if (utimens (dst_name, timespec))
{
@@ -322,7 +317,7 @@ re_protect (char const *const_dst_name, size_t src_offset,
if (x->preserve_ownership)
{
- if (chown (dst_name, src_sb.st_uid, src_sb.st_gid) != 0
+ if (lchown (dst_name, p->st.st_uid, p->st.st_gid) != 0
&& ! chown_failure_ok (x))
{
error (0, errno, _("failed to preserve ownership for %s"),
@@ -333,12 +328,12 @@ re_protect (char const *const_dst_name, size_t src_offset,
if (x->preserve_mode)
{
- if (copy_acl (src_name, -1, dst_name, -1, src_sb.st_mode))
+ if (copy_acl (src_name, -1, dst_name, -1, p->st.st_mode) != 0)
return false;
}
else if (p->restore_mode)
{
- if (lchmod (dst_name, p->mode) != 0)
+ if (lchmod (dst_name, p->st.st_mode) != 0)
{
error (0, errno, _("failed to preserve permissions for %s"),
quote (dst_name));
@@ -421,14 +416,14 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
int src_errno;
/* This component does not exist. We must set
- *new_dst and new->mode inside this loop because,
+ *new_dst and new->st.st_mode inside this loop because,
for example, in the command `cp --parents ../a/../b/c e_dir',
make_dir_parents_private creates only e_dir/../a if
./b already exists. */
*new_dst = true;
- src_errno = (stat (src, &stats) != 0
+ src_errno = (stat (src, &new->st) != 0
? errno
- : S_ISDIR (stats.st_mode)
+ : S_ISDIR (new->st.st_mode)
? 0
: ENOTDIR);
if (src_errno)
@@ -437,7 +432,7 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
quote (src));
return false;
}
- src_mode = stats.st_mode;
+ src_mode = new->st.st_mode;
/* If the ownership or special mode bits might change,
omit some permissions at first, so unauthorized users
@@ -485,7 +480,7 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
if (omitted_permissions & ~stats.st_mode
|| (stats.st_mode & S_IRWXU) != S_IRWXU)
{
- new->mode = stats.st_mode | omitted_permissions;
+ new->st.st_mode = stats.st_mode | omitted_permissions;
new->restore_mode = true;
}
}
diff --git a/src/install.c b/src/install.c
index dc7ff0141..39d514957 100644
--- a/src/install.c
+++ b/src/install.c
@@ -62,6 +62,10 @@ static bool use_default_selinux_context = true;
# define endpwent() ((void) 0)
#endif
+#if ! HAVE_LCHOWN
+# define lchown(name, uid, gid) chown (name, uid, gid)
+#endif
+
/* Initial number of entries in each hash table entry's table of inodes. */
#define INITIAL_HASH_MODULE 100
@@ -606,7 +610,7 @@ change_attributes (char const *name)
want to know. */
if (! (owner_id == (uid_t) -1 && group_id == (gid_t) -1)
- && chown (name, owner_id, group_id) != 0)
+ && lchown (name, owner_id, group_id) != 0)
error (0, errno, _("cannot change ownership of %s"), quote (name));
else if (chmod (name, mode) != 0)
error (0, errno, _("cannot change permissions of %s"), quote (name));