bin/176136: cp(1) fails to overwrite a symlnk pointing to a directory

[Akinori MUSHA]

>Number:         176136
>Category:       bin
>Synopsis:       cp(1) fails to overwrite a symlnk pointing to a directory
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Feb 14 08:00:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     Akinori MUSHA
>Release:        9.1-STABLE
>Organization:
>Environment:
FreeBSD 9.1-STABLE/amd64
>Description:
Our implementation of cp(1) cannot overwrite a symlink pointing to an existing directory.
This is because it always calls stat(2) on a destination file path even if -R (with -P by default) is given, where lstat(2) should be used instead.

Dragonfly BSD, OS X and OpenBSD all share the same problem with us.
NetBSD cp(1) has once had this problem but fixed it in 2006 (bin/cp.c rev.1.42).
GNU cp does not have this problem.
Solaris/OpenIndiana's cp(1) (/usr/xpg4/bin/cp) seems to have this problem.

As far as I read, SUSv4 is not very clear as to how cp(1) should do about the situation, but there should be no reason a symlink cannot be overwritten.

>How-To-Repeat:
% ln -s . foo
% mkdir bar
% ln -s .. bar/foo

Now we have <foo> and <bar/foo>.
Let's try copying <foo> to <bar/foo>.

% cp -RP foo bar/
cp: cannot overwrite directory bar/foo with non-directory foo

It failed!

% ls -l bar
total 1
lrwxr-xr-x  1 knu  knu  2 Feb 14 16:18 foo -> ..

It seems cp(1) decided not to copy the source file just because the destination symlink was resolved to a directory, but it is certainly not what you would expect.
>Fix:
Index: cp.c
===================================================================
--- cp.c	(revision 246770)
+++ cp.c	(working copy)
@@ -262,7 +262,7 @@ copy(char *argv[], enum op type, int fts
 	struct stat to_stat;
 	FTS *ftsp;
 	FTSENT *curr;
-	int base = 0, dne, badcp, rval;
+	int base = 0, dne, badcp, rval, sval;
 	size_t nlen;
 	char *p, *target_mid;
 	mode_t mask, mode;
@@ -383,8 +383,18 @@ copy(char *argv[], enum op type, int fts
 			continue;
 		}
 
+		/*
+                 * lstat(2) should be used if neither -H or -L is
+                 * given, or we will fail to overwrite an existing
+                 * symlink pointing to a directory.
+                 */
+		if (fts_options & (FTS_LOGICAL | FTS_COMFOLLOW))
+			sval = stat(to.p_path, &to_stat);
+                else
+			sval = lstat(to.p_path, &to_stat);
+
 		/* Not an error but need to remember it happened */
-		if (stat(to.p_path, &to_stat) == -1)
+		if (sval == -1)
 			dne = 1;
 		else {
 			if (to_stat.st_dev == curr->fts_statp->st_dev &&


>Release-Note:
>Audit-Trail:
>Unformatted: