[PATCH 1/3] afs: Fix validation/callback interaction

[David Howells]

When afs_validate() is called to validate a vnode (inode), there are two
unhandled cases in the fastpath at the top of the function:

 (1) If the vnode is promised (AFS_VNODE_CB_PROMISED is set), the break
     counters match and the data has expired, then there's an implicit case
     in which the vnode needs revalidating.

     This has no consequences since the default "valid = false" set at the
     top of the function happens to do the right thing.

 (2) If the vnode is not promised and it hasn't been deleted
     (AFS_VNODE_DELETED is not set) then there's a default case we're not
     handling in which the vnode is invalid.  If the vnode is invalid, we
     need to bring cb_s_break and cb_v_break up to date before we refetch
     the status.

     As a consequence, once the server loses track of the client
     (ie. sufficient time has passed since we last sent it an operation),
     it will send us a CB.InitCallBackState* operation when we next try to
     talk to it.  This calls afs_init_callback_state() which increments
     afs_server::cb_s_break, but this then doesn't propagate to the
     afs_vnode record.

     The result being that every afs_validate() call thereafter sends a
     status fetch operation to the server.

Clarify and fix this by:

 (A) Setting valid in all the branches rather than initialising it at the
     top so that the compiler catches where we've missed.

 (B) Restructuring the logic in the 'promised' branch so that we set valid
     to false if the callback is due to expire (or has expired) and so that
     the final case is that the vnode is still valid.

 (C) Adding an else-statement that ups cb_s_break and cb_v_break if the
     promised and deleted cases don't match.

Fixes: c435ee34551e ("afs: Overhaul the callback handling")
Signed-off-by: David Howells <dhowe...@redhat.com>
---

 fs/afs/inode.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 4c6d8e1112c2..6b17d3620414 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -382,7 +382,7 @@ void afs_zap_data(struct afs_vnode *vnode)
 int afs_validate(struct afs_vnode *vnode, struct key *key)
 {
        time64_t now = ktime_get_real_seconds();
-       bool valid = false;
+       bool valid;
        int ret;
 
        _enter("{v={%llx:%llu} fl=%lx},%x",
@@ -402,15 +402,21 @@ int afs_validate(struct afs_vnode *vnode, struct key *key)
                        vnode->cb_v_break = vnode->volume->cb_v_break;
                        valid = false;
                } else if (vnode->status.type == AFS_FTYPE_DIR &&
-                          test_bit(AFS_VNODE_DIR_VALID, &vnode->flags) &&
-                          vnode->cb_expires_at - 10 > now) {
-                       valid = true;
-               } else if (!test_bit(AFS_VNODE_ZAP_DATA, &vnode->flags) &&
-                          vnode->cb_expires_at - 10 > now) {
+                          (!test_bit(AFS_VNODE_DIR_VALID, &vnode->flags) ||
+                           vnode->cb_expires_at - 10 <= now)) {
+                       valid = false;
+               } else if (test_bit(AFS_VNODE_ZAP_DATA, &vnode->flags) ||
+                          vnode->cb_expires_at - 10 <= now) {
+                       valid = false;
+               } else {
                        valid = true;
                }
        } else if (test_bit(AFS_VNODE_DELETED, &vnode->flags)) {
                valid = true;
+       } else {
+               vnode->cb_s_break = vnode->cb_interest->server->cb_s_break;
+               vnode->cb_v_break = vnode->volume->cb_v_break;
+               valid = false;
        }
 
        read_sequnlock_excl(&vnode->cb_lock);