[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Bugfix: Failure of -execdir to process arguments at depth 0
From: |
James Youngman |
Subject: |
Re: Bugfix: Failure of -execdir to process arguments at depth 0 |
Date: |
Mon, 7 Feb 2005 00:43:28 +0000 |
User-agent: |
Mutt/1.3.28i |
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:
- Re: [patch] find/pred.c: basename versus base_name., (continued)
- Re: [patch] find/pred.c: basename versus base_name., Dmitry V. Levin, 2005/02/02
- Re: [patch] find/pred.c: basename versus base_name., Bas van Gompel, 2005/02/03
- Re: [patch] find/pred.c: basename versus base_name., Dmitry V. Levin, 2005/02/03
- Re: [patch] find/pred.c: basename versus base_name., Bas van Gompel, 2005/02/03
- Re: [patch] find/pred.c: basename versus base_name., James Youngman, 2005/02/03
- Bugfix: Failure of -execdir to process arguments at depth 0, James Youngman, 2005/02/05
- Re: Bugfix: Failure of -execdir to process arguments at depth 0, Bas van Gompel, 2005/02/05
- Re: Bugfix: Failure of -execdir to process arguments at depth 0, Dmitry V. Levin, 2005/02/05
- Re: Bugfix: Failure of -execdir to process arguments at depth 0, James Youngman, 2005/02/06
- Re: Bugfix: Failure of -execdir to process arguments at depth 0, Dmitry V. Levin, 2005/02/06
- Re: Bugfix: Failure of -execdir to process arguments at depth 0,
James Youngman <=
Re: [patch] find/pred.c: basename versus base_name., James Youngman, 2005/02/02