4.19-stable review patch. If anyone has any objections, please let me know.
------------------
[ Upstream commit ae3b7361dc0ee9a425bf7d77ce211f533500b39b ]
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 dhowells@redhat.com Signed-off-by: Al Viro viro@zeniv.linux.org.uk Signed-off-by: Sasha Levin sashal@kernel.org --- 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 479b7fdda124..071075d775a9 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -379,7 +379,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={%x:%u} fl=%lx},%x", @@ -399,15 +399,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);