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:
>
>>> Yes, it's total overkill for this application.  Another thing: we
>>> don't need to resolve internal symlinks, just the last one.
>>
>> Why would we want to resolve *any* of them, if we can get
>> the dir-fd, ent-name pair I mentioned?
>
> Sorry, I lost context; but I take your point to mean is that if the
> destination /x/y/z is a symlink to /a/b/c, and /a/b/c is a symlink to
> /d/e/f, and /d/e/f is not a symlink, then we want to open /d/e/f with
> O_EXCL.

In that case, yes.  However, all of those symlink values are absolute.
The tricky part comes when you have relative ones.
Imagine relative symlink chains
  1 -> ../2 -> ../3 -> ../4 -> ...
or
  a -> b/a -> b/a -> b/a

Each relative link is "relative" to its position in the file system
hierarchy, so you can't open the link value directly.

To construct the final referent you have to choose:

 1) either accumulate the full, relative name.
      Then, you can use open, but it may fail with ENAMETOOLONG.

 2) or traverse using *at functions and return the final dirfd,entname pair
      Then you can open via openat (dirfd, entname, flags), but only if
      all intermediate directories are readable.

 3) or use a hybrid approach:
      use *at functions when you can, else accumulate multiple components.
      Then, you lose only when there are so many/long components in
      consecutive, unreadable directories that an accumulated name ends
      up being longer than PATH_MAX.  Not likely.

Such symlink chains are unusual, and not generally useful:

  $ ( for i in $(seq 10); do ln -s b/a && mkdir b && cd b;done )
  $ find b
  b
  b/a
  b/b
  b/b/a
  b/b/b
  b/b/b/a
  b/b/b/b
  b/b/b/b/a
  b/b/b/b/b
  b/b/b/b/b/a
  b/b/b/b/b/b
  b/b/b/b/b/b/a
  b/b/b/b/b/b/b
  b/b/b/b/b/b/b/a
  b/b/b/b/b/b/b/b
  b/b/b/b/b/b/b/b/a
  b/b/b/b/b/b/b/b/b
  b/b/b/b/b/b/b/b/b/a
  b/b/b/b/b/b/b/b/b/b
  $ cat a
  cat: a: Too many levels of symbolic links
  [Exit 1]

[BTW, use "8" in place of "10" above, and cat fails with ENOENT on Linux.
 On Solaris 10, you can go up to 20 before getting ELOOP:
   cat: a: Number of symbolic links encountered during path name \
     traversal exceeds MAXSYMLINKS
]

So, at least on some systems, with such an example, there'd be no
need to do anything, because the initial open would fail with ELOOP,
and not ENOENT.  Hmm... this is reminiscent of the test that led to
remove.c's non-handling of ELOOP.

While with Linux's current low limit (even with 255-byte dir names),
you can't trigger ENAMETOOLONG, on Solaris 10 and the Hurd, you can.
So at least it is possible to write a test.

> We don't care whether /x or /x/y or /a or /a/b or /d or /d/e
> are symlinks.  But canonicalize.c cares, and does more work than we
> want.
>
>>> were based on some existing coreutils code that I think is wrong in
>>> this area as well.
>>
>> Oh!  Then I'll hunt for it tomorrow if you don't tell first.
>
> If you like, here's the code I have right now: look for the patches
> that replace XSTAT to see the problem areas.  The XSTAT part of this
> patch is clearly incorrect (it doesn't pass the test cases) but the
> rest of the patch feels "right" and I hope the XSTAT part is on the
> right track.  If I make further progress I'll let you know but I have
> to work on other stuff right now.

Thanks for taking the time.
At first glance this looks good.
As you say, it'll take some time (and more tests)
to do this properly.

> 2007-06-13  Paul Eggert  <address@hidden>
>       and Jim Meyering  <address@hidden>
>
>       * src/copy.c: Include "canonicalize.h".
>       (copy_reg): When following a symlink, used 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.
>       Use canonicalize_filename_mode to follow the symlink, so that we
>       can always open with O_EXCL and avoid a race.
>       (copy_internal): Follow destination symlinks when copying a regular
>       file, regardless of whether following source symlinks, since POSIX
>       and tradition says we should always follow destination symlinks if
>       the system calls would ordinarily do so.
>       * src/cp.c (make_dir_parents_private): Likewise.
>
>         THIS PATCH IS NOT CORRECT.  <-- (warning if you didn't read the above)