Skip to content

Commit

Permalink
autofs4: fix execution order race in mount request code
Browse files Browse the repository at this point in the history
Jeff Moyer has identified a race in due to an execution order dependency
in the autofs4 function root.c:try_to_fill_dentry().

Jeff's description of this race is:

"P1 does a lookup of /mount/submount/foo.  Since the VFS can't find an entry
for "foo" under /mount/submount, it calls into the autofs4 kernel module to
allocate a new dentry, D1.  The kernel creates a new waitq for this lookup and
calls the daemon to perform the mount.

The daemon performs a mkdir of the "foo" directory under /mount/submount,
which ends up creating a *new* dentry, D2.

Then, P2 does a lookup of /mount/submount/foo.  The VFS path walking logic
finds a dentry in the dcache, D2, and calls the revalidate function with this.
 In the autofs4 revalidate code, we then trigger a mount, since the dentry is
an empty directory that isn't a mountpoint, and so set DCACHE_AUTOFS_PENDING
and call into the wait code to trigger the mount.

The wait code finds our existing waitq entry (since it is keyed off of the
directory name) and adds itself to the list of waiters.

After the daemon finishes the mount, it calls back into the kernel to release
the waiters.  When this happens, P1 is woken up and goes about clearing the
DCACHE_AUTOFS_PENDING flag, but it does this in D1!  So, given that P1 in our
case is a program that will immediately try to access a file under
/mount/submount/foo, we end up finding the dentry D2 which still has the
pending flag set, and we set out to wait for a mount *again*!

So, one way to address this is to re-do the lookup at the end of
try_to_fill_dentry, and to clear the pending flag on the hashed dentry.  This
seems a sane approach to me."

And Jeff's patch does this.

Signed-off-by: Jeff Moyer <[email protected]>
Signed-off-by-by: Ian Kent <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
JeffMoyer authored and torvalds committed May 1, 2008
1 parent cab0936 commit 0337904
Showing 1 changed file with 22 additions and 0 deletions.
22 changes: 22 additions & 0 deletions fs/autofs4/root.c
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ static int try_to_fill_dentry(struct dentry *dentry, int flags)
{
struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
struct autofs_info *ino = autofs4_dentry_ino(dentry);
struct dentry *new;
int status = 0;

/* Block on any pending expiry here; invalidate the dentry
Expand Down Expand Up @@ -318,6 +319,27 @@ static int try_to_fill_dentry(struct dentry *dentry, int flags)
spin_lock(&dentry->d_lock);
dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
spin_unlock(&dentry->d_lock);

/*
* The dentry that is passed in from lookup may not be the one
* we end up using, as mkdir can create a new one. If this
* happens, and another process tries the lookup at the same time,
* it will set the PENDING flag on this new dentry, but add itself
* to our waitq. Then, if after the lookup succeeds, the first
* process that requested the mount performs another lookup of the
* same directory, it will show up as still pending! So, we need
* to redo the lookup here and clear pending on that dentry.
*/
if (d_unhashed(dentry)) {
new = d_lookup(dentry->d_parent, &dentry->d_name);
if (new) {
spin_lock(&new->d_lock);
new->d_flags &= ~DCACHE_AUTOFS_PENDING;
spin_unlock(&new->d_lock);
dput(new);
}
}

return status;
}

Expand Down

0 comments on commit 0337904

Please sign in to comment.