Re: Bugfix: Failure of -execdir to process arguments at depth 0

[James Youngman]

On Sun, Feb 06, 2005 at 07:20:28PM +0300, Dmitry V. Levin wrote:
> Hi,
> 
> On Sat, Feb 05, 2005 at 12:19:55PM +0000, James Youngman wrote:
> [...]

> > Therefore I think there won't be any corner cases around
> > inaccessible directories.

> Unfortunately, they are.
> 
> Imagine this test case:
> $ mkdir dir1
> $ mkdir dir1/dir2
> $ touch dir1/dir2/foo
> $ ln -s dir1 link1
> $ find-4.2.15 link1/dir2 -type f
> link1/dir2/foo
> $ find-4.2.16 link1/dir2 -type f
> find: link1
> $ find-4.2.16 -H link1/dir2 -type f
> link1/dir2/foo
> 
> This happens because of "find -P" (never follow symbolic links) is the
> default behaviour, and starting point "link1" is symlink while starting
> point "link1/dir2" is not.
> 
> You can try to fix this bug by temporarily allowing to follow symlinks
> during this "first chdir" in process_top_path(), but I'm not certainly
> sure.

That certainly seems to work - checked in a fix into CVS.  I have also
added a test case to the test suite that detects this problem.  This
is the code patch :-

Index: find.c
===================================================================
RCS file: /cvsroot/findutils/findutils/find/find.c,v
retrieving revision 1.64
diff -u -r1.64 find.c
--- find.c      6 Feb 2005 09:24:42 -0000       1.64
+++ find.c      7 Feb 2005 00:38:03 -0000
@@ -110,6 +110,12 @@
 /* The stat buffer of the initial working directory. */
 struct stat starting_stat_buf;
 
+enum ChdirSymlinkHandling
+  {
+    SymlinkHandleDefault,      /* Normally the right choice */
+    SymlinkFollowOk            /* see comment in process_top_path() */
+  };
+
 
 enum TraversalDirection
   {
@@ -851,10 +857,11 @@
 /* Safely perform a change in directory.
  *
  */
-static int 
+static enum SafeChdirStatus
 safely_chdir(const char *dest,
             enum TraversalDirection direction,
-            struct stat *statbuf_dest)
+            struct stat *statbuf_dest,
+            enum ChdirSymlinkHandling symlink_handling)
 {
   struct stat statbuf_arrived;
   int rv, dotfd=-1;
@@ -878,12 +885,45 @@
       if (0 == options.xstat(dest, statbuf_dest))
        {
 #ifdef S_ISLNK
+         /* symlink_handling might be set to SymlinkFollowOk, which
+          * would allow us to chdir() into a symbolic link.  This is
+          * only useful for the case where the directory we're
+          * chdir()ing into is the basename of a command line
+          * argument, for example where "foo/bar/baz" is specified on
+          * the command line.  When -P is in effect (the default),
+          * baz will not be followed if it is a symlink, but if bar
+          * is a symlink, it _should_ be followed.  Hence we need the
+          * ability to override the policy set by following_links().
+          */
          if (!following_links() && S_ISLNK(statbuf_dest->st_mode))
            {
-             rv = SafeChdirFailSymlink;
-             rv_set = true;
-             saved_errno = 0;  /* silence the error message */
-             goto fail;
+             /* We're not supposed to be following links, but this is 
+              * a link.  Check symlink_handling to see if we should 
+              * make a special exception.
+              */
+             if (symlink_handling == SymlinkFollowOk)
+               {
+                 /* We need to re-stat() the file so that the 
+                  * sanity check can pass. 
+                  */
+                 if (0 != stat(dest, statbuf_dest))
+                   {
+                     rv = SafeChdirFailNonexistent;
+                     rv_set = true;
+                     saved_errno = errno;
+                     goto fail;
+                   }
+               }
+             else
+               {
+                 /* Not following symlinks, so the attempt to
+                  * chdir() into a symlink should be prevented.
+                  */
+                 rv = SafeChdirFailSymlink;
+                 rv_set = true;
+                 saved_errno = 0;      /* silence the error message */
+                 goto fail;
+               }
            }
 #endif   
 #ifdef S_ISDIR
@@ -1058,6 +1098,7 @@
   else
     {
       enum TraversalDirection direction;
+      enum SafeChdirStatus chdir_status;
       struct stat st;
 
       dirchange = 1;
@@ -1065,10 +1106,26 @@
        direction = TraversingUp;
       else
        direction = TraversingDown;
-      if (SafeChdirOK != safely_chdir(parent_dir, direction, &st))
-       {
-         error (0, errno, "%s", parent_dir);
 
+      /* We pass SymlinkFollowOk to safely_chdir(), which allows it to
+       * chdir() into a symbolic link.  This is only useful for the
+       * case where the directory we're chdir()ing into is the
+       * basename of a command line argument, for example where
+       * "foo/bar/baz" is specified on the command line.  When -P is
+       * in effect (the default), baz will not be followed if it is a
+       * symlink, but if bar is a symlink, it _should_ be followed.
+       * Hence we need the ability to override the policy set by
+       * following_links().
+       */
+      chdir_status = safely_chdir(parent_dir, direction, &st, SymlinkFollowOk);
+      if (SafeChdirOK != chdir_status)
+       {
+         if (errno)
+           error (0, errno, "%s", parent_dir);
+         else
+           error (0, 0, "Failed to safely change directory into `%s'",
+                  parent_dir);
+           
          /* We can't process this command-line argument. */
          state.exit_status = 1;
          return;
@@ -1429,7 +1486,7 @@
 #if USE_SAFE_CHDIR
       if (strcmp (name, "."))
        {
-         enum SafeChdirStatus status = safely_chdir (name, TraversingDown, 
&stat_buf);
+         enum SafeChdirStatus status = safely_chdir (name, TraversingDown, 
&stat_buf, SymlinkHandleDefault);
          switch (status)
            {
            case SafeChdirOK:
@@ -1588,7 +1645,7 @@
              dir = parent;
            }
          
-         status = safely_chdir (dir, TraversingUp, &stat_buf);
+         status = safely_chdir (dir, TraversingUp, &stat_buf, 
SymlinkHandleDefault);
          switch (status)
            {
            case SafeChdirOK: