bugfindutils - Bugs: bug #11148, find does not report symlink loop...

 
 

bug #11148: find does not report symlink loop when trying to follow symlinks

Submitter:  Geoff Clare <geoffclare>
Submitted:  Sat 27 Nov 2004 11:41:34 AM UTC
   
 
Category:  find Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  jay
Originator Name:  Open/Closed:  Closed
Release:  4.2.8 Fixed Release:  4.2.9
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 05 Dec 2004 05:20:12 PM UTC, comment #7: 

You can download a release of findutils in which this problem is
fixed from ftp://alpha.gnu.org/gnu/findutils.

The releases on alpha.gnu.org are for testing purposes, so please
take the time to download the release and verify that your
problem has been solved.  Once the release has been sufficiently
tested, it can be uploaded to ftp.gnu.org for everybody to use it.

James Youngman <jay>
Group administrator
Mon 29 Nov 2004 12:39:29 PM UTC, comment #6: 

I would say that this is the appropriate way to handle each of the standard stat() errors:

  switch (errno)
    {
    case ENOENT:
    case ENOTDIR:
      return lstat(name, p);
 
    case EACCES:
    case EIO:
    case ELOOP:
    case ENAMETOOLONG:
    case EOVERFLOW:
    default:
      return prev_rv;
    }

It may be that there are also some non-standard errors that do indicate non-existence (ENOLINK perhaps?), in which case they should go in with ENOENT and ENOTDIR (protected by #ifdef's).

Geoff Clare <geoffclare>
Sun 28 Nov 2004 11:56:15 PM UTC, comment #5: 

Well, perhaps confusingly we currently treat EACCES, ELOOP and ENOENT the same.  fallback_stat() is called when stat() fails (which obviously never happens in the default case, because stat() is only called if -H or-L is in effect).  That function teats the errors similarly:-

static int
fallback_stat(const char *name, struct stat *p, int prev_rv)
{
  /* Our original stat() call failed.  Perhaps we
   * can't follow a symbolic link.  If that might be
   * the problem, lstat() the link.
   * Otherwise, admit defeat.
   */
  switch (errno)
    {
    case ENOENT:
    case EACCES:
    case ELOOP:
      return lstat(name, p);

    case ENAMETOOLONG:
    default:
      return prev_rv; /* lstat() won't help. */
    }
}

From your earlier remarks, I assume that you will regard this implementation as being unacceptable.  What would you suggest?

James Youngman <jay>
Group administrator
Sat 27 Nov 2004 10:27:45 PM UTC, comment #4: 

Just realised another thing about ELOOP.  There are two ways that it can occur.  One is an actual loop (which I used in the original report as it was easiest to set up).  However, the other is a sequence of symlinks that exceeds SYMLOOP_MAX.  In this case the situation is similar to EACCES - it is not possible to tell whether the file exists or not.  (Had SYMLOOP_MAX been greater, maybe following a few more symlinks would have reached a real file, or maybe it wouldn't.)  I think this is reason enough to treat ELOOP the same as EACCES, not the same as ENOENT.

Geoff Clare <geoffclare>
Sat 27 Nov 2004 09:35:04 PM UTC, comment #3: 

The relevant part of the normative text in POSIX is "If the referenced file does not exist, the file information and type shall be for the link itself". The condition "does not exist" is a particular type of error for which this behaviour is required, and only those stat() errors which definitely indicate non-existence should produce this behaviour.  Some errors such as ENOENT and ENOTDIR clearly indicate that the file does not exist, whereas I would say that EACCES clearly does not.  (EACCES constitutes an error that is encountered while trying to determine whether the file exists - when it occurs it is not possible to know whether the file exists or not, so the error should be reported).  ELOOP I suppose could be thought of as indicating non-existence, but I didn't see it that way.  Neither it seems did IBM, as the latest version of AIX (which is UNIX03 certified) reports an error when ELOOP is encountered.  You might want to check the behaviour of Solaris 10 as well when it comes out in January.

Geoff Clare <geoffclare>
Sat 27 Nov 2004 07:41:07 PM UTC, comment #2: 

You will also notice that we visit "deepfile" twice when -L is in effect; this is because the directory containing it is part of a loop, but not part of an infinite loop.

James Youngman <jay>
Group administrator
Sat 27 Nov 2004 07:24:18 PM UTC, comment #1: 

Summary - I have fixed bugs, but not the one you reported.  Please read on to discover what I did and why.

I disagree with your interpretation.  My reading of the POSIX standard tis that the find utility is only required to diagnose an infinite loop if it enters a directory forming part of that loop.  Hence I don't think the specific example you gave is entirely appropriate.   The diagnostic message is presumably intended to warn the user that the output of find may not be as they expect.  

However, there are (were) indeed closely related problems with findutils-4.2.8.  There are three interesting cases:

1. Infinite cycles involving hard links to directories.    These were previously detected and find used to silently avoid visiting any particular directory entry which was a hard link to one of its ancestors (where the ancestor had not already been visited, the loop would be detected later).
The effect of the leaf optimisation is frequently such that find knows that it doesn't need to examine such entries anyway (the deep directory entry does not contribute to the link count of its immediate parent).   I have implemented a bugfix in the CVS code, which is to issue a diagnostic when this happens, instead of silently avoiding such cycles.

2. Loops including symbolic links.  There were problems diagnosing this since the -L option had failed to imply -noleaf, contrary to what the documentation said.   Your bug report led me to check this, and so I have just fixed this in CVS.

3. Symbolic links which cause ELOOP (i.e. your example).  I think that in this case findutils-4.2.8 treats these as if they were broken symbolic links.  I think that's a reasonable interpretation.  The (admittedly non-normative) RATIONALE text states:

"Since the -L option resolves all symbolic links and the -type l primary is true for symbolic links that still exist after symbolic links have been resolved, the command:

find -L . -type l

prints a list of symbolic links reachable from the current directory that do not resolve to accessible files."

Since symlinks pointing to each other do not resolve to accessible files (or directories) I think it is reasonable for "find -L . -type l" to print them.  Does your interpretation match this?  If not, can you explain why you expect something else?  

I attach a compressed tar file which contains a "symbolic link torture test" for "find -L".  I also insert below the result of running the development code on it.  The second attachment is a hand-crafted ext2 filesystem image which has an infinite loop in it.  Please take care when testing with the filesystem image, it may not be kind to your system!

--- symbolic link torture test result ---
$ set -x ; ls -lR   /tmp/testdir/symloop ; ./find -L  /tmp/testdir/symloop -ls
+ ls -lR /tmp/testdir/symloop
/tmp/testdir/symloop:
total 1
lrwxrwxrwx  1 james users    2 2004-11-27 18:40 a -> ..
lrwxrwxrwx  1 james users   17 2004-11-27 18:42 b -> subdir/mid/bottom
lrwxrwxrwx  1 james users    3 2004-11-27 19:04 boo -> far
lrwxrwxrwx  1 james users    3 2004-11-27 19:04 far -> boo
drwxr-xr-x  3 james users 1024 2004-11-27 18:50 subdir

/tmp/testdir/symloop/subdir:
total 1
drwxr-xr-x  3 james users 1024 2004-11-27 18:50 mid

/tmp/testdir/symloop/subdir/mid:
total 1
drwxr-xr-x  2 james users 1024 2004-11-27 18:49 bottom

/tmp/testdir/symloop/subdir/mid/bottom:
total 1
-rw-r--r--  1 james users 6 2004-11-27 18:49 deepfile
+ ./find -L /tmp/testdir/symloop -ls
 16636    1 drwxr-xr-x   3 james    users        1024 Nov 27 19:04 /tmp/testdir/symloop
 16635    1 drwxr-xr-x   3 james    users        1024 Nov 27 19:06 /tmp/testdir/symloop/a
./find: Filesystem loop detected; `/tmp/testdir/symloop/a/symloop' has the same device number and inode as a directory which is 2 levels higher in the filesystem hierarchy.
 16636    1 drwxr-xr-x   3 james    users        1024 Nov 27 19:04 /tmp/testdir/symloop/a/symloop
 16646    1 -rw-r--r--   1 james    users         311 Nov 27 19:06 /tmp/testdir/symloop/a/symloop.tar.gz
 16638    1 drwxr-xr-x   2 james    users        1024 Nov 27 18:49 /tmp/testdir/symloop/b
 16642    1 -rw-r--r--   1 james    users           6 Nov 27 18:49 /tmp/testdir/symloop/b/deepfile
 16639    1 drwxr-xr-x   3 james    users        1024 Nov 27 18:50 /tmp/testdir/symloop/subdir
 16640    1 drwxr-xr-x   3 james    users        1024 Nov 27 18:50 /tmp/testdir/symloop/subdir/mid
 16638    1 drwxr-xr-x   2 james    users        1024 Nov 27 18:49 /tmp/testdir/symloop/subdir/mid/bottom
 16642    1 -rw-r--r--   1 james    users           6 Nov 27 18:49 /tmp/testdir/symloop/subdir/mid/bottom/deepfile
 16643    0 lrwxrwxrwx   1 james    users           3 Nov 27 19:04 /tmp/testdir/symloop/far -> boo
 16644    0 lrwxrwxrwx   1 james    users           3 Nov 27 19:04 /tmp/testdir/symloop/boo -> far


James Youngman <jay>
Group administrator
Sat 27 Nov 2004 11:41:34 AM UTC, original submission:  

In the following situation, find should report an error:

ln -s a b
ln -s b a
find -L a -print

Likewise for -H.  Instead, find is doing an lstat() when the stat() fails (with ELOOP).

The optionl_stat() and optionh_stat() functions need to distinguish been different stat() errors, and only do the lstat() when appropriate. They should do the lstat() if stat() failed with ENOENT.  It may be appropriate for some other errors as well (e.g. ENOTDIR), but definitely not for ELOOP.

Geoff Clare <geoffclare>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #2112:  ext2-fs-withloop.bin.gz added by jay (11KiB - application/x-tar - CAUTION: ext2 filesystem with loop)
file #2111:  symloop.tar.gz added by jay (311B - application/x-tar - Symbolic loop torture test)

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

Only logged-in users can vote.

 

Follow 6 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2004-12-05 jay Open/ClosedOpen Closed
    Fixed ReleaseNone 4.2.9
2004-12-04 jay StatusNone Fixed
2004-11-27 jay Attached File- Added ext2-fs-withloop.bin.gz, #1923
2004-11-27 jay Assigned toNone jay
    Attached File- Added symloop.tar.gz, #1922

Back to the top

Powered by Savane 3.13-f8d8.
Corresponding source code