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

 
 

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

Submitted by:  Geoff Clare <geoffclare>
Submitted on:  Mon 16 Apr 2007 10:25:25 AM UTC  
 
Category: findSeverity: 3 - Normal
Item Group: Wrong resultStatus: Fixed
Privacy: PublicAssigned to: James Youngman <jay>
Originator Name: Geoff ClareOpen/Closed: Closed
Release: 4.3.3Fixed Release: 4.3.4

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 17 Apr 2007 11:23:34 AM UTC, comment #8:

You are still trying to bring the POSIX requirements for directory loop detection into this, which I have already pointed out are a completely different thing and not relevant. The requirements for a diagnostic in the symlink loop case come from XCU section 1.11 Utility Description Defaults, under the heading "CONSEQUENCES OF ERRORS":

"If the requested action cannot be performed on an operand representing a file, directory, user, process, and so on, the utility shall issue a diagnostic message to standard error and continue processing the next operand in sequence, but the final exit status shall be returned as non-zero.

For a utility that recursively traverses a file hierarchy (such as find or chown -R), if the requested action cannot be performed on a file or directory encountered in the hierarchy, the utility shall issue a diagnostic message to standard error and continue processing the remaining files in the hierarchy, but the final exit status shall be returned as non-zero."

Part of the "requested action", when the -L option is used, is to determine whether symlinks point to a directory or not, and to descend into the directory if they do.

As regards -H, I believe the "referenced file does not exist" part is related to dangling links (i.e. stat() fails with ENOENT) and should not be applied in the ELOOP case. AIX, HP-UX and Solaris all report an error for a path operand that is a symlink loop with both -L and -H.

Geoff Clare <geoffclare>
Tue 17 Apr 2007 10:27:11 AM UTC, comment #7:

Executive summary: I agree that a diagnostic should be issued when -L is in effect, but not for the reason you originally stated.

I would regard the "find -L a/foo" problem (which also affects -P and -H) to be a separate (and catastrophic) bug. I've logged a sepaarte bug for it.

When -L is in effect, if an attempt to stat("x") fails with ELOOP for some directory entry x, find will not try to recurse into x.

If find does not descend into "x", is it required to issue a diagnostic under the infinite loop rule? My reading of http://www.opengroup.org/onlinepubs/009695399/utilities/find.html is that the diagnostic is only required for directories find actually enters (or tries to enter) and which are ancestors.

In this example I can see two reasons why a diagnostic would not be required:
1. x (or in our earlier example a and b) does not point to an ancestor of any directory actually visited.
2. x is not entered.

I think either of reasons (1) and (2) should be sufficient to avoid a diagnostic required by the statement:

<< The find utility shall detect infinite loops; that is, entering a previously visited directory that is an ancestor of the last file encountered. >>

You write:

<<
With the -L option find follows symlinks, so all the pathnames it writes to stdout should be files that exist when symlinks are followed. If "a" is a symlink loop it effectively does not exist when symlinks are followed, because the pathname does not resolve successfully.

>>


This, I believe, is a very good reason to omit printing names corresponding to symlink loops. I'm not completely convinced that a diagnostic should be issued in this case. However, I can understand that the user might want to be able to control this. Control over this sort of thing is essentially already provided though, via -L, -P and -H.

If the user does not care about symbolic link destinations, they will be in -P mode. Hence I would interpret use of -L as indicating that the user cares about where symbolic links point, and so I believe that find should be modified to issue a diagnostic in this case.

I believe at this stage that "find -H a" should behave similarly to "find -P a" when a is a symlink loop, since the "referenced file does not exist" (that is, no file is referenced). Therefore for that case, no diagnostic.

James Youngman <jay>
Project AdministratorIn charge of this item.
Tue 17 Apr 2007 09:40:26 AM UTC, comment #6:

On second thoughts, d_type is a red herring. With the -L option, d_type doesn't provide enough information for symlinks - find needs to know what type of file the symlink points to (in order to tell whether the symlink points to a directory that it should descend into).

Geoff Clare <geoffclare>
Tue 17 Apr 2007 09:28:05 AM UTC, comment #5:

I take your point about d_type (the system I tried it on does have d_type). I still think that find should report this error. With the -L option find follows symlinks, so all the pathnames it writes to stdout should be files that exist when symlinks are followed. If "a" is a symlink loop it effectively does not exist when symlinks are followed, because the pathname does not resolve successfully.

Here is a slightly different case that may be more convincing:

$ ln -s a b
$ ln -s b a
$ find -L a/foo
a/foo

Here find is reporting that a file called a/foo exists, which is obviously not the case.

P.S. I have tried the patch, and I can confirm that it fixes the problem.

Geoff Clare <geoffclare>
Tue 17 Apr 2007 08:51:04 AM UTC, comment #4:

But find does nto need to stat() a directory entry in order to discover that the entry itself is not a directory. Using -size causes the loop tobe diagnosed, because that requries a call to stat(). Determining that an item is not a directory requires no stat call because that information is present in struct dirent (on some systems at least).

As far as I can see, the output that find produces on stdout is correct and matches the criteria passed to find by the user in this case.

I believe that the current behaviour is standard-conforming unless find is required to diagnose all symlink loops (even for nondirectories). I am not aware of any such requirement.

James Youngman <jay>
Project AdministratorIn charge of this item.
Tue 17 Apr 2007 08:39:37 AM UTC, comment #3:

You are confusing directory loops and symlink loops, which are completely different things. In a directory loop, a symlink points up to a parent directory of the symlink. A stat() of the symlink succeeds. In a symlink loop the symlink ends up (possibly through other symlinks) pointing to itself. A stat() of the symlink fails with ELOOP. The POSIX requirements relating to detection of directory loops have no bearing on this problem, which is about symlink loops.

It's interesting that adding -size produces the desired result, but it shouldn't make any difference. With -L (or -H), find needs to stat() each path operand in order to tell whether it resolves to a directory that it should descend into.

You had your doubts about bug 11148 as well, but in the end you decided to make the change I requested.

The current versions of AIX, HP-UX and Solaris all report the expected error, the same as findutils 4.2.28 does. I can see no justification for making 4.3.3 behave differently.

Geoff Clare <geoffclare>
Tue 17 Apr 2007 07:51:02 AM UTC, comment #2:

Thinking about this again, I am less convinced this is a bug. AIUI find is required to diagnose directory loops. The standard says:

<< The find utility shall detect infinite loops; that is, entering a previously visited directory that is an ancestor of the last file encountered. When it detects an infinite loop, find shall write a diagnostic message to standard error and shall either recover its position in the hierarchy or terminate. >>

In this case however, neither a nor b point to a directory and hence do not constitute a directory loop. Therefore it is not obvious to me that findutils is obliged to diagnose anything. If you include a predicate which requires stat() to succeed, a diagnosis is made:

orbital:tmp$ ../find --version | head -1
GNU find version 4.3.3
orbital:tmp$ /bin/ls -l
total 0
lrwxrwxrwx 1 james users 1 2007-04-17 08:39 a -> b
lrwxrwxrwx 1 james users 1 2007-04-17 08:39 b -> a
orbital:tmp$ ../find -L
.
./a
./b
orbital:tmp$ ../find -L -size 2
../find: ./a: Too many levels of symbolic links
../find: ./b: Too many levels of symbolic links
orbital:tmp$ echo $?
1

(I just tested this on NetBSD also, and there was a failure which I am about to log as a separate bug)

James Youngman <jay>
Project AdministratorIn charge of this item.
Mon 16 Apr 2007 08:23:46 PM UTC, comment #1:

Ouch. Fixed by the attached patch. The patch also fixes a problem with the test suite (not detecting some cases whetre a test should fail) and a problem in oldfind which relates to using out-of-date stat information.

I have not yet committed this to CVS.

(file #12505)

James Youngman <jay>
Project AdministratorIn charge of this item.
Mon 16 Apr 2007 10:25:25 AM UTC, original submission:

This is effectively the same problem as bug 11148 (fixed long ago) but now in the fts version of the code.

$ ln -s a b
$ ln -s b a
$ find --version
GNU find version 4.3.3
Built using GNU gnulib version 2007-04-14
Features enabled: D_TYPE O_NOFOLLOW(enabled) LEAF_OPTIMISATION FTS(FTS_CWDFD) CBO(level=0)
$ find -L a
a

It should report an error when it gets ELOOP, as with the non-fts version of find:

$ find-4.2.28 -L a
find-4.2.28: a: Too many levels of symbolic links

The original fts code also did not have the problem. It has come in since the last time I tried the fts version (from CVS in August 2005).

Geoff Clare <geoffclare>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #12505:  loop.patch added by jay (9KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jay (Updated the item)
  • -unavailable- added by geoffclare (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 21 Apr 2007 01:50:52 PM UTCjayOpen/ClosedOpen=>Closed
      Fixed ReleaseNone=>4.3.4
    Tue 17 Apr 2007 10:50:26 PM UTCjayStatusNone=>Fixed
    Tue 17 Apr 2007 10:27:11 AM UTCjayAssigned toNone=>jay
    Mon 16 Apr 2007 08:23:46 PM UTCjayAttached File-=>Added loop.patch, #12505

    Back to the top


    Powered by Savane 3.1-cleanup1