From f249fdd8c19ff65825c0be67212cdf22e556668e Mon Sep 17 00:00:00 2001 From: Harvey Harrison Date: Tue, 29 Apr 2008 00:59:03 -0700 Subject: autofs4: fix sparse warning in root.c fs/autofs4/root.c:536:23: warning: symbol 'ino' shadows an earlier one fs/autofs4/root.c:510:22: originally declared here There is no need to redeclare, we are at the end of the loop and in the next iteration of the loop, ino will be reset. Signed-off-by: Harvey Harrison Acked-by: Ian Kent Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/autofs4/root.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/autofs4/root.c') diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index a54a946a50ae..aa4c5ff8a40d 100644 --- a/fs/autofs4/root.c +++ b/fs/autofs4/root.c @@ -533,9 +533,9 @@ static struct dentry *autofs4_lookup_unhashed(struct autofs_sb_info *sbi, struct goto next; if (d_unhashed(dentry)) { - struct autofs_info *ino = autofs4_dentry_ino(dentry); struct inode *inode = dentry->d_inode; + ino = autofs4_dentry_ino(dentry); list_del_init(&ino->rehash); dget(dentry); /* -- cgit v1.2.3 From 033790449ba9c4dcf8478a87693d33df625c23b5 Mon Sep 17 00:00:00 2001 From: Jeff Moyer Date: Thu, 1 May 2008 04:35:08 -0700 Subject: 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(+) (limited to 'fs/autofs4/root.c') diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index aa4c5ff8a40d..0533d37c73ae 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; } -- cgit v1.2.3 From 9d2de6ad2a78bb8b60bf7a54e6043dca44e9a801 Mon Sep 17 00:00:00 2001 From: Jeff Moyer Date: Thu, 1 May 2008 04:35:09 -0700 Subject: autofs4: fix incorrect return from root.c:try_to_fill_dentry() Jeff Moyer has identified a case where the autofs4 function root.c:try_to_fill_dentry() can return -EBUSY when it should return 0. Jeff's description of the way this happens is: "automount starts an expire for directory d. after the callout to the daemon, but before the rmdir, another process tries to walk into the same directory. It puts itself onto the waitq, pending the expiration. When the expire finishes, the second process is woken up. In try_to_fill_dentry, it does this check: status = d_invalidate(dentry); if (status != -EBUSY) return -EAGAIN; And status is EBUSY. The dentry still has a non-zero d_inode, and the flags do not contain LOOKUP_CONTINUE or LOOKUP_DIRECTORY So, we fall through and return -EBUSY to the caller." Signed-off-by: Jeff Moyer Signed-off-by: Ian Kent Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/autofs4/root.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/autofs4/root.c') diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index 0533d37c73ae..6e250030a690 100644 --- a/fs/autofs4/root.c +++ b/fs/autofs4/root.c @@ -243,7 +243,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; + int status; /* Block on any pending expiry here; invalidate the dentry when expiration is done to trigger mount request with a new @@ -340,7 +340,7 @@ static int try_to_fill_dentry(struct dentry *dentry, int flags) } } - return status; + return 0; } /* For autofs direct mounts the follow link triggers the mount */ -- cgit v1.2.3 From 868eb7a8539d3e8c494209be2b1f4084a274dfef Mon Sep 17 00:00:00 2001 From: Jan Blunck Date: Thu, 1 May 2008 04:35:10 -0700 Subject: autofs: path_{get,put}() cleanups Here are some more places where path_{get,put}() can be used instead of dput()/mntput() pair. Besides that it fixes a bug in autofs4_mount_busy() where mntput() was called before dput(). Signed-off-by: Jan Blunck Cc: Ian Kent Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/autofs4/root.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'fs/autofs4/root.c') diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index 6e250030a690..edf5b6bddb52 100644 --- a/fs/autofs4/root.c +++ b/fs/autofs4/root.c @@ -146,17 +146,17 @@ static int autofs4_dir_open(struct inode *inode, struct file *file) if (d_mountpoint(dentry)) { struct file *fp = NULL; - struct vfsmount *fp_mnt = mntget(mnt); - struct dentry *fp_dentry = dget(dentry); + struct path fp_path = { .dentry = dentry, .mnt = mnt }; - if (!autofs4_follow_mount(&fp_mnt, &fp_dentry)) { - dput(fp_dentry); - mntput(fp_mnt); + path_get(&fp_path); + + if (!autofs4_follow_mount(&fp_path.mnt, &fp_path.dentry)) { + path_put(&fp_path); dcache_dir_close(inode, file); goto out; } - fp = dentry_open(fp_dentry, fp_mnt, file->f_flags); + fp = dentry_open(fp_path.dentry, fp_path.mnt, file->f_flags); status = PTR_ERR(fp); if (IS_ERR(fp)) { dcache_dir_close(inode, file); -- cgit v1.2.3