Re: FYI: bug-fix: cp would fail to write through a dangling symlink

[Jim Meyering]

Paul Eggert <address@hidden> wrote:

> Jim Meyering <address@hidden> writes:
>
>> I'm glad it's only a problem in a very unusual situation
>> (copying through a dangling symlink) and on a system that is
...
> The basic idea is that if 'cp' detects a dangling symlink, it resolves
> the link using gnulib's canonicalize_filename_mode (name, CAN_ALL_BUT_LAST).
> It then uses O_EXCL on the resolved name.
>
> Obviously there is still a race between detecting the link and opening
> the destination file.  But that race is inevitable and cannot be
> worked around.  The race I'm worried about is different: it's the one
> that fools 'cp' into thinking it created the destination even though
> it didn't.

That sounds like a reasonable approach.
However, I don't like using canonicalize_filename_mode, partly
for its internal "xmalloc".  Technically, copy.c aspires to "librarihood",
so it is not supposed to exit on OOM errors, but there is precedent there,
and I'm not going to fix *that* right now.
Also, it would be subject to the PATH_MAX name length limitation.
Another thing not to like: it may use xgetcwd.  Here, we generally
don't need the entire absolute name.

What I really want is more specialized:
a function like canonicalize_filename_mode that
traverses (via openat, readlinkat, etc.) the symlinks
until it reaches the final one.  Then, assuming L is
the final link name, I'd want it
to return two things:
  1) basename (L)
  2) a file descriptor open on dirname (L)
With those, I can use openat, thus avoiding the PATH_MAX limitation,
while at the same time avoiding using functions like xmalloc.

Yes, I'll implement this pretty soon :)

FYI, here's the canonicalize_filename_mode-based solution:
[the less-limited version will be only slightly different]

diff --git a/src/copy.c b/src/copy.c
index b46221c..68e768f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -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"
@@ -353,21 +354,30 @@ 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_BINARY | O_EXCL;
+      dest_desc = open (dst_name, open_flags,
                        dst_mode & ~omitted_permissions);

       /* 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.  */
+        the open call, but this time with the name of the final,
+        missing directory entry.  */
       if (dest_desc < 0 && 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);
+           {
+             char *missing_name
+               = canonicalize_filename_mode (dst_name, CAN_ALL_BUT_LAST);
+             if (missing_name)
+               {
+                 dest_desc = open (missing_name, open_flags,
+                                   dst_mode & ~omitted_permissions);
+                 free (missing_name);
+               }
+           }
        }
     }
   else