LINUX: Avoid d_revalidate failure on mtpt mismatch
authorAndrew Deason <adeason@sinenomine.net>
Tue, 28 Oct 2014 05:10:56 +0000 (00:10 -0500)
committerD Brashear <shadow@your-file-system.com>
Fri, 31 Oct 2014 12:38:29 +0000 (08:38 -0400)
Currently, if afs_linux_dentry_revalidate is given an inode that
corresponds to a mtpt vcache ('vcp'), it resolves the mtpt to its root
dir if it's easy to do so (mvid and CMValid are set). Later on, we run
afs_lookup to see if looking up our dentry's name returns the same
vcache that we got; afs_lookup presumably will also resolve the mtpt
if it's easy to do so.

However, it is possible that afs_linux_dentry_revalidate and
afs_lookup will make different decisions as to whether or not they
resolve a mtpt to a dir. Specifically, if CMValid is cleared after
afs_linux_dentry_revalidate checks for it, but before afs_lookup does,
then afs_lookup will return a different vcache than
afs_linux_dentry_revalidate is expecting, even though the relevant
directory entry has not changed. That is, tvc is not equal to vcp, but
tvc could be a mtpt that resolves to vcp, or vice versa. CMValid can
be cleared by another thread at virtually any time, since this is
cleared in some situations when we're not sure if the mtpt resolution
is still valid (callbacks are broken, vldb cache entries expire, etc).

afs_linux_dentry_revalidate interprets this situation to mean that the
directory entry has changed, and so it eventually d_drop's the
associated dentry. The way that this manifests to users is that a
"fakestatted" mtpt can appear to be deleted effectively randomly, even
when nothing has changed. This can be a problem because this causes
the getcwd() syscall to return ENOENT when the working directory
involves such an affected directory.

To fix this situation, we just detect if afs_lookup returned either
'vcp' (our possibly-resolved vcache), or the original inode associated
with the dentry we are revalidating. If the returned vcache matches
either of these, then the entry is okay and we don't need to
invalidate or drop anything.

FIXES 131780

Change-Id: Ide1dd224d1ea1e29a82eb7130a010877cf4e9fc7
Reviewed-on: http://gerrit.openafs.org/11559
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Anders Kaseorg <andersk@mit.edu>
Reviewed-by: Anders Kaseorg <andersk@mit.edu>
Reviewed-by: D Brashear <shadow@your-file-system.com>

src/afs/LINUX/osi_vnodeops.c

index 0cdd9e0..b2ab9d5 100644 (file)
@@ -1291,9 +1291,36 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags)
        if (locked && (parent_dv > dp->d_time || !(vcp->f.states & CStatd))) {
            struct vattr *vattr = NULL;
            int code;
+           int lookup_good;
 
            code = afs_lookup(pvcp, (char *)dp->d_name.name, &tvc, credp);
-           if (code || tvc != vcp) {
+
+           if (code) {
+               /* We couldn't perform the lookup, so we're not okay. */
+               lookup_good = 0;
+
+           } else if (tvc == vcp) {
+               /* We got back the same vcache, so we're good. */
+               lookup_good = 1;
+
+           } else if (tvc == VTOAFS(dp->d_inode)) {
+               /* We got back the same vcache, so we're good. This is
+                * different from the above case, because sometimes 'vcp' is
+                * not the same as the vcache for dp->d_inode, if 'vcp' was a
+                * mtpt and we evaluated it to a root dir. In rare cases,
+                * afs_lookup might not evalute the mtpt when we do, or vice
+                * versa, so the previous case will not succeed. But this is
+                * still 'correct', so make sure not to mark the dentry as
+                * invalid; it still points to the same thing! */
+               lookup_good = 1;
+
+           } else {
+               /* We got back a different file, so we're definitely not
+                * okay. */
+               lookup_good = 0;
+           }
+
+           if (!lookup_good) {
                dput(parent);
                /* Force unhash; the name doesn't point to this file
                 * anymore. */