bugfindutils - Bugs: bug #11879, all mount-points lstat()-ed, but...

 
 

bug #11879: all mount-points lstat()-ed, but shouldn't be

Submitter:  Tim Waugh <twaugh>
Submitted:  Mon 07 Feb 2005 10:23:13 AM UTC
   
 
Category:  find Severity:  3 - Normal
Item Group:  Wrong result Status:  Fixed
Privacy:  Public Assigned to:  jay
Originator Name:  Open/Closed:  Closed
Release:  4.2.15 Fixed Release:  4.2.18
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 01 Mar 2005 08:33:39 PM UTC, comment #9: 

Note - this change caused bug #12181.   That's fixed in CVS now, too.

James Youngman <jay>
Group administrator
Sun 20 Feb 2005 09:39:25 PM UTC, comment #8: 

Yes, this is platform dependent.  The Findutils manual explains everything.  See bug #12044.

James Youngman <jay>
Group administrator
Sun 20 Feb 2005 12:38:06 PM UTC, comment #7: 

Looks like this issue is now platform-dependent:
find version 4.2.18 no longer stats filesystems on GNU/Linux.

Dmitry V. Levin <ldv>
Sun 20 Feb 2005 07:20:28 AM UTC, comment #6: 

(From Martin Buchholz <martin.buchholz@sun.com>)

I still see hangs in find 4.2.18, despite the
ChangeLog entry that suggests this has been fixed.

Even find --version hangs with this stacktrace

(dbx) where
  [1] _lstat64(0x4, 0xffbff160, 0x1, 0xfeabbe60, 0x81010100, 0xff00), at 0xfeb1abac
=>[2] optionp_stat(name = 0x7a548 "/java/tl", p = 0xffbff160), line 229 in "find.c"
  [3] set_fstype_devno(p = 0x79b58), line 182 in "fstype.c"
  [4] get_mounted_devices(n = 0x71114), line 278 in "fstype.c"
  [5] init_mounted_dev_list(), line 668 in "find.c"
  [6] main(argc = 2, argv = 0xffbff3c4), line 448 in "find.c"

where /java/tl is the remotely mounted filesystem that
is "down" during command execution.

This is pretty serious.  In a large networked environment,
there is always some NFS server that is broken or very slow
at the moment.

The --version flag at the very least should be handled
before trying to "do" any real work.

Anonymous
Sat 19 Feb 2005 12:56:46 PM UTC, comment #5: 

The current release of findutils fixes this problem; please
download it from ftp://ftp.gnu.org/gnu/findutils or from one of
the mirror sites listed at http://www.gnu.org/software/findutils/.

James Youngman <jay>
Group administrator
Sun 13 Feb 2005 10:20:20 AM UTC, comment #4: 

Dmitry, thanks for looking at this.

1. That hunk of test code existed only to place a marker in the strace output.  It should have been removed earlier.  It would have leaked a file descriptor!

2. You're right.  That exposes find to the very security problem that this mechanism is supposed to fix!  Stupid me.  Fixed. 

3. Most of the checks in dafely_chdir_lstat() just relate to checking and reporting problems with the lstat() results.  I believe that these checks are no longer required, or do you believe I have missed something?  I have moved complete_pending_execdirs() up into safely_chdir() though. 

Thanks for the very useful feedback.   I attach an updated patch.   The updated patch is really a patch afainst findutils 4.2.17, so it also contains some minor changes to the find manpage which aren;t relevant here.

James Youngman <jay>
Group administrator
Sat 12 Feb 2005 11:50:30 PM UTC, comment #3: 

1. this hunk looks odd:

+  time(NULL);
+  opendir(".");
+  time(NULL);
+

2. safely_chdir_nofollow() will switch to safely_chdir_lstat() if open() fails due to some race.

3. I'd prefer to avoid runtime fallback to safely_chdir_lstat() on platforms where fchdir() is known to work.

4. At this moment, safely_chdir_nofollow() does less then safely_chdir_lstat().  Maybe some stuff could be moved back to safely_chdir()?

Dmitry V. Levin <ldv>
Sat 12 Feb 2005 09:16:34 PM UTC, comment #2: 

I have just checked the attached patch into CVS.  This resolves the problem by using open(..., O_NOFOLLOW) followed by fchdir() instead of chdir().  This eliminates the need to lstat() the target directory before and after, hence eliminating the need for the mounted device list.  However, fchdir() does not always work on all systems where the system call exists, and so in these circumstances we fall back on the previous implementation (enumerating the mounted devices at that point). 

This is, to say the least, a significant change to the way that find works.  The regression test suite passes, but then it only has 19 tests. 

Please test this change thoroughly and let me know how it works for you.  You can do this either by downloading the code from CVS (which I would recommend) or by applying the attached patch (against findutils-4.2.17).  If I get negative feedback I can redo the fix or whatever, but if I get positive feedback we can move towards another release.

James Youngman <jay>
Group administrator
Mon 07 Feb 2005 01:53:01 PM UTC, comment #1: 

This code was introduced to fix bug #9043 and bug #3998.

It would be nice if init_mounted_dev_list() was only called if we know we're going to need its result, but as I understand things by the time we know we need its (initial) result, it's too late to call it. 

Any suggestions as to how all three bugs could be fixed simultaneously would be welcome.   Thanks.

James Youngman <jay>
Group administrator
Mon 07 Feb 2005 10:23:13 AM UTC, original submission:  

In main(), init_mounted_dev_list() is called unconditionally.  This results in all mount-points getting lstat() called on them.

This happens when no options are given.

If one of the mount-points is a hard NFS mount, and the remote machine is down, find will never complete even though it should be able to without problems.

Tim Waugh <twaugh>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #2544:  findutils-4.2.17-nofollow-try2.patch added by jay (24KiB - text/x-patch - Second proposed fix (as patch against 4.2.17 release))
file #2541:  findutils-4.2.17-nofollow.patch added by jay (20KiB - text/x-patch - First proposed fix for bug #11879 (patch against 4.2.17 release))

 

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 8 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2005-02-19 jay Open/ClosedOpen Closed
    Fixed ReleaseNone 4.2.18
2005-02-13 jay Attached File- Added findutils-4.2.17-nofollow-try2.patch, #2187
2005-02-12 ldv Carbon-Copy- Added ldv
2005-02-12 jay StatusNone Fixed
    Attached File- Added findutils-4.2.17-nofollow.patch, #2184
2005-02-07 jay Item GroupNone Wrong result
    Assigned toNone jay

Back to the top

Powered by Savane 3.13-d3ae.
Corresponding source code