LINUX: Workaround d_splice_alias/d_lookup race 38/12638/3
authorAndrew Deason <adeason@sinenomine.net>
Thu, 15 Jun 2017 20:32:41 +0000 (15:32 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Wed, 26 Jul 2017 12:37:12 +0000 (08:37 -0400)
Before Linux kernel commit 4919c5e45a91b5db5a41695fe0357fbdff0d5767,
d_splice_alias in some cases can d_rehash the given dentry without
attaching it to the given inode, right before the dentry is unhashed
again. This means that for a few moments, that negative dentry is
visible to __d_lookup, and thus is visible to path lookup and can be
given to afs_linux_dentry_revalidate.

Currently, afs_linux_dentry_revalidate will say that the dentry is
valid, because d_time and other fields are set; it's just not attached
to an inode. This causes an ENOENT error on lookup, even though the
file is there (and no OpenAFS code said otherwise).

Normally this race is rare, but it can be frequently exercised if
we access the same directory via different names at the same time.
This can happen with multiple mountpoints to the same volume, or by
accessing an @sys directory via its abbreviated and expanded forms.

To get around this, make afs_linux_dentry_revalidate check negative
'dentry's to see if they are unhashed. We also lock the parent inode,
in order to guarantee that a problematic d_splice_alias call isn't
running at the same time (and thus, we know the dentry will not be
unhashed immediately afterwards). This slows down
afs_linux_dentry_revalidate for valid negative 'dentry's a little, but
it allows us to use negative dentry's at all.

Linux kernel commit 4919c5e45a91b5db5a41695fe0357fbdff0d5767 fixes
this issue, which was included in 2.6.34, so don't do this workaround
for 2.6.34 and on.

Change-Id: I8e58ebed4441151832054b1ef3f1aa5af1c4a9b5
Reviewed-on: https://gerrit.openafs.org/12638
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/afs/LINUX/osi_vnodeops.c

index 357751c..d058545 100644 (file)
 #define MAX_ERRNO 1000L
 #endif
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,34)
+/* Enable our workaround for a race with d_splice_alias. The race was fixed in
+ * 2.6.34, so don't do it after that point. */
+# define D_SPLICE_ALIAS_RACE
+#endif
+
 int cachefs_noreadpage = 0;
 
 extern struct backing_dev_info *afs_backing_dev_info;
@@ -1130,6 +1136,44 @@ parent_vcache_dv(struct inode *inode, cred_t *credp)
     return hgetlo(pvcp->f.m.DataVersion);
 }
 
+#ifdef D_SPLICE_ALIAS_RACE
+/* Leave some trace that this code is enabled; otherwise it's pretty hard to
+ * tell. */
+static __attribute__((used)) const char dentry_race_marker[] = "d_splice_alias race workaround enabled";
+
+static int
+check_dentry_race(struct dentry *dp)
+{
+    int raced = 0;
+    if (!dp->d_inode) {
+        struct dentry *parent = dget_parent(dp);
+
+        /* In Linux, before commit 4919c5e45a91b5db5a41695fe0357fbdff0d5767,
+         * d_splice_alias can momentarily hash a dentry before it's fully
+         * populated. This only happens for a moment, since it's unhashed again
+         * right after (in d_move), but this can make the dentry be found by
+         * __d_lookup, and then given to us.
+         *
+         * So check if the dentry is unhashed; if it is, then the dentry is not
+         * valid. We lock the parent inode to ensure that d_splice_alias is no
+         * longer running (the inode mutex will be held during
+         * afs_linux_lookup). Locking d_lock is required to check the dentry's
+         * flags, so lock that, too.
+         */
+        afs_linux_lock_inode(parent->d_inode);
+        spin_lock(&dp->d_lock);
+        if (d_unhashed(dp)) {
+            raced = 1;
+        }
+        spin_unlock(&dp->d_lock);
+        afs_linux_unlock_inode(parent->d_inode);
+
+        dput(parent);
+    }
+    return raced;
+}
+#endif /* D_SPLICE_ALIAS_RACE */
+
 /* Validate a dentry. Return 1 if unchanged, 0 if VFS layer should re-evaluate.
  * In kernels 2.2.10 and above, we are passed an additional flags var which
  * may have either the LOOKUP_FOLLOW OR LOOKUP_DIRECTORY set in which case
@@ -1166,6 +1210,13 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags)
        return -ECHILD;
 #endif
 
+#ifdef D_SPLICE_ALIAS_RACE
+    if (check_dentry_race(dp)) {
+        valid = 0;
+        return valid;
+    }
+#endif
+
     AFS_GLOCK();
     afs_InitFakeStat(&fakestate);