From 033790449ba9c4dcf8478a87693d33df625c23b5 Mon Sep 17 00:00:00 2001 From: Jeff Moyer Date: Thu, 1 May 2008 04:35:08 -0700 Subject: [PATCH] autofs4: fix execution order race in mount request code 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 Signed-off-by-by: Ian Kent Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/autofs4/root.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index aa4c5ff8a4..0533d37c73 100644 --- a/fs/autofs4/root.c +++ b/fs/autofs4/root.c @@ -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 @@ -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; } -- 2.39.5