| [2637] | 1 | From c9bbebe869acadd61abfa50656447429cde18aaf Mon Sep 17 00:00:00 2001 |
|---|
| 2 | From: Andrew Deason <adeason@sinenomine.net> |
|---|
| 3 | Date: Tue, 28 Oct 2014 00:10:56 -0500 |
|---|
| 4 | Subject: [PATCH] LINUX: Avoid d_revalidate failure on mtpt mismatch |
|---|
| 5 | |
|---|
| 6 | Currently, if afs_linux_dentry_revalidate is given an inode that |
|---|
| 7 | corresponds to a mtpt vcache ('vcp'), it resolves the mtpt to its root |
|---|
| 8 | dir if it's easy to do so (mvid and CMValid are set). Later on, we run |
|---|
| 9 | afs_lookup to see if looking up our dentry's name returns the same |
|---|
| 10 | vcache that we got; afs_lookup presumably will also resolve the mtpt |
|---|
| 11 | if it's easy to do so. |
|---|
| 12 | |
|---|
| 13 | However, it is possible that afs_linux_dentry_revalidate and |
|---|
| 14 | afs_lookup will make different decisions as to whether or not they |
|---|
| 15 | resolve a mtpt to a dir. Specifically, if CMValid is cleared after |
|---|
| 16 | afs_linux_dentry_revalidate checks for it, but before afs_lookup does, |
|---|
| 17 | then afs_lookup will return a different vcache than |
|---|
| 18 | afs_linux_dentry_revalidate is expecting, even though the relevant |
|---|
| 19 | directory entry has not changed. That is, tvc is not equal to vcp, but |
|---|
| 20 | tvc could be a mtpt that resolves to vcp, or vice versa. CMValid can |
|---|
| 21 | be cleared by another thread at virtually any time, since this is |
|---|
| 22 | cleared in some situations when we're not sure if the mtpt resolution |
|---|
| 23 | is still valid (callbacks are broken, vldb cache entries expire, etc). |
|---|
| 24 | |
|---|
| 25 | afs_linux_dentry_revalidate interprets this situation to mean that the |
|---|
| 26 | directory entry has changed, and so it eventually d_drop's the |
|---|
| 27 | associated dentry. The way that this manifests to users is that a |
|---|
| 28 | "fakestatted" mtpt can appear to be deleted effectively randomly, even |
|---|
| 29 | when nothing has changed. This can be a problem because this causes |
|---|
| 30 | the getcwd() syscall to return ENOENT when the working directory |
|---|
| 31 | involves such an affected directory. |
|---|
| 32 | |
|---|
| 33 | To fix this situation, we just detect if afs_lookup returned either |
|---|
| 34 | 'vcp' (our possibly-resolved vcache), or the original inode associated |
|---|
| 35 | with the dentry we are revalidating. If the returned vcache matches |
|---|
| 36 | either of these, then the entry is okay and we don't need to |
|---|
| 37 | invalidate or drop anything. |
|---|
| 38 | |
|---|
| 39 | FIXES 131780 |
|---|
| 40 | |
|---|
| 41 | Change-Id: Ide1dd224d1ea1e29a82eb7130a010877cf4e9fc7 |
|---|
| 42 | (cherry picked from commit 45d9ff86571cbe70fedd524266fac50bd2d4bc2d) |
|---|
| 43 | --- |
|---|
| 44 | src/afs/LINUX/osi_vnodeops.c | 29 ++++++++++++++++++++++++++++- |
|---|
| 45 | 1 file changed, 28 insertions(+), 1 deletion(-) |
|---|
| 46 | |
|---|
| 47 | diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c |
|---|
| 48 | index 9ed6f01..8c63f96 100644 |
|---|
| 49 | --- a/src/afs/LINUX/osi_vnodeops.c |
|---|
| 50 | +++ b/src/afs/LINUX/osi_vnodeops.c |
|---|
| 51 | @@ -1231,10 +1231,37 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags) |
|---|
| 52 | if (hgetlo(pvcp->f.m.DataVersion) > dp->d_time || !(vcp->f.states & CStatd)) { |
|---|
| 53 | struct vattr *vattr = NULL; |
|---|
| 54 | int code; |
|---|
| 55 | + int lookup_good; |
|---|
| 56 | |
|---|
| 57 | credp = crref(); |
|---|
| 58 | code = afs_lookup(pvcp, (char *)dp->d_name.name, &tvc, credp); |
|---|
| 59 | - if (code || tvc != vcp) { |
|---|
| 60 | + |
|---|
| 61 | + if (code) { |
|---|
| 62 | + /* We couldn't perform the lookup, so we're not okay. */ |
|---|
| 63 | + lookup_good = 0; |
|---|
| 64 | + |
|---|
| 65 | + } else if (tvc == vcp) { |
|---|
| 66 | + /* We got back the same vcache, so we're good. */ |
|---|
| 67 | + lookup_good = 1; |
|---|
| 68 | + |
|---|
| 69 | + } else if (tvc == VTOAFS(dp->d_inode)) { |
|---|
| 70 | + /* We got back the same vcache, so we're good. This is |
|---|
| 71 | + * different from the above case, because sometimes 'vcp' is |
|---|
| 72 | + * not the same as the vcache for dp->d_inode, if 'vcp' was a |
|---|
| 73 | + * mtpt and we evaluated it to a root dir. In rare cases, |
|---|
| 74 | + * afs_lookup might not evalute the mtpt when we do, or vice |
|---|
| 75 | + * versa, so the previous case will not succeed. But this is |
|---|
| 76 | + * still 'correct', so make sure not to mark the dentry as |
|---|
| 77 | + * invalid; it still points to the same thing! */ |
|---|
| 78 | + lookup_good = 1; |
|---|
| 79 | + |
|---|
| 80 | + } else { |
|---|
| 81 | + /* We got back a different file, so we're definitely not |
|---|
| 82 | + * okay. */ |
|---|
| 83 | + lookup_good = 0; |
|---|
| 84 | + } |
|---|
| 85 | + |
|---|
| 86 | + if (!lookup_good) { |
|---|
| 87 | dput(parent); |
|---|
| 88 | /* Force unhash; the name doesn't point to this file |
|---|
| 89 | * anymore. */ |
|---|
| 90 | -- |
|---|
| 91 | 2.1.2 |
|---|
| 92 | |
|---|