Skip Menu |

This queue is for tickets about the File-Path CPAN distribution.

Report information
The Basics
Id: 75162
Status: rejected
Priority: 0/
Queue: File-Path

People
Owner: RICHE [...] cpan.org
Requestors: jaekel@cablecats.de (no email address)
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 2.08
Fixed in: (no value)



Race condition check triggered by automounter. File::Path's _rmtree function contains a race condition check that clashes with OpenSolaris NFS mirror mounts, and possibly NFS automounts, aswell. The check is necessary and good, but needs a small additional step when dynamic mounts are used. The current steps are: 1. lstat($root) 2. chdir($root) 3. stat(".") and then compare inode and FS ids. NFS mirror mounts are not triggered by an lstat() on the mount point, so the inode and FS id of the mount point are returned. chmod() however triggers the mounting of the share, and the second stat() returns different ids - the inode of the share's root directory. I've added a simple "step 0" to work around this, which will trigger the auto-mount before the unaltered race condition check is performed. This should have only minimal performance and no security impact, but it also works with mirror mounts. With added step: 0. stat(File::Spec->catdir($root, $curdir)) # e.g. "$root/." on *nix 1. lstat($root) 2. chdir($root) 3. stat(".") Step 0 triggers the automount, so the inodes will now match. I've added a (very simple) patch file for your convenience.
Subject: file_path_mirrormount.patch
diff -crB File-Path-2.08/Path.pm File-Path-2.08.patched/Path.pm *** File-Path-2.08/Path.pm Sun Oct 4 12:15:58 2009 --- File-Path-2.08.patched/Path.pm Mon Feb 20 16:29:21 2012 *************** *** 276,281 **** --- 276,285 ---- : $root ; + # trigger possible automount / mirror mount before race condition stat + my $root_plus_curdir = File::Spec->catfile($root, $curdir); + stat $root_plus_curdir; # ignore result + my ($ldev, $lino, $perm) = (lstat $root)[0,1,2] or next ROOT_DIR; if ( -d _ ) {
On Mon Feb 20 10:51:59 2012, AJAEKEL wrote: Show quoted text
> Race condition check triggered by automounter. > > File::Path's _rmtree function contains a race condition check that > clashes with OpenSolaris NFS mirror mounts, and possibly NFS automounts, > aswell. The check is necessary and good, but needs a small additional > step when dynamic mounts are used. > > The current steps are: > > 1. lstat($root) > 2. chdir($root) > 3. stat(".") > > and then compare inode and FS ids. > > NFS mirror mounts are not triggered by an lstat() on the mount point, so > the inode and FS id of the mount point are returned. chmod() however > triggers the mounting of the share, and the second stat() returns > different ids - the inode of the share's root directory. > > I've added a simple "step 0" to work around this, which will trigger the > auto-mount before the unaltered race condition check is performed. This > should have only minimal performance and no security impact, but it also > works with mirror mounts. > > With added step: > > 0. stat(File::Spec->catdir($root, $curdir)) # e.g. "$root/." on *nix > 1. lstat($root) > 2. chdir($root) > 3. stat(".") > > Step 0 triggers the automount, so the inodes will now match. > > I've added a (very simple) patch file for your convenience.
Can you suggest how we might write a test for this? Thank you very much. Jim Keenan
Show quoted text
> Can you suggest how we might write a test for this?
Tests in t/ would not be practical. In order to make it repeatable we would need to have the capability to create and mount NFS shares. This capability requires root privilege. We can make them author tests, though. The OP neglected to post the OpenSolaris version and processor architecture, so perhaps I can look at what the most commonly used version was in 2012 and start from there. If I can’t reproduce on Solaris x86 then we would need to ask a p5p solaris user if it can be repro. I personally have stumbled across this issue (over 10 years ago) and in bash scripts just did "ls" on a directory I knew was NFS -- so I know that the problem can exist. I guess the fundamental question is -- is File::Path responsible for making sure a user's NFS directories are mounted? Is there anything bad that can happen if we don't force the mount?
Note the following commit describing the limitation and the separation of concerns regarding this issue. https://github.com/rpcme/File-Path/commit/60c14331cfdf30980adbcd61ec2ac1fb1a65c4e5
Documentation relating to this reported problem is shipped in 2.11. However. the patch will be rejected. Thank you for highlighting what can be frustrating to users so we could document it.